-
Notifications
You must be signed in to change notification settings - Fork 170
[Spring MVC] 민지인 미션제출합니다 #506
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
base: jxxxxxn
Are you sure you want to change the base?
Conversation
juanxiu
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.
고생하셨습니다~!!!! 리뷰 반영해서 푸시 부탁드려요!
| ReservationReq newReservation =new ReservationReq(index.incrementAndGet(),reservation.getName(),reservation.getDate(),reservation.getTime()); | ||
| reservations.add(newReservation); | ||
|
|
||
| // ResponseEntity 사용하면 header 명시적으로 지정하지 않아도 된다고 한다. |
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.
@ResponseStatus 를 사용하고 있군요. ResponseEntity 와 차이점을 생각해볼까요? 리턴값을 일관된 구조로 클라에게 전달하기 위해 둘 중 어느 것을 사용해야 할까요?
이 글 을 참고해보면 좋을 것 같아요!
| @ResponseBody | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public ReservationReq addReservation(@RequestBody ReservationAddReq reservation, HttpServletResponse response){ | ||
| if(reservation.getName().isEmpty()||reservation.getDate().isEmpty()||reservation.getTime().isEmpty()){ |
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에 Valid 어노테이션을 추가하면 수동으로 검증 코드를 작성하지 않아도 됩니다. DTO 에 정의된 필드가 Controller 에서 ResquestBody 에 들어올 때 검증해주어요. 자세한 내용은 이 글을 참고해볼까요? 그리고 이 어노테이션은 검증 실패 시 어떤 에러를 반환하는지 알아보고 리팩터링 해볼까요?
| public class ExceptionHandlers { | ||
|
|
||
| @ExceptionHandler(NotFoundReservationException.class) | ||
| public ResponseEntity NotFoundReservationException() { //딱히 메시지에 id 등 추가할 필요를 못 느껴서 매개변수 없앰. |
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.
메서드명과 예외클래스명을 일치시키면 혼동이 발생할 수 있어요. 메서드명을 변경해볼까요?
|
|
||
| @ExceptionHandler(NotFoundReservationException.class) | ||
| public ResponseEntity NotFoundReservationException() { //딱히 메시지에 id 등 추가할 필요를 못 느껴서 매개변수 없앰. | ||
| return ResponseEntity.badRequest().build(); |
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.
현재는 상태코드만 리턴하고 있네요. 예외처리를 하는 이유는 클라에게 오류 정보를 전달하기 위해서예요. 오류 메시지를 담는 적절한 객체를 만들어 전달하면 더 좋을 것 같네요!
|
|
||
| @ExceptionHandler(NotFoundReservationException.class) | ||
| public ResponseEntity NotFoundReservationException() { //딱히 메시지에 id 등 추가할 필요를 못 느껴서 매개변수 없앰. | ||
| return ResponseEntity.badRequest().build(); |
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.
NotFound 예외는 404 상태코드로 전달하는 게 좋을 것 같아요! 현재는 Bad request 400 코드로 처리하고 있네요!
[[Fix] dto 멤버변수 private으로 변경했을 시 오류 수정 (@Getter 사용), @AllArgsConstructor 추가
해당 커밋부터 step34 과제입니다.
감사합니다.