[Spring MVC] 김시영 미션 제출합니다. #464
Conversation
| private String date; | ||
| private String time; | ||
|
|
||
| public Reservation() { |
There was a problem hiding this comment.
아무것도 안받고 사용하지 않는 Reservation이라는 생성자가 있군요.
만약 요렇게 생성된 객체는 실제로 유효한 Reservation일까요?
또 사용하지 않는 메서드를 미리 만들어 놓는건 괜찮을까요?
| } | ||
|
|
||
| private void validate(String name, String date, String time){ | ||
| if (name == null || name.isEmpty() || |
There was a problem hiding this comment.
Reservation은 일종의 도메인적 성격을 띄고 있다고 생각해요.
요런 부분은 입력에 대한 검증부분인데 어떻게 해결해볼 수 있을까요?
There was a problem hiding this comment.
요렇게 특정 값이 null인지 비어있는지 확인하는 로직들은 굉장히 많습니다.
이런 중복된 로직을 제거하기 위해서 @Valid와 @Validation이라는 어노테이션이 있는데요.
이 어노테이션과 DTO를 조합하면 정말 효과적으로 사용할 수 있습니다. 이에 대해서도 찾아보고 적용해보면 좋을 것 같아요
| import roomescape.exception.InvalidReservationException; | ||
|
|
||
| public class Reservation { | ||
| private long id; |
There was a problem hiding this comment.
요거는 한번 이야기했어야 했는데 조금 늦었군요.
혹시 자바의 final 키워드에 대해 아시나요? 변수에 대해서 reassign 을 막는 키워드인데, 객체의 불변을 유지하기에 좋습니다.
이 키워드를 이용해서 우리는 immutable하게 값을 사용할 수 있습니다. 최근 개발 패러다임에선 이런 immutable한 값을 사용하는게 선호되고 있는데 왜 그런걸까요?
| } | ||
|
|
||
| public Reservation(long id, String name, String date, String time) { | ||
| validate(name, date, time); |
There was a problem hiding this comment.
요런 validate는 테스트 코드로 검증해보면 좋았을 것 같아요
| return ResponseEntity.noContent().build(); | ||
| } | ||
|
|
||
| @ExceptionHandler({NotFoundReservationException.class, InvalidReservationException.class}) |
There was a problem hiding this comment.
Controller안에서 ExceptionHandler를 쓸 수도 있지만 다른 방법도 있습니다.
한번 ControllerAdvice에 대해서 알아보고 적용해보면 좋을 것 같습니다!
| } | ||
|
|
||
| @PostMapping("/reservations") | ||
| public ResponseEntity<Reservation> addReservation(@RequestBody Reservation reservation) { |
There was a problem hiding this comment.
직전 미션 끝날떄쯤에 이야기했던 키워드 중에 dto라는 것이 있습니다.
domain을 view에서 직접 사용하지 않게 함으로써 변경에서 내성을 얻도록 하는 것입니다.
이렇게 RequestBody로 입력을 받는 것은 위험할 수 있습니다.
domain이 바뀌면 실제 http 요청 형식이 바뀌기도 하고, 심지어 응답 형태도 바뀔 수 있으니까요.
There was a problem hiding this comment.
한번 dto에 대해 찾아보고 적용해보면 좋을것 같아요.
너무 이해가 안간다 싶으면 상현님 코드를 봐도 조금 도움이 될 것 같습니다.
| } | ||
|
|
||
| private void validate(String name, String date, String time){ | ||
| if (name == null || name.isEmpty() || |
There was a problem hiding this comment.
요렇게 특정 값이 null인지 비어있는지 확인하는 로직들은 굉장히 많습니다.
이런 중복된 로직을 제거하기 위해서 @Valid와 @Validation이라는 어노테이션이 있는데요.
이 어노테이션과 DTO를 조합하면 정말 효과적으로 사용할 수 있습니다. 이에 대해서도 찾아보고 적용해보면 좋을 것 같아요
|
|
||
| @DeleteMapping("/reservations/{id}") | ||
| public ResponseEntity<Void> deleteReservation(@PathVariable long id) { | ||
| Reservation reservation = reservations.stream() |
There was a problem hiding this comment.
잘 짜셨습니다. 로직이 깔끔하군요.
하지만, 하나 더 개선해볼만한 포인트가 있을 것 같아요.
List 형식으로 된 collection에 객체를 저장하게 되면, 조건에 맞는 값을 조회할 떄 이렇게 O(n)만큼의 시간이 걸립니다.
존재하지 않는 경우엔 더 걸리죠.
요걸 O(1)안에 찾게하려면 어떻게 해야할까요?
| <!-- 로고 영역 --> | ||
| <div class="sidebar-logo p-4"> | ||
| <a href="/"> | ||
| <a href="/static"> |
There was a problem hiding this comment.
이걸 home.html에서 index.html로 이름을 바꾸고 별도의 controller를 안 만든건 인상적이네요
| public class ReservationController { | ||
|
|
||
| private final List<Reservation> reservations = new ArrayList<>(); | ||
| private final AtomicLong index = new AtomicLong(0); |
| } | ||
| } | ||
|
|
||
| public long getId() { |
There was a problem hiding this comment.
요런 반복적인 코드를 나중에도 작성을 많이 해야합니다. 이런것을 해결하기 위해 lombok이라는 라이브러리가 있는데 이것도 한번 찾아서 학습해보면 좋을 것 같아요.
요 lombok은 진짜 많이 씁니다.
|
또 전체적으로 상현님이 스프링에 대한 경험이 있는 것 같아서, 한번 제가 말한 요구사항들을 다 학습하고 반영하고 나서 상현님 코드를 봐보면 좋을 것 같습니다. |
이번 미션도 잘 부탁드립니다!
아직 스프링이 익숙하지 않아서 요구사항을 반영하는 데 집중했습니다.
혹시 개선할 점이 있다면 피드백 부탁드립니다!!
감사합니다.