-
Notifications
You must be signed in to change notification settings - Fork 170
[그리디] 허석준 Spring MVC 3,4단계 제출합니다 #440
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
Conversation
TaeyeonRoyce
left a comment
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.
안녕하세요, 석준님!
고민이 잘 묻어난 섬세한 코드네요. 고생 많으셨습니다!
몇가지 질문과 고민 혹은 학습 해볼만한 내용을 적어 두었습니다.
확인 하시고 답변주세요!
PR에 남겨주신 질문은 하나씩 하단에 남겨둘게요!
레이어드 아케텍쳐와 DTO
각 계층은 역할에 맞게 잘 구성 하신 것 같아요.
석준님이 애매하게 느끼신 점은, 계층간 데이터를 주고 받을 때 사용한 DTO는 어디서 생성하지? 인 것 같습니다.
우선, DTO에 대해 잠깐 짚고 넘어갈게요. Data Transfer Object인건 아실테고,,, 단순히 데이터만 담는 것을 넘어서 계층간 데이터를 주고 받을 때 사용 됩니다.
그림에서처럼 API 요청-응답에서만 사용 되는 것만은 아니고, 모든 계층간의 이동에서도 사용 할 수 있어요.
API ---dtoA---> Controller ---dtoB---> Service
현재 석준님은 dto를 계층과 무관하게 전역 레벨로 두고, service, controller 모두 동일하게 사용하고 있죠. 각 계층에 요구되는 데이터 구조에 맞게 dto를 만들 수 있을 것 같아요.
예시로 설명 해 볼게요!
예약 생성을 할 때, 기존 필드에 추가적으로 회원 식별 값이 추가적으로 필요 해졌다고 해봅시다.
그리고, 일종의 규칙으로 인해 Http body가 아닌 header에 요청을 담아 보내 준다고 해볼게요.
예약 생성 요청을 받는 Controller에서는 달라진 정책에 맞게 아래처럼 변경 될 거에요.
package dto;
public record ReservationRequest(
String name,
LocalDate date,
LocalTime time
) {
public Reservation toEntity() {
return new Reservation(name, date, time);
}
}@RestController
public class ReservationApiController {
@PostMapping("/reservations")
public ResponseEntity<ReservationResponse> save(
@RequestBody ReservationRequest reservationRequest,
@RequestHeader("userId") String userId // 요청 헤더 추가
) {
// 로직 생략...
}DTO인 ReservationRequest은 그대로 유지 되고, Header 값을 추가로 받아요.
그럼 이제, ReservationRequest를 같이 사용하는 ReservationService의 save()는 어떻게 될까요?
Service에서는 userId가 필요하지만 ReservationRequest에 포함되지 않아 사용 할 수 없습니다. 메서드 파라미터에 추가할 수도 있지만, Service에서 **"예약 생성에 필요한 데이터를 담는 객체"**를 만들 수도 있어요.
// Service에서 사용 되는 "예약 생성에 필요한 데이터를 담는 객체"
package service.dto; // service 영역에 포함
public record SaveReservationCommand(
String userId, // 추가
String name,
LocalDate date,
LocalTime time
) {
}@Service
public class ReservationService {
public ReservationResponse save(SaveReservationCommand command) {
command.userId();
}
}위 방식처럼 각 계층이 필요한 Dto를 정의하여 사용 할 수도 있습니다.
계층이 공유하던 결합을 끊어 내니 변경사항이 계층을 넘어 영향을 주지 않기도 하죠.
대부분의 의존관계 설계에서는 양방향 의존성을 없애는 것이 유지보수에 유리합니다.
- ref 양방향 의존성을 피하라
계층형 아키텍쳐 역시 양방향 의존성을 줄이기 위해, "의존성이 한 방향으로 흘러야 한다" 는 특징을 준수하면 좋아요. 요청의 흐름을 파악하기 쉬운 특징을 가진 것 처럼,
요청 ---> Presentation Layer ---> Application Layer ---> Persistence Layer의 의존 방향을 유지해야 좋습니다.
현재 DTO를 객체로 변환하는 로직은 서비스에서 처리하고 있고, DTO 내부에서 엔티티로 변환하는 메서드를 정의해 사용하고 있는데요.
저는 이 구조가 크게 문제는 없어보여요. 하지만, Service(Application Layer)에서 controller.dto 에 위치한 ReservationResponse를 만드는 것은 양방향 의존이 발생하는 겁니다.
이 부분을 개선해보면 좋을 것 같아요! 아래 사진처럼 Applcation Layer에서 정의하는 dto를 만들어 결합을 끊어내볼 수 있을 것 같아요!

예외 처리
Java에서의 예외 사용은 잘 하신 것 같습니다! 어색한 부분은 없었어요.
잘 활용 했는지에 대한 물음에는 간단하게 답변 가능 하지만, 좀 더 찾아보시면서 여러 방식을 터득 하면 좋을 것 같아요.
각 상황에 맞는 예외를 정의 하는 것은 매우 중요해요.
빠르게 파악하고 대응 하기에도 유리하기 때문입니다.
석준님은 예외 상황 마다 class를 만들어서 사용 하셨는데, 이건 프로젝트 규모나 취향, 특징에 따라 많이 달라질 수 있어요.
어떤식의 네이밍을 가질 지, 어떤 메세지를 어디까지 담을 지, 어떤 방식으로 구현할 지 등등...
다행히, 국내에서 Java/Spring으로 이루어진 프로젝트를 많이많이 발견 할 수 있어요...!
저희 그리디 팀 프로젝트 뿐만 아니라, 여러 교육 기관 및 다른 동아리의 프로젝트를 보면서 "예외 처리"를 어떻게 하는지 파악 해보시면 좋을 것 같습니다!
제가 "이렇게 하는게 좋아!", "이게 정답이야"라고 할 수 있는 문제도 아니다 보니, 여러 케이스를 보면서 참고해보시죠!
아마 크게 2, 3가지의 구조가 보일 텐데, 각각의 장단점도 분석하시면서 석준님의 케이스를 만들어 보면 좋을 것 같아요.
발견하기 쉽게 몇개 남겨둘게여...
@ResponseStatus 사용
단순 두 개의 선택지만 놓고 비교하자면, ResponseEntity로 응답하는게 더 유연하고 안전 할 것 같아요.
ResponseEntity는 응답 상태 뿐만 아니라 body를 포함하여 header 지정 등 추가적인 옵션을 다룰 수 있어요. Http 응답에 관련된 대부분의 기능을 한번에 제공합니다. @ResponseStatus의 역할도 포함 하고 있죠.
응답에 대한 관심사가 흩어져있다면, 관리하기 불편할거에요.
그리고, @ResponseStatus을 사용한다고 하더라도, 응답 body나 header를 지정하기 위해 ResponseEntity를 사용하게 될텐데, 개발자의 실수로 메서드 상단에 정의된 @ResponseStatus와 다른 응답값을 ResponseEntity에 지정 할수도 있어요.
상황에 따라, 팀 내규에 따라 선택할 수 있지만... 저라면 ResponseEntity를 통해 응답에 대한 관심사를 한 곳에서 관리 하도록 할 것 같습니다...!
키워드 발견 뿐만 아니라, 고민과 함께 질문 주셔서 감사합니다..! 의문이 드는 점들이 많으실텐데 언제든지 질문 주세요~!
README.MD
Outdated
|
|
||
| ### 잘못된 요청 (필드 누락 등) | ||
|
|
||
| - 예외: `IllegalArgumentException` |
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.
각 필드에 대한 설명과 필수값 여부, 예외 발생 조건 까지..!
친절하게 기재된 API 명세서 좋네요! 고생 많으셨습니다.
다만, IllegalArgumentException, NotFoundReservationException 처럼 백엔드 코드에서만 사용 되는 용어는 굳이 안넣어도 될 것 같아요.
프로덕트를 만들어가는 팀원들, 특히 클라이언트 개발자 모두에게 익숙하고 이해 가능한 정보만 포함 되면 좋습니다.
응답으로 해당 이름의 메세지가 포함 된다면, 기재 하여도 문제 없겠지만... 불필요한 정보는 명확성을 저해하여 팀 단위의 작업 속도를 늦추거나 불필요한 소통이 발생 할 수 있어서요!
흠 잡을 곳 없는 API 명세서지만, 굳이 언급 드린 이유는 소통을 할 때 단어/용어에 대한 고민을 하는 관점도 있으면 좋을 것 같았습니다.
개발을 하다보면 개인에게 자연스러운 용어 선택이 소통을 저해하는 경우도 발생 할 수 있어서요!
타 직군(Client 개발자 포함)을 고려하여, 프로덕트 중심의 언어 선택 역시 좋은 소프트스킬이 될 수 있어요.
이미지 출처 위 출처에 유비쿼터스 언어, MDD라는 키워드도 있는데 심심하면 읽어보세요!
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.
백엔드 개발자뿐만 아니라 다양한 직군의 사람들이 함께 협업하는 만큼,
특정 기술 영역에서만 사용되는 용어나 예외 클래스명을 문서에 직접 노출하는 건 지양해야겠다는 생각이 들었습니다.
예전에 프로젝트를 하면서 기획 단계에서 소통이 원활하지 않아 엉뚱한 방향으로 구현해버린 경험이 떠오르네요... 😅
자바 스터디를 할 때 "개발자가 아닌 사람도 README를 읽고 이해할 수 있어야 한다"는 조언을 들었던 기억이 있는데,
이번 경험을 통해 그 말의 진짜 의미를 다시금 되새기게 되었습니다!
|
|
||
| @Controller | ||
| public class HomeController { | ||
| public class PageController { |
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.
정적 페이지를 제공하는 역할에 맞는 더 적합한 네이밍이네요.
섬세하시군요!👍
| return ResponseEntity.created(URI.create("/reservations/" + savedReservation.id())) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(savedReservation); |
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.
생성된 자원에 대한 201 CREATED 응답코드를 사용 하셨군요! 좋습니다.
근데, 그냥 200 OK로 응답하면 안되나요?
Location 헤더에 링크를 담는 이유는 어떤걸까요?
Rest API를 설계할 때 HATEOS, Self-descriptive라는 규칙도 존재 하는데,
(개인적으로 꼭 지켜야 할 필요는 없다고 생각합니다만...) 한번 찾아보시면 좋을 것 같아요!
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.
사실 statusCode(201)이랑 Location: /reservations/1 헤더는 테스트 코드를 맞추기 위해 적용해봤어요.
테스트가 없었다면 저도 그냥 익숙하게 200 OK로 응답했을 것 같아요.
이번 기회를 통해 Self-descriptive와 HATEOAS라는 개념을 처음 알게 됐는데요
Self-descriptive는 응답 메시지만 봐도 그 의미나 상태를 파악할 수 있어야 한다는 원칙이고,
HATEOAS는 애플리케이션 상태 전이가 하이퍼링크를 통해 이루어져야 한다는 개념이더라고요.
이런 구조 덕분에 클라이언트는 서버 내부 구조를 몰라도
링크나 응답 메시지만 보고 어떤 동작을 할 수 있다는 점이 꽤 인상 깊었습니다.
공부하면서 평소에 아무 생각 없이 쓰던 웹이 왜 이렇게 잘 확장되고, 독립적으로 진화해왔는지에 대한 힌트를 얻은 느낌이었고
동시에 지금 우리가 만들고 있는 REST API들이 과연 진짜 RESTful한가? 라는 고민도 들었어요.
참고로 GitHub API도 한 번 구경해봤는데 응답 안에 links, rel, Location 같은 필드들이 자연스럽게 포함돼 있어서
제가 평소에 만들던 API와는 꽤 다른 느낌이 들더라고요.
RESTful이 뭔지, 그리고 왜 호환성이 좋은지 앞으로 한번 더 알아봐야겠네요 😄
| private String code; | ||
| private String message; |
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.
불변이 가능한 곳은 final을 잊지 말아주세요!
| private String code; | |
| private String message; | |
| private final String code; | |
| private final String message; |
+ 얘도 데이터 객체와 다르지 않아서 record도 적합해 보이네요
| import org.springframework.web.bind.annotation.RestControllerAdvice; | ||
| import roomescape.controller.ReservationApiController; | ||
|
|
||
| @RestControllerAdvice(basePackageClasses = ReservationApiController.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.
basePackageClasses를 통해 에러 핸들링 범위를 지정 하셨네요.
의도하신 것 같은데 어떤 목적인지 알려주세요..!
일부러 범위를 지정했다면 고려해야 할 점.
- 현재 커버 되지 않는 핸들러(PageController.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.
basePackageClasses를 사용해 에러 핸들링 범위를 지정한 이유는,
처음에는 전역적으로 처리하기보다는 특정 컨트롤러에 맞는 예외만 분리해서 다루는 게 좋겠다는 막연한 생각 때문이었습니다.
그래서 공부 중에 핸들링 범위를 지정할 수 있다는 걸 알게 되었고, ReservationApiController에 대한 예외만 처리되도록 구현했습니다.
하지만 지금 다시 생각해보면,
스프링은 더 구체적인 핸들러가 우선 호출된다는 특징이 있기 때문에,
전역 핸들러를 두고, 필요한 경우에만 특정 컨트롤러용 핸들러를 추가하는 쪽이 더 유연하고 안전하다고 생각했어요.
예상하지 못한 예외나 커버되지 않는 컨트롤러에 대비해 전역 핸들러도 함께 두는 방향으로 개선해보겠습니다!
| @Repository | ||
| public class ReservationRepository { | ||
|
|
||
| private static Map<Long, Reservation> store = new LinkedHashMap<>(); |
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.
LinkedHashMap 구조를 선택한 이유가 있나요?!
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.
처음엔 사실 별 생각 없이 LinkedHashMap을 썼어요.
그냥 Map이 필요하니까 무심코 선택했던 것 같아요. 😅
이번처럼 id로 조회하고 수정/삭제하는 게 주된 용도라면, 순서는 전혀 중요하지 않으니 HashMap이 더 적절하겠네요.
이번 기회에 자료구조도 그냥 쓰는 게 아니라 "왜 이걸 쓰는지"를 한 번쯤 더 생각하고 쓰는 습관을 가져야겠다고 느꼈어요!
| } | ||
|
|
||
| public ReservationResponse save(ReservationRequest reservationRequest) { | ||
| validate(reservationRequest); |
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.
검증이 static class에서 이루어지고 있네요.
계층형 아키텍쳐를 도입한 이상, 의존성과 관련 없이 어느 계층에서나 사용 가능한 static class는 최대한 안쓰는게 좋을 것 같아요.
물론, utility성(ex. Random, DateTime, ...)이 강하여 프로덕트 도메인과 무관하다면 사용 할 수 있죠.
하지만, 하나뿐인 Reservation 도메인의 규칙을 담는 로직은 말 그대로 도메인에 담겨야 할 것 같아요.
현재는 reservationRequest에 대한 검증이 이루어지고 있는데,
- 입력값에 대한 검증이라면 -> View영역에서 request를 받을 때 검증
- reservation 도메인에 대한 검증이라면 -> reservation에서 이루어지면 될 것 같아요.
그리고 reservation 객체로만 이루어질 수 없는 검증(ex. DB와의 협력을 통한 검증... 중복 저장 여부 등)이 필요하다면,
ReservationService에서 이루어지거나, ReservationValidator로 분리하여 Service가 Validator를 의존하도록 구성 할 수 있을 것 같아요.
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.
검증할게 많아질 수 있다고 생각해서 static class로 빼두었던건데 레이어드 계층에서는 의존성 주입 불가, 테스트 불가, 계층간 흐름 무시 등 다양한 단점이 존재하네요. 검증하는 로직은 dto나 entity에 두도록 하겠습니다!
|
리뷰 정말 감사합니다! 사실 그동안은 Entity를 외부로 직접 노출하지 않기 위해 Controller 단에서만 DTO를 만들어 사용하는 정도였고, 예외 처리도 리뷰 내용에 맞춰 구현을 했습니다. 그런데 테스트를 하면서 알게 된 점은, 그래서 지금은 예외 핸들링을 전역 핸들러 하나로 통합해서, 말씀해주신 피드백들을 최대한 반영해보았는데, 혹시 제가 잘못 이해하고 구현한 부분이 있거나 |
|
리팩토링하면서 생긴 궁금증도 함께 공유드리고 싶어요. 1.DTO 네이밍 관련 고민 자료를 찾아봐도 특별한 정답은 없던데,
처음엔 편의상 Entity에 메서드를 넣었다가, 그래서 저는 변환 책임을 Mapper 클래스에 위임해서 entityToDto() 같은 메서드를 따로 만들었고, 다만 이 방식대로 가다 보면 Mapper가 점점 많아지고,
저는 지금까지 혹시 모를 상황을 대비해 Entity 쪽에서도 한 번 더 검증하는 방식으로 구현을 해왔습니다. |
TaeyeonRoyce
left a comment
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 활용, 예외 커버리지 및 활용/구조 역시 의도대로 파악하신 것 같아요.
잘 작성해주셨고, 변경된 코드에 대한 몇몇 코멘트도 추가적으로 남겼습니다.
한 번만 더 확인해주세요..ㅎㅎ
남겨주신 몇몇 궁금증에 대해서 코멘트에 추가해 두었어요.
한가지 궁금증에 대해서만 여기에 답변 드리겠습니다.
DTO와 Entity 간 검증 책임에 대한 고민
DTO에서 값에 대한 검증이 이루어졌더라도, Domain에서 당연히 이루어져야 합니다..!
외부 요청을 받을 때 수행한 검증은 Domain 코드의 안전성을 보장 해주지 못해요.
"name에 빈 값을 입력 할 수 없다" 라는 정책은 Domain에서 역시 이루어지고 설명 되어야 해요.
Reservation이라는 객체의 특징/정책은 되도록이면 해당 객체가 책임지고 있어야죠!
외부요청이 아니라 test 코드에서 Reservation을 사용 할 때도, 라이브러리로 제공 할 때도, 다른 도메인과 협력 할 때도 "name에 빈 값을 입력 할 수 없다" 라는 정책은 보장 되어야 하니깐요.
검증 구현 순서가 바뀌었어요!
도메인에서 먼저 검증 로직을 만들고, 그걸 보고 외부에서 fail-fast 할 수 있는 영역을 추가 하는게 좋죠.
관련하여 예전에 제가 남긴 리뷰가 있는데 한번 참고 해보세요!
next-step/java-racingcar-simple-playground#114 (comment)
| public ReservationRequest(String name, LocalDate date, LocalTime time) { | ||
| if (name == null || name.isBlank()) { | ||
| throw new IllegalArgumentException("이름은 필수입니다."); | ||
| } | ||
| if (date == null) { | ||
| throw new IllegalArgumentException("날짜는 필수입니다."); | ||
| } | ||
| if (time == null) { | ||
| throw new IllegalArgumentException("시간은 필수입니다."); | ||
| } |
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로 들어오는 요청 데이터를 검증 하는 로직이 추가 되었네요.
간단한 퀴즈 하나 내보겠습니다..!
{
"date": "2025-08-05",
"name": null,
"time": "15:40"
}이런 요청이 들어 왔을 때, ExceptionHandler에서는 어떤 예외를 catch해서 처리할까요?
한번 예외 케이스의 요청으로 보내고 응답하는 예외 메세지를 확인 해보세요!
기본적으로 jackson 라이브러리를 통해서 json 형태의 텍스트가 저희가 정의한 객체로 만들어지게 됩니다.
해당 과정에서 석준님이 정의한 예외(IllegalArgumentException)이 전환 되는 것 같아요.
퀴즈와는 조금 먼 내용이지만,
json 요청을 기반으로 객체가 생성 되는 과정 혹은 방식을 살펴보는 것도 좋은 지식이 될 수 있어요.
참고 하면 좋을 자료
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.
IllegalArgumentException으로 처리될 줄 알았는데, 실제로는 HttpMessageNotReadableException으로 처리되더라고요.
왜 그런지 궁금해서 디버깅을 해보니, AbstractJackson2HttpMessageConverter 내부에서
IllegalArgumentException을 감싸 HttpMessageNotReadableException으로 변환해 던지네요.
현재는 HttpMessageNotReadableException이 발생할 경우
"형식이 옳바르지 않습니다"라는 고정된 메시지를 반환하고 있는데,
실제 예외의 원인을 확인할 수 있도록 e.getMessage()를 그대로 활용하는 것이 더 좋은 방법 같아요.
나중에 로그를 남길 때도 이 부분에 대해서 생각을 하며 로그를 남기면
문제 원인을 파악하는 데 훨씬 도움이 되겠다는 생각이 드네요. 😊
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.
꼼꼼하게 트래킹 해보셨군요..!👍👍
원인 분석까지 잘 파악하셨네요.
이처럼 의도치 않게 작동하는 로직이 존재 할 수 있으니, 추후 테스트를 더 보강 하여 이런 문제를 조기에 발견 할 수 있음 좋겠네요ㅎㅎ
| public Reservation(String name, LocalDate date, LocalTime time) { | ||
| validateDate(date); | ||
| this.id = null; | ||
| this.name = name; | ||
| this.date = date; | ||
| this.time = time; | ||
| } |
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.
생성 단계에서 검증이 이루어지네요.
현재 Reservation에는 2개의 생성자가 있는데...
날짜에 대한 정책은 해당 생성자를 통해서 만드는 경우에만 지켜질 것 같아요.
validateDate()를 주 생성자로 옮기고 this()를 통해 재사용 할 수도 있어요.
| public Reservation(String name, LocalDate date, LocalTime time) { | |
| validateDate(date); | |
| this.id = null; | |
| this.name = name; | |
| this.date = date; | |
| this.time = time; | |
| } | |
| // 주 생성자 | |
| public Reservation(Long id, String name, LocalDate date, LocalTime time) { | |
| validateDate(date); | |
| this.id = id; | |
| this.name = name; | |
| this.date = date; | |
| this.time = time; | |
| } | |
| // 부 생성자 | |
| public Reservation(String name, LocalDate date, LocalTime time) { | |
| this(null, name, date, time); | |
| } |
| @ExceptionHandler(Exception.class) | ||
| public ResponseEntity<ErrorResult> handleUnknownException(Exception e) { | ||
| ErrorResult errorResult = new ErrorResult("INTERNAL_SERVER_ERROR", "예상치 못한 오류 발생"); | ||
| return new ResponseEntity<>(errorResult, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } |
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.
예상치 못한 예외에 대해서도 핸들러를 추가했네요 👍
좋습니다.
한가지 추가 하면 좋은점은, 어떤 예외인지 확인할 방법이 없을 것 같아요.
지금은 반영하지 않아도 괜찮지만, 추후 로그를 남겨 어떤 예상치 못한 예외가 발생했는지 개발자가 파악 할 수 있도록 하면 좋겠네요!
| import roomescape.service.dto.SaveReservationCommand; | ||
|
|
||
| @Component | ||
| public class ReservationMapper { |
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.
타입간의 변환을 위해 Mapper를 도입하는 경우도 많아요.
질문으로 남겨주신 대로, Mapper의 도입은 번거롭기도 하고 불필요한 설계일 수 있습니다.
제가 현재 규모의 프로젝트에서 Dto-Entity간의 변환을 고민한다면, 아래처럼 할 것 같아요.
- [ReservationRequest -> SaveReservationCommand]
해당 케이스를 다루는 controller내 핸들러에서 변환 작업 orReservationRequest에toCommand()메서드 추가
- [SaveReservationCommand -> Reservation]
ReservationService.save() 메서드에서 변환 작업 orSaveReservationCommand에toCommand()메서드 추가
- [Reservation -> ReservationResult]
ReservationService.save() 메서드에서 변환 작업 orReservationResult에 Reservation 타입을 인자로 받는 생성자 추가 (fromReservation()이라는 정적 팩토리 메서드도 괜찮아 보여요)
- [ReservationResult -> ReservationResponse]
해당 케이스를 다루는 controller내 핸들러에서 변환 작업
몇가지 방식들을 제안 드렸어요. 현재 프로젝트에선 아무거나 선택해도 문제는 없어 보입니다.
그래도, 모든 방식 mapper를 필요로 하지 않고 각 객체가 책임을 갖고 변환 작업을 수행 합니다.
mapper를 제거하는 방식으로 개선할 수 있을 것 같아요!
그리고 남겨주신 답변처럼 Domain -> Dto를 의존하는 방향은 좋지 않아요.
dto는 언제든지 쉽게 바뀌거나 없어질 수 있어요.
프로젝트의 기준인 Domain이 코어가 되는 구조가 제일 좋습니다.
다른 계층들의 의존성을 도메인으로 몰아 프로젝트의 기준(코어)이 되고, 도메인은 최대한 다른 곳의 의존성을 줄이는 거죠!
| import java.time.LocalDate; | ||
| import java.time.LocalTime; | ||
|
|
||
| public record SaveReservationCommand( |
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.
네이밍에 대한 질문에 대한 답변도 같이 드릴게요..!
말씀대로 정답은 없어요.
웹 애플리케이션 기준으로 제가 사용하는 네이밍 방식은, 석준님이 하신것과 동일해요.
[예약 저장]이라는 유즈케이스(usecase)에 대해 살펴 보면,
Web ---> Controller: `SaveReservationRequest`
Controller ---> Service(app): `SaveReservationCommand`
Controller <--- Service(app): `SaveReservationResult`
Web <--- Controller: `SaveReservationResponse`
이렇습니다.
네이밍은 자유롭게 지어도 괜찮아요. 고민해보고 더 좋은 이름이 있다면 언제든지 사용하셔도 상관없죠!
다만, 일관성은 중요한거 같아요! 네이밍만 보고 어떤 계층에서 어떤 의도로 활용 되는지 파악 할 수 있도록요.
위 예시에서 접미사를 통해 어느계층에서 사용 되는 DTO인지 파악 할 수 있는 것 처럼요!
TaeyeonRoyce
left a comment
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.
Spring 첫 미션 수행 하시느라 고생 많으셨습니다!
충분한 역량 뿐만 아니라 남겨둔 리뷰에 대한 답변, 반영 그리고 스스로 고민한 의문점들까지..
리뷰어인 저에게도 재밌고 보람찬 경험을 주셨네요.
앞으로도 남은 그리디의 스터디/미션들 잘 수행 하시길 바랍니다!
수고 많으셨어요👍
| return time; | ||
| } | ||
|
|
||
| private static void validate(String name, LocalDate date, LocalTime time) { |
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.
static일 필요는 없겠어요!
| private static void validate(String name, LocalDate date, LocalTime time) { | |
| private void validate(String name, LocalDate date, LocalTime time) { |

안녕하세요! 충돌을 해결하느라 조금 애를 먹었네요ㅎㅎ 이번 미션도 잘 부탁드립니다!
이번 미션에서는 기존 코드의 구조를 조금 수정했어요. API와 관련된 Controller를 별도로 분리해 관리하고, Service 계층도 따로 나눠 레이어드 아키텍처 형태로 정리해보았습니다. 또, 반환에 필요한 DTO들도 새롭게 만들어 응답 구조를 좀 더 명확하게 구성했습니다.
이번 미션에서는 예외 처리 부분을 거의 처음 접하게 되어, 관련 내용을 공부하고 적용하는 데 시간이 꽤 걸렸습니다. 예외 처리를 처음 다뤄보는 만큼, 각 클래스와 애노테이션을 상황에 맞게 잘 활용했는지 궁금합니다.
예를 들어, 현재는 ResponseEntity를 사용해 상태 코드를 함께 반환하고 있는데, 예외 객체를 반환하며 @ResponseStatus 애노테이션을 붙여 처리하는 방식도 있다는 것을 알게 되었습니다. 이러한 방식들 중 어떤 것이 더 적절한지 혹은 상황에 따라 어떤 기준으로 선택해야 하는지에 대한 피드백을 받고 싶습니다.
또한 컨트롤러와 서비스의 역할 분리에 대해서도 고민이 많았습니다. 현재 DTO를 객체로 변환하는 로직은 서비스에서 처리하고 있고, DTO 내부에서 엔티티로 변환하는 메서드를 정의해 사용하고 있는데요. 이러한 구조가 적절한지, 혹은 개선할 여지가 있는지도 검토 부탁드립니다.
아직 부족한 점이 많지만, 최선을 다해 구현해보았습니다. 부담 없이 편하게 리뷰해주시면 감사하겠습니다! 😊