[Spring Core] 정민주 미션 제출합니다. #416
Conversation
정상적인 직렬화/역직렬화를 위해 Time 객체에 기본생성자 추가
| private final ReservationRepository reservationRepository; | ||
| private final TimeRepository timeRepository; |
There was a problem hiding this comment.
db에 데이터를 조회하고 저장하고 삭제하는 역할을 하는 객체의 명칭을 000Repository라고 네이밍을 했는데
한번 DAO와 Repository를 비교해보면서 어떤 네이밍이 더 적절할 지 한번 고민해보면 좋을 것 같습니다!
There was a problem hiding this comment.
전혀 생각못했던 부분인데 ! 요 경우엔 dao가 맞는 것 같네요! db기준으로 클래스를 분리했기도 하고 .. 맞춰서 변경하도록 하겠습니다 :)
공부할 때 참조한 블로그는 https://ttl-blog.tistory.com/1285 요거 입니다!
| return reservationRepository.findAll().stream() | ||
| .map(reservationOutDto -> { | ||
| Time time = findTimeById(reservationOutDto.getTimeId()); | ||
| return Reservation.of(reservationOutDto, time); | ||
| }) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
현재 해당 로직을 보면 reservation의 개수가 늘어나는 만큼 time을 불러오기 위한 조회 쿼리도 계속 늘어나는 것 같습니다. (5개의 reservation이 조회되면 5개의 time 조회를 위한 쿼리가 추가로 발생)
처음부터 reservation을 불러올 때 time 정보도 한번에 불러올 수 없을까요?
There was a problem hiding this comment.
join하는 쿼리를 써놓고도 제대로 활용하지 못했네요 ;__; N+1은 전혀 생각도 못하고 있었습니다
RepositoryDto 삭제하고 기존 Repository 활용하도록 수정했는데 혹시 관련해서 문제 생길 부분같은게 있다면 코멘트 부탁드리겠습니다!!!
| public List<Time> findAllTimes() { | ||
| return timeRepository.findAll(); | ||
| } | ||
|
|
||
| public Time saveTime(TimeInDto timeInDto) { | ||
| return timeRepository.save(timeInDto); | ||
| } |
There was a problem hiding this comment.
time을 저장하고 불러오는 것은 ReservationService의 책임일까요? 아니면 따로 처리해야할 새로운 service가 필요할까요?
There was a problem hiding this comment.
예약 자체가 시간 value를 품고 있어서..! 굳이 TimeService를 새로 팔 필욘없다고 생각했습니다!
| public Time(Long id, String value) { | ||
| this.id = id; | ||
| this.time = parseTime(value); | ||
| } |
There was a problem hiding this comment.
현재 해당 생성자에서는 value에 대한 검증이 이루어지지 않는 것 같습니다
|
|
||
| @RestController | ||
| @RequestMapping("/reservations") |
There was a problem hiding this comment.
이전 단계에서 적용하신 @Transactional controller에 있는게 적절할까요? 아니면 새로 생긴 service layer에 있는 것이 적절할까요? 한번 고민해주시고 답해주세요!
There was a problem hiding this comment.
음 get의 경우 굳이 Transactional을 안붙여도 되니, 필요한 메서드(POST, DELETE)에만 명시해주는 것이 더 좋은 구현방식일 것이라 생각했습니다!
| @Transactional | ||
| @PostMapping | ||
| public ResponseEntity<Time> createTime(@RequestBody TimeInDto timeInDto) { | ||
| final Time time = reservationService.saveTime(timeInDto); | ||
| URI location = URI.create("/times/" + time.getId()); | ||
| return ResponseEntity.created(location).body(time); | ||
| } | ||
|
|
||
| @Transactional | ||
| @DeleteMapping("/{id}") | ||
| public ResponseEntity<Void> deleteTime(@PathVariable Long id) { | ||
| reservationService.deleteTimeById(id); | ||
| return ResponseEntity.noContent().build(); | ||
| } |
| @@ -0,0 +1,26 @@ | |||
| package roomescape.entity.Dto; | |||
|
|
|||
| public class ReservationInDto { | |||
There was a problem hiding this comment.
해당 dto는 현재 reservation을 생성할 때 이용하는 dto인 것 같습니다. 해당 명칭이 와닿지 않는데 ReservationCreateDto와 같이 의미를 한번에 나타내주는 네이밍으로 관리하는 것은 어떤가요?
|
|
||
| private String name; | ||
| private String date; | ||
| private Long time; |
There was a problem hiding this comment.
해당 필드도 time의 값이 아닌 id 정보를 받아오는 것이기에 필드 명칭으로 timeId로 관리하는 것은 어떤가요?
There was a problem hiding this comment.
아 화면단에서 요청이 넘어올때 key값이 'time'으로 넘어와서요!!
| String sql = "SELECT \n" + | ||
| " r.id as reservation_id, \n" + | ||
| " r.name, \n" + | ||
| " r.date, \n" + | ||
| " t.id as time_id, \n" + | ||
| " t.time as time_value \n" + | ||
| "FROM reservation as r inner join time as t on r.time_id = t.id\n"; |
There was a problem hiding this comment.
| String sql = "SELECT \n" + | |
| " r.id as reservation_id, \n" + | |
| " r.name, \n" + | |
| " r.date, \n" + | |
| " t.id as time_id, \n" + | |
| " t.time as time_value \n" + | |
| "FROM reservation as r inner join time as t on r.time_id = t.id\n"; | |
| String sql = """ | |
| SELECT | |
| r.id AS reservation_id, | |
| r.name, | |
| r.date, | |
| t.id AS time_id, | |
| t.time AS time_value | |
| FROM reservation AS r | |
| INNER JOIN time AS t ON r.time_id = t.id | |
| """; | |
아래 같이 수정하면 가독성이 조금 더 좋아질 것 같습니다.
내용