게시글 상세조회 시 로그인/비로그인 조건 분리 #38
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if (authentication.getPrincipal().equals("anonymousUser")) { | ||
| throw new NotFoundException(ErrorCode.LOGIN_REQUIRED); | ||
| } | ||
| return authentication.getPrincipal().equals("anonymousUser"); | ||
| } | ||
| } |
There was a problem hiding this comment.
주요 변경 사항 및 제안:
NotFoundException이 삭제되었으므로, 해당 예외 관련 코드도 제거됨.loginCheck메서드가isLogin으로 변경되었고, 호출 시에 true 또는 false를 반환하게 되었습니다.isLogin메서드 이름으로부터 반환 값을 기대하기 때문에 처리 동작이 명시적으로 표현됩니다.isLogin메서드에서 인증 principal을 "anonymousUser"와 비교하고 있기 때문에, 이 부분이 안전한지 재검토 필요합니다.- 코드 리뷰 후
isLogin메서드 반환 값의 사용 방식을 확인하여 해당 로직의 안정성을 보완하는 것이 좋습니다.
여기서 수정된 부분들은 예기치 않은 동작을 유발하진 않지만, isLogin 메서드의 반환 값을 활용하기 위해선 코드 범위 내에서 이를 적절히 처리하는 것이 중요합니다.
| .createAt(comment.getCreatedAt()) | ||
| .reactions(comment.getEmojiCounts()) | ||
| .build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 조각은 전체적으로 깔끔하고 효율적인 것으로 보입니다. 몇 가지 제안 및 개선 사항은 다음과 같습니다:
-
보안 문제: 사용자 ID (userId)를 기반으로 하는 필터링 로직은 null 포인터 예외를 일으킬 수 있습니다. 사용자 ID가 null인 경우에 대비하여 추가 체크가 필요합니다.
-
중복 로직: from 메서드에서 일부 중복된 로직이 보입니다. 이러한 부분을 별도의 private 메서드로 추출하여 코드 중복을 방지할 수 있습니다.
-
명확성: 각 메서드의 역할을 더 명확하게 설명하는 주석을 추가함으로써 코드의 가독성을 향상시킬 수 있습니다.
-
네이밍: 변수 이름과 메소드 이름은 읽기 쉬워야 합니다. 변수 및 메소드 이름을 더 명확하고 의미 있는 이름으로 변경하여 코드 이해를 용이하게 할 수 있습니다.
-
테스트: 단위 테스트가 구현되어 있는지 확인하고, 모든 케이스에 대한 테스트 커버리지가 충분한지 확인하는 것이 좋습니다.
-
Error Handling: 잠재적인 에러 상황에 대한 처리가 충분한지 확인하세요. 에러 발생 시 적절한 방식으로 처리할 수 있도록 보완해야 합니다.
-
Java 버전: Java 버전에 따라
toList()메소드가 지원되지 않을 수 있으므로 Java 버전을 고려해야 합니다.
위 제안사항들을 고려하여 코드를 개선할 수 있을 것입니다.
| .commentCount(comments.size()) | ||
| .comments(commentDtos) | ||
| .build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드는 from 메서드를 사용하여 PostDetailDto 객체를 생성하는 두 가지 오버로드된 메서드를 제공합니다.
-
개선 사항:
disAgreeCount메서드의 이름이 오타가 있습니다. "disagreement" 대신에 "disAgree"로 수정되어야 합니다.agreeCount및disagreeCount변수를 선언하고 할당하는 과정에서null값이 반환될 수 있으므로, 이에 대한 안전한 처리가 필요합니다.
-
버그 위험:
Map<Boolean, Long>형태의voteCounts맵을 사용하기 때문에 boolean 값 외에 다른 키가 포함될 수 있는데, 이에 대한 예외 처리나 보다 명확한 구분을 위한 코드 수정이 필요할 수 있습니다.
-
추가적인 권장:
- 입력 값으로부터 예외 처리를 보강할 필요가 있습니다.
- 메서드명과 변수명은 의미 전달력 있게 작성되어야 합니다.
- 주석을 추가하여 각 부분의 역할을 더 명확하게 설명할 수 있습니다.
정리하면, 오타 수정, null 값 처리방식 강화, 예외 처리 추가, 변수 및 메서드명의 의미 전달력 강화를 고려해야 합니다.
| return PostDetailDto.from(post, comments, securityUtil.getCustomUserId()); | ||
| return securityUtil.isLogin() | ||
| ? PostDetailDto.from(post, comments) | ||
| : PostDetailDto.from(post, comments, securityUtil.getCustomUserId()); | ||
| } | ||
|
|
||
| public Post findById(final Long id) { |
There was a problem hiding this comment.
주어진 코드에서 주요 변경 사항은 readPost 메서드 내부의 조건식이다. 이 조건식을 살펴보면 로그인 여부를 확인하고 그에 따라 다른 방식으로 PostDetailDto를 반환한다는 점을 알 수 있습니다.
개선 제안:
- 조건문 개선: 삼항 연산자 대신 if-else 문을 사용하여 가독성을 향상시킬 수 있습니다.
- 코드 중복 최소화:
PostDetailDto.from메소드를 호출할 때에도 중복 코드가 있는데, 이를 최대한 줄일 수 있는 방법을 고려해 볼 필요가 있습니다. - 예외처리:
findById메소드에서 예외 처리가 없는데, 오류 상황에 대비하여 적절한 예외 처리를 추가하는 것이 좋습니다.
잠재적 버그 위험 요인은 현재로서는 명확히 식별되지 않았습니다. 전체 시스템 구조와 요구사항에 맞게 코드가 작성되었는지를 더 자세히 분석해야 합니다.
No description provided.