커스텀 예외 처리 #76
커스텀 예외 처리 #76
Conversation
| private HttpStatus httpStatus; | ||
| private String message; |
There was a problem hiding this comment.
(개인 취향)
필드 순서가 상태코드-메시지가 아니라 메시지-상태코드 여도 좋을것같아요! 반영 필요 없음!
| .orElseThrow(() -> new MoimException(NOT_FOUND, MOIM_NOT_FOUND)); | ||
|
|
||
| return MoimDetailsFindResponse.toResponse(moim); | ||
| } | ||
|
|
||
| public void joinMoim(MoimJoinRequest moimJoinRequest) { | ||
| Moim moim = moimRepository.findById(moimJoinRequest.moimId()) | ||
| .orElseThrow(() -> new IllegalArgumentException("모임이 존재하지 않습니다.")); | ||
| .orElseThrow(() -> new MoimException(NOT_FOUND, MOIM_NOT_FOUND)); | ||
| moim.join(); | ||
| } | ||
|
|
||
| public void deleteMoim(long id) { | ||
| Moim moim = moimRepository.findById(id) | ||
| .orElseThrow(IllegalArgumentException::new); | ||
| .orElseThrow(() -> new MoimException(NOT_FOUND, MOIM_NOT_FOUND)); |
There was a problem hiding this comment.
지금 여기서 MoimException을 생성할땐 HttpStatus가 static import 되어있네용😄
There was a problem hiding this comment.
회의 내용 반영하여 static import 하지 않고, 필드에서 도메인 prefix는 제거했습니다~!
|
|
||
| return MoimDetailsFindResponse.toResponse(moim); | ||
| } | ||
|
|
||
| public void joinMoim(MoimJoinRequest moimJoinRequest) { | ||
| Moim moim = moimRepository.findById(moimJoinRequest.moimId()) | ||
| .orElseThrow(() -> new IllegalArgumentException("모임이 존재하지 않습니다.")); | ||
| .orElseThrow(() -> new MoimException(NOT_FOUND, MOIM_NOT_FOUND)); |
There was a problem hiding this comment.
Moim 객체에서는 static import가 안되어 있지만 서비스에서는 되어있네요!
이 컨벤션에 대해 이야기 해보는 것도 좋을 것 같습니당
There was a problem hiding this comment.
회의 내용 반영하여 static import 하지 않고, 필드에서 도메인 prefix는 제거했습니다~!
|
|
||
| @Getter | ||
| @AllArgsConstructor | ||
| public class ServiceException extends RuntimeException { |
There was a problem hiding this comment.
현재 RuntimeException -> ServiceException -> MoimException 3개의 뎁스를 타고 있는 것 같아요.
혹시 ServiceException은 어떤 역할인가요..?!
There was a problem hiding this comment.
RuntimeException을 핸들링 하기에는 저희가 놓친 예외도 핸들링 될 가능성이 있고, 도메인 별 예외 핸들러를 만들기엔 중복 코드가 많아질 것으로 판단했습니다!
ServiceException을 개발팀이 인지하고 있는 예외로 정의하고, 추가적으로 도메인 별 예외 클래스가 생기는 경우 ServiceException을 상속받아 정의하는 방식을 택하고자 합니다 😊
| public enum MoimErrorMessage { | ||
|
|
||
| MOIM_NOT_FOUND("모임이 존재하지 않습니다."), | ||
| MOIM_MAX_PEOPLE("모임 최대 인원 수를 초과합니다."), | ||
| ; | ||
|
|
||
| private final String message; | ||
| } |
There was a problem hiding this comment.
MoimErrorMessage라고 자원이 명시된 경우라면 상수 이름의 Prefix에 Moim을 명시하지 않아도 될 것 같아요 !😄
Moim이라는 도메인명이 바뀌게되면 같이 바뀌는 부분이 많아질까 하는 생각입니다.
- static import를 사용한다면 더더욱 ..
There was a problem hiding this comment.
회의 내용 반영하여 static import 하지 않고, 필드에서 도메인 prefix는 제거했습니다~!
| public ResponseEntity<ErrorResponse> handleGlobalException(ServiceException e) { | ||
| return ResponseEntity.status(e.getHttpStatus()).body(new ErrorResponse(e.getMessage())); |
There was a problem hiding this comment.
줄임말 e 대신에 exception으로 통일하는 게 좋을까요?
이것도 컨벤션으로 정해봅시다!
There was a problem hiding this comment.
회의 시간에 다루지 못한 내용이지만 축약을 지양하고자 exception으로 변경 후 다시 리뷰 요청 드려요~
|
현재 기존에 어떤 예외인지 인지하기 어렵다는 문제를 해결하고자 저희 서비스에서 규정하는 예외라는 의미를 담았습니다. 이견있으시다면 코멘트 부탁드려용 !~ |
PR의 목적이 무엇인가요?
이슈 ID는 무엇인가요?
설명
예외 응답의 경우 아래와 같은 JSON 형태를 반환합니다.
{ "message": "모임이 존재하지 않습니다." }질문 혹은 공유 사항 (Optional)