Conversation
| nickName = nickName + suffix; | ||
| suffix++; | ||
| } | ||
| return nickName; |
There was a problem hiding this comment.
이렇게 하면 suffix가 100일 때 쿼리가 100번 날라갈 것 같아요
There was a problem hiding this comment.
그쵸... 효율적인 방법 찾아서 리팩토링 하겠습니답!
| public UserInfoResponse completeStep(Long userId, OnboardingRequest onboardingRequest) { | ||
| User user = userRepository.findById(userId) | ||
| .orElseThrow(() -> new BadRequestException(ErrorCode.USER_NOT_FOUND)); | ||
| if (user.getOnboardingStep() == null) { |
There was a problem hiding this comment.
OnboardingStep이 null인 경우가 있나요?
| public OnboardingRequest { | ||
| if (onboardingStep == null | ||
| || onboardingStep.isEmpty() | ||
| || onboardingStep.values().stream().noneMatch(Boolean.TRUE::equals) |
There was a problem hiding this comment.
이 부분이 잘 이해가 안가는데 onboardingStep이 전부 false여야 하는건가요?
그리고 이 부분이 만약에 단순 타입 검증같은 부분이 아니라면 dto에 있는 건 어색한 것 같아요
There was a problem hiding this comment.
처음 user생성 시에는 onboardingStep이 false로 초기화 돼있고,
onboardingStep 요청 왔을 때, 둘 중 하나는 꼭 true여야 업데이트가 가능합니다.
요청에 둘다 false면 true를 반환해서 예외처리 했습니다!
그리고, 보통 request DTO에 적절한 요청인지 검증하는 로직을 넣지 않나요..?
There was a problem hiding this comment.
처음 user생성 시에는 onboardingStep이 false로 초기화 돼있고,
onboardingStep 요청 왔을 때, 둘 중 하나는 꼭 true여야 업데이트가 가능합니다.
요청에 둘다 false면 true를 반환해서 예외처리 했습니다!
onboardingStep을 한 번에 모두 true로 처리하는 경우가 있나요?
그리고, 보통 request DTO에 적절한 요청인지 검증하는 로직을 넣지 않나요..?
dto는 데이터의 단순 검증(null, 타입, 형태 등)만 처리하고, 말씀해주신 처음 user생성 시에는 onboardingStep이 false로 초기화 돼있고, onboardingStep 요청 왔을 때, 둘 중 하나는 꼭 true여야 업데이트가 가능합니다. 처럼 약간 애매하긴 해도 비즈니스 로직의 성향을 띄는 경우는 서비스, 또는 도메인 영역에서 처리를 해야한다고 생각합니다!
There was a problem hiding this comment.
onboardingStep에 모든 항목이 true로 들어와 처리 하는 경우는 없습니다. 현재는 예외처리를 하지 않고 풀어놨습니다.
다만, onboardingStep에 모든 항목(FIRST_VOTE, WELCOME_GUIDE)이 false로 들어오는 경우는 요청자체가 무의미하기 때문에 예외처리를 했습니다.
두번째로, 도메인 레벨에서의 검증은 도메인이 영원불변하게 지켜할 값, 예를들면 게시글 설명은 100자 이상 불가와 같이 영원히 100자 이상으로 설명을 초기화 할 수 없는 규칙을 검증하는게 맞다고 생각합니다. onboardingStep은 생성 당시, false 초기화이고 true가 될 수 있는 것이기 때문에 도메인레벨에서의 검증이 어색하다고 느꼈습니다.
제가 이번에 DTO로 뺀 이유는 요청의 유스케이스별 검증은 Dto에서 바로 검증 후 컨트롤러 단에서 에러를 발생시키는게 자연스럽다고 생각해서 였습니다.
DTO에 비즈니스 규칙을 넣는 느낌이라면, 서비스레벨에서 검증하는 것도 좋습니다.
일단은 서비스레벨에 분리하는 방식으로 수정 커밋하겠습니다!
| import com.chooz.user.domain.OnboardingStepType; | ||
| import com.chooz.user.domain.User; | ||
|
|
||
| import java.util.*; |
There was a problem hiding this comment.
지금 코드 대부분 와일드카드로 바뀌고 있는데 인텔리제이 설정 한 번 확인해주시면 감사하겠습니다!
| while (usedSuffixes.contains(suffix)) { | ||
| suffix = suffix.add(BigInteger.ONE); | ||
| } | ||
| return suffix.signum() == 0 ? prefix : prefix + suffix; |
There was a problem hiding this comment.
그냥 단순하게 최대값 + 1으로 하는 방식은 어떠신가요?
지금 방식대로 하면 중간에 없는 숫자를 재사용할 수 있어서 좋긴 한데 그냥 단순한게 더 좋지 않나 싶어서 여쭤봅니다
물론 이대로 하셔도 괜찮습니다!
There was a problem hiding this comment.
그럴까하다가 진호님이 좋아할 것 같아서...
2 participants
🚀 작업 내용 설명
📢 그 외
유저 정보 수정은 금주 내로 완료 하겠습니다
📌 관련 이슈
#194