-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement core module #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~! 추가로 논의하면 좋을 것 같은 부분들 코멘트 남겼어요!!
@@ -0,0 +1,7 @@ | |||
package nexters.dividend.core.exception; | |||
|
|||
public record ErrorResponse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
record로 선언한게 인상깊네요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불변 객체이면서 보일러플레이트 코드를 줄여준다는 점에서 DTO 로 사용하면 좋을 것 같더라구요ㅎㅎ
kotlin에서도 유사하게 DTO 로 class
보다는 data class
를 주로 사용해요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 그러네요 data class를 찾아봤는데 record랑 거의 똑같은 문법이군요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불변 객체, 상속 가능 여부를 제외하고는 비슷한 용도로 사용하기에 좋은 것 같아요! 😸
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HttpMessageNotReadableException (잘못된 형식으로 request를 요청할 경우), MissingServletRequestParameterException (필수 request parameter가 누락되었을 때),
MethodArgumentNotValidException (validation 오류가 발생했을 때)
이 정도 예외는 꽤 빈번하게 발생할 수 있을거 같아서 global exception handler에서 핸들링하는건 어떠신가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로 예외 처리를 빠뜨리거나 등등 이유로 controller 단에서 잡히지 않은 exception에 대해서 클라이언트에게 알려주기 위해 Exception.class를 global exception handler에서 잡는 것은 어떠신가요??
그냥 500 에러 발생하는 것보다 메시지라도 띄워주는게 어떤가 해서요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉 500 에러를 놓쳤군용
누락된 HttpMessageNotReadableException
, MissingServletRequestParameterException
도 추가하는게 좋을 것 같아요!!
꼼꼼한 리뷰 감사합니다ㅎㅎ 수정해놓을게요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~!
Issue Number #7
close: #7
작업 내역
변경사항
api-server
,batch
가core
모듈을 의존PR 특이 사항
core
모듈 구성하면서 추가로 넣어놓았는데 세부 설정 (롤링 정책 등)은 추후 논의해보면 좋을 것 같습니다!