Conversation
github-actions
bot
requested review from
hwangjokim,
junggyo1020,
rladmstn and
s-hwan
Test Results 19 files 19 suites 6s ⏱️ Results for commit e2e3222. ♻️ This comment has been updated with latest results. |
| if (status == ProblemListStatus.IN_PROGRESS) { | ||
| if (unsolvedOnly == null) { | ||
| throw new InvalidRequestException(HttpStatus.BAD_REQUEST, "IN_PROGRESS 상태에서는 unsolvedOnly 는 필수입니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
개인적으로는 IN_PROGRESS 상태에 대해서 unsolvedOnly 값이 null로 들어오면 unsolvedOnly=false라는 것과 동일한 의미라고 생각이 드는데요! 그래서 위 부분에 대한 검증이 없어도 되지 않을까 싶었습니다. 어떻게 생각하시나요?!
There was a problem hiding this comment.
음 이 unsolvedOnly가 이후에서 findAllInProgressProblem 이 쿼리부분에서 사용되어서요! null로 들어오면 NPE가 발생해서 검증을 걸어둔거긴해요.
사실 말한거에 동의하기는 해서 서비스 로직부분에서 null 인경우 false로 바꾸도록 할까요?
There was a problem hiding this comment.
저는 서비스계층에서 false로 변경해도 될 것 같아보입니다!
| this.status = status; | ||
| this.error = error; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
예외 객체 record로 만들면 편하게 쓸 수 있을 것 같아요!
There was a problem hiding this comment.
record를 사용한 것에 이점이 뭘까요?! 어처피 지금 구현하는 예외 객체들은 대부분 status, error로 단순하게 사용하니까 record로 관리하는게 편한건가? record사용시 확장성은 잃는다고해서요..!!
There was a problem hiding this comment.
저는 그냥 생성자, getter 명시가 없어도 되어지니, 이렇게 불변적인 클래스는 웬만하면 record로 만들어서 간결함을 유지하려고 하는 편입니다!
확장성은 예외 객체에서는 고려하지 않아도 된다고 생각해요. BaseException 같은 객체면 모르겠지만, 이런 상세 경우에 대한 Exception 객체는 확장할 경우가 없다고 생각해서요!
There was a problem hiding this comment.
넵 다음 예외 클래스 만들때마다 record로 만들게요! 이거 만든게 필요없어져서 삭제했습니다
4 participants
📌 Related Issue
be-41
🚀 Description
📢 Review Point
📚Etc (선택)