[10단계 - 콘솔 UI 지원] 리브(김민주) 미션 제출합니다. #176
Conversation
Minjoo522
changed the title
| return Optional.of(1); | ||
| } else { | ||
| return Optional.empty(); |
There was a problem hiding this comment.
Optional 잘 적용해주셨네요! 👍
그런데 이 부분은 제가 '이런 방식으로도 할 수 있다'라고만 남길 의도였는데
ExceptionHandler 와 함께 설명하면서 꼭 변경해보자는 뉘앙스로 리뷰를 달았군요... 죄송합니다 🙏
Optional 을 꼭 사용하지 않으셔도 괜찮습니다!
There was a problem hiding this comment.
감사합니다!
해피케이스라도 잘 다뤄보자는 생각에 Optional은 생각하지도 못했고 Optional을 알고는 있었지만 대체 언제 써야하나 생각했었는데 피케이 덕분에 적합한 사용 용도를 알게 되었습니다!
지금은 삭제시 사용하고 있지만 findById 조회시 사용하면 굉장히 유용할 것 같습니다!
그리고 ExceptionHandler를 사용하고 나니 코드가 훨씬 깔끔해졌고 예외 발생시 처리 방법을 한 눈에 볼 수 있어서 유용하다고 생각했습니다. 😆
|
|
||
| --- | ||
|
|
||
| ## 콘솔 UI |
| public class MemoryReservationTimeRepository implements ReservationTimeRepository { | ||
|
|
||
| private final Map<Long, ReservationTime> reservationTimes; | ||
| private final AtomicLong index; |
There was a problem hiding this comment.
class 시작 부분과 필드 사이 공백은 보통 안 넣어주시던데 여기는 들어가있네요!
There was a problem hiding this comment.
꼼꼼하게 봐주셔서 감사합니다! 👍
수정했습니다. 😆
| private final AtomicLong index; | ||
|
|
||
| public MemoryReservationTimeRepository() { | ||
| this.reservationTimes = new ConcurrentSkipListMap<>(); |
There was a problem hiding this comment.
오 ConcurrentSkipListMap 구현체를 사용해주셨군요!
이 선택을 하신 이유가 궁금합니다!
There was a problem hiding this comment.
먼저 concurrent는 동기화를 위해서 사용했습니다.
concurrentMap의 하위 구현체로 ConcurrentHashMap과 ConcurrentSkipListMap 중 하나를 고민 했습니다.
ConcurrentHashMap을 사용할 경우 키가 정렬되어 있지 않기 때문에 전체 조회 시 sorting을 해줘야 했습니다.
반면에 ConcurrentSkipListMap을 사용하면 키가 정렬되어 있어 조회 시 sorting 할 필요가 없습니다.
ConcurrentSkipListMap의 경우 ConcurrentHashMap보다 저장에 시간이 더 소요되지만 현재 기능 상 저장, 삭제보다 조회 기능을 더 많이 사용하기 때문에 ConcurrentSkipListMap을 선택했습니다. 😊
| System.out.println("[예약 시간 추가]"); | ||
| System.out.println("추가 할 예약 시간을 입력해주세요.(ex. 14:00)"); |
There was a problem hiding this comment.
readSelectedMenu 에서도 multi line string 을 사용해주셨으니,
이렇게 연속적으로 출력되는 메시지는 비슷한 형태로 바꿔주는 것은 어떨까요?
| time_id BIGINT, | ||
| time_id BIGINT NOT NULL, |
| System.out.println("예약이 없습니다."); | ||
| System.out.println(); |
There was a problem hiding this comment.
System.lineSeparator() 을 사용해봐도 좋을 것 같아요 :)
| ); | ||
| }), | ||
| dynamicTest("저장된 예약을 모두 조회한다.", () -> { | ||
| assertThat(reservationService.findReservations()).hasSize(1); |
There was a problem hiding this comment.
이건 그냥 참고용으로 봐주세요 :
저는 '목록 조회' 를 테스트할 때는 최소한 두 개의 객체를 저장하고 테스트하는 편이에요.
아무래도 '목록' 을 조회하는 것이다보니, 단일 조회라는 차이를 두려고 하는거죠!
|
안녕하세요, 피케이! 10 단계까지 할 수 있을 거라고 생각을 못했는데 피케이의 리뷰 덕분에 잘 올 수 있었던 것 같습니다. 👍 즐거운 한 주 되세요. 😊 |
안녕하세요, 피케이!
리브입니다. 😆
10 단계 구현 완료하여 PR 드립니다.
콘솔 UI 관련 설명은
README.md하단에서 확인 가능합니다.이전 PR에 남겨주셨던 리뷰 관련하여 반영한 내용은 다음과 같습니다.
Optional을 반환하고Service레이어에서 처리하도록 했습니다.ExceptionHandler를 추가했습니다.ReservationService에서ReservationTimeRepository를 주입 받아timeId에 해당하는ReservationTime을 조회한 후ReservationRepository에 전달했습니다.리뷰 잘 부탁 드립니다.
즐거운 주말 되세요 😊