이 코드 리팩터링 어떻게 해야 될까요? (java, spring)

Question

이런 로직입니다.DB 접근도 너무 많이 하는 것 같고,,, (어쩔 수 없는 부분이겠죠 이건?)전체적으로 코드가 좀 더러운 느낌입니다. (Builder 패턴 때문인 것 같기도 하고요)메서드 추출로 이것저것 추출 해봤지만 그래도 더러운 코드라서 다시 합쳤습니다.이런 상황에서 리팩터링 하고 싶은데 어떻게 해야 될까요?시간 되신다면 따끔한 피드백도 하나 해주시면 감사하겠습니다. (진짜 따끔해도 괜찮습니다.)

감독님테스트가하고싶어요 · Accepted Answer

2번, 3번 반환 예외도 같은데 굳이 2번에 걸쳐서 할 필요가 있을까요?처음부터 지금 진행중인 프로모션을 타이틀로 조회해서 없으면 예외처리해도 되지않을까요?4번, 6번 순서가 조금 의문입니다. 유저의 프로모션 참여 기록을 확인한 후 유저가 존재하는지 확인하는게 개인적으론 조금 이상하게 느껴지네요.5번 내부 Rank 생성의 경우 해당 로직은 Rank enum class안에서 해결할 수 있을 것 같은데 여기까지 꺼내온게 조금 아쉽게 느껴집니다.4번, 7번의 경우 participate 관련된 서비스를 만들어서 한번에 처리할 수 있지 않을까요?예를 들면 보통 유저 생성시 중복가입을 검증한 후 통과하면 유저를 생성합니다. 마찬가지로 참가 기록에 대한 중복 확인한 후 없으면 기록을 저장하는 방식으로요. 이렇게 하면 코드 재활용이 더 쉬워지지 않을까요. 어차피 참가 중복 확인과 참가 기록하는 것은 묶여서 다닐 것 같은데요.리워드 저장할때 꽝일때도 해당 메세지와 함께 저장하면 ParticipateResponse 생성을 if로 분기할 필요는 없어보입니다. 다만 rank에 꽝에 해당하는 Rank.FAIL 생성 및 rank.toString으로 하지말고 enum 클래스 안에 String message 필드를 만들어서 처리해야할 것 같지만요.생성 관련해서 정적 팩토리 메서드 사용하면 좀 더 깔끔하게 보일 수도 있습니다.해당 의견은 제 개인적인 의견일 뿐 더 좋은 방법이 아닐 수도 있습니다.

yeori · Answer

1) 경품을 디비에 밀어넣는 방식을 살짝 바꾸고, 2) 경품 당첨 로직도 살짝 바꿔서 디비 접근을 줄이고 로직도 간단해질 수 있을듯 합니다.그냥 참고만 하시고..일단 제가 경품 당첨 확률을 이해하기를, 100면짜리 주사위를 던져서 숫자 1~5는 1등, 6~20은 2등, 21~50은 3등, 나머지 꽝으로 이해했습니다. 근데 주사위가 3이 나왔는데 이미 1등 경품이 오링난 경우는 그냥 꽝으로 처리하는 것인지, 이 부분이 약간 정리가 안되는데 일단 넘어가고...경품 밀어넣기편의상 1등 5개, 2등 15개 3등 30개 합쳐서 50개라고 칩시다.50개를 랜덤하게 섞습니다.(쉐킷쉐킷)그리고 seq 를 차례대로 붙어서 밀어넣습니다.당첨로직먼저 프로모션에 참여했다고 기록부터 합니다.당첨 로직을 정리하면 50%의 확률로 뭐라도 받거나 꽝이 나옵니다.그래서 먼저 꽝인지 결정합니다.꽝이 나오면 바로 끝냅니다.꽝이 아니면 select로 당첨자가 없는 rewrad중에 seq가 가장 낮은값이든 높은값이든 걍 아무거나 하나만 뽑습니다.이미 경품이 모두 소진됐다면 null이 나올겁니다.끝냅니다.이전에 디비에 집어넣을때 잘 섞었기때문에 1~3등 경품 중 확률에 맞게 나옵니다.reward에 당첨자를 기록하고 끝냅니다.그런데 이 로직은 님이 짠 코드랑 약간 결이 다릅니다.님이 짠 코드에서는 주사위 던져서 1등이 나왔는데 남은 경품이 없으면 그냥 꽝이 됩니다.제가 정리한 방식에서는 1등이든 3등이든 일단 50% 확률만 넘기면 반드시 받어먹게 됩니다.이거는 알아서 하시고..

이 코드 리팩터링 어떻게 해야 될까요? (java, spring)

2개의 답변

#refactoring 목록