[H팀 임재영] 백엔드 API 과제 제출 - #2
Conversation
yim0327
changed the title
jinu0328
left a comment
jinu0328
left a comment
There was a problem hiding this comment.
과제 고생 많으셨습니다! 몇가지 피드백을 남겨드렸으니 한번 봐주시고 이해가 안되는 점 있다면 편하게 연락주세요 👍
There was a problem hiding this comment.
현재 Task 클래스에서 setTitle과 setIsDone을 통해 외부에서 필드값을 직접 수정하도록 하고있습니다. 단순한 프로젝트에서 해당 방식은 문제가 되지 않지만 객체 지향적인 설계를 지향한다고 했을 때 개선이 필요한 부분이기도합니다. set 메서드는 상태를 무분별하게 변경할 수 있는 위험을 내포하고 있으므로 지양하는 것이 바람직합니다. 간단한 구조더라도 update와 같은 메서드의 도입을 통해 상태 변경을 별도로 관리하려는 습관을 들여보면 좋을 것 같습니다. 👍
| public TaskRequestDto toRequestDto() { | ||
| return new TaskRequestDto(this.title, this.isDone); | ||
| } | ||
|
|
There was a problem hiding this comment.
내부의 dto 변환 메서드를 통해 getter를 우회하신 것은 좋습니다 👍
There was a problem hiding this comment.
GlobalExceptionHandler를 사용하여 상위 레벨에서 예외를 처리하는 방식은 좋은 설계 방식이라고 생각합니다. 현재는 IllegalArgumentException만을 처리하고있지만 커스텀 예외 클래스를 더 추가해준다면 해당 클래스의 이점이 더 살아날 것 같습니다.
다만 패키지의 위치는 조금 애매해 보입니다. 전역적인 예외처리를 담당하는 만큼 별도의 exception이나 error 패키지를 두는 것이 좋을 것 같습니다 👍
There was a problem hiding this comment.
RequestDto는 보통 Request Body의 형식에 맞춰 구성하게 됩니다. 그러나 현재 반환에도 RequestDto가 사용되고 있습니다.
Response Body 형식에 맞는 별도의 dto를 구현하고 그것을 반환에 이용하는 것이 맞는 방식이 될 것 같습니다.
ex) 현재 TaskRequestDto에는 id 값을 포함하고있지 않아 추후 클라이언트측에서 path parameter에 필요한 id 값을 넣어 요청을 보낼 수 없을 것 같습니다. id가 포함된 TaskResponseDto를 별도로 구현해야합니다
2 participants
No description provided.