[Spring Core] 오채현 미션 제출합니다. #69
Conversation
There was a problem hiding this comment.
리뷰이님 미션 진행하느라 고생많으셨습니다 😄
롬복을 적절히 활용하셔서 코드가 좀 더 깔끔해보였습니다.
9주차 요구사항도 만족하는 것 같습니다!
브랜치 이름에 대한 저의 생각
리뷰어의 입장에서 생각했을 때는 브랜치의 이름이 main3이라는 것이 정확히 어떠한 커밋이 담겨있는지 유추하기 어려울 수 있을 것 같아요.
어떠한 작업을 했는지 알 수 있도록 미션 브랜치의 이름을 조금 더 구체적으로 지어주신다면 리뷰하는데에 도움이 될 것 같아요. 혹시 main3이라는 것이 어떠한 의미가 담겨있는거라면 말씀해주세요!
남은 해커톤 일정에서 좋은 결과 있길 바랍니다~
| @@ -0,0 +1,33 @@ | |||
| package roomescape.Waiting; | |||
There was a problem hiding this comment.
Waiting 패키지만 유일하게 이름이 대문자로 시작하는 것 같아요!
There was a problem hiding this comment.
DataLoader와 TestDataLoader는 스프링 부트가 실행된 이후로 필요한 데이터를 저장하는 역할을 하는 공통점을 갖고 있습니다. 두 클래스를 하나의 패키지로 묶어서 이들의 역할을 나타내보는건 어떨까요?
|
|
||
| public WaitingResponse save(LoginMember loginMember, WaitingRequest waitingRequest) { | ||
|
|
||
| Member member = memberRepository.findById(loginMember.getId()).get(); |
There was a problem hiding this comment.
| Member member = memberRepository.findById(loginMember.getId()).get(); | |
| Member member = memberRepository.findById(loginMember.getId()) | |
| .orElseThrow(() -> new IllegalArgumentException("회원이 존재하지 않습니다.")); |
Optional 객체를 바로 꺼내올 경우, 인텔리제이에서 값의 존재 여부를 확인하지 않고 가져오는 것에 대한 알림 창이 뜨는걸 알 수 있습니다. 반환받은 Optional이 비어있을 경우를 대비해 조건문을 추가할 수 있을 것 같으며 코드를 더욱 간소화하고 직관적으로 나타내기 위해서는 .orElseThrow()를 쓸 수 있을 것 같습니다.
스터디에서 언급했던 자료를 확인해보면 좋을 것 같아요!
https://www.daleseo.com/java8-optional-effective/
| } | ||
|
|
||
| public List<MyReservationResponse> findByMemberId(Long memberId){ | ||
| List<WaitingWithRank> waitings = waitingRepository.findWaitingsWithRankByMemberId(memberId); |
There was a problem hiding this comment.
해당 메서드에서 waitings는 사용되고 있지 않은 것 같아요! 기존에는 waitings가 사용되다가 코드를 리팩토링하는 과정에서 더이상 사용하지 않게 된걸까요?
| @RequiredArgsConstructor | ||
| public class AdminInterceptor implements HandlerInterceptor { | ||
|
|
||
| private final MemberService memberService; |
There was a problem hiding this comment.
해당 빈은 MemberService를 주입받고 있지만, 로직에서는 MemberService를 활용하고 있지 않는 것 같아요!
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
|
||
| @ResponseStatus(HttpStatus.CONFLICT) |
There was a problem hiding this comment.
커스텀 예외를 만들어서 예약 중복에 대한 예외가 발생한다는걸 바로 알 수 있을 것 같아요 👍
혹시 @ResponseStatus는 어떠한 용도로 사용하신건지 궁금합니다.
드디어 마지막 과제가 끝이 났네요! 정말 수고 많으셨습니다. 이제 배포랑 해커톤이 남았는데 끝까지 파이팅 입니다!!
*테스트는 다 통과 했습니다. 혹시 안 지켜진 요구사항이나, 수정이 필요한 부분이 보이면 말씀해주세요!
미션 커밋 범위