[Feature] - 여행기 나라 관련 리팩토링 #586
Conversation
Test Results 30 files 30 suites 8s ⏱️ Results for commit 0246790. ♻️ This comment has been updated with latest results. |
| @@ -29,25 +21,6 @@ public class TagController { | |||
|
|
|||
| private final TagService tagService; | |||
|
|
|||
| @Operation(summary = "태그 생성") | |||
backend/src/main/java/kr/touroot/travelogue/service/TravelogueCountryService.java
Outdated
Show resolved
Hide resolved
| private void validateCountryCode(CountryCode countryCode) { | ||
| if (countryCode == CountryCode.NONE) { | ||
| throw new BadRequestException("여행기의 국가 코드는 NONE 일 수 없습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
오잉? enum에 존재하는 값 중 하나만 예외적으로 처리되네요.
이 부분은 많이 어색한 것 같습니다.
CountryCode에 NONE이라는 값이 없던지 예외 처리를 안하는 방식이 자연스럽다고 생각해요.
해당 검증은 왜 필요한가요??
There was a problem hiding this comment.
먼저 NONE은, 사용자가 검색 시 존재하지 않는 나라에 대해 검색을 하더라도 예외를 발생시키는 것이 아니라 빈 결과를 반환하기 위해 도입되었습니다. null을 반환해서 처리하는 방법도 있겠지만, 일종의 null object pattern으로 NONE을 도입한거구요!
하지만 TravelogueCountry 는 NONE 코드에 대해 저장될 필요가 없습니다.
NONE 을 null 로 감싼 것이라고 생각하면 말이죠.
하지만 해당 부분이 되려 도메인을 조금 복잡하게 만드는 경향이 있다는 생각이 드는 것 같습니다.
차라리 null로 처리하는 것이 나을수도 있을 것 같은데, 이에 대해 어떻게 생각하실까요??
| private Map<CountryCode, Long> countCountries(TravelogueRequest request) { | ||
| return request.days().stream() | ||
| .flatMap(day -> day.places().stream()) | ||
| .map(place -> CountryCode.valueOf(place.countryCode())) | ||
| .filter(countryCode -> countryCode != CountryCode.NONE) | ||
| .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())); | ||
| } |
There was a problem hiding this comment.
해당 부분은 일급 컬렉션으로 묶을까 하다, 두었는데 이에 대해 어떻게 생각하실까욥?
| private void validateCount(Integer count) { | ||
| if (count < MIN_COUNT) { | ||
| throw new BadRequestException(String.format("국가 코드의 개수는 %d 보다 커야합니다.", MIN_COUNT)); | ||
| } | ||
| } |
There was a problem hiding this comment.
해당 부분은 개발자가 직접 계산해서 넣어주는 값이기 때문에, 사용자의 잘못된 요청이 아닌 서버의 에러로 봐야 한다고 생각하는데요.
이뿐만 아니라 null 체킹도 일부는 서버의 에러로 봐야할 것 같다고 생각합니다.
해당 부분은 이후에 같이 이야기해보면 좋을 것 같아요.
✅ 작업 내용
🙈 참고 사항
충분히 리팩토링을 하려 했지만, 버그 수정이 빨리 올라가야 할 것 같아 빠르게 PR 올립니다.