[그리디] 이채현 JPA4-5단계 미션 제출합니다 - #266
Conversation
# Conflicts: # src/main/java/roomescape/login/LoginInterceptor.java # src/main/java/roomescape/login/LoginMemberArgumentResolver.java
There was a problem hiding this comment.
안녕하세요, 채현님! 오랜만에 다시 뵙네요 😊
코드 변경 사항에서 Dao → Repository로 일관되게 전환하고
엔티티에 @Entity, @Id 같은 어노테이션들이 적절히 추가한 구조가 가장 눈에 띄더라구요
학습 목표를 정확히 파악하신 것 같아 좋았습니다 ㅎㅎ
미션 진행하면서 추가적으로 어려운 점은 없으셨나요?
현재 상황을 구체적으로 공유해주실수록 리뷰 방향도 더 명확해질 수 있어서요!
이 부분 하나만 부탁드려도 될까요~~~??
EntityManager를 사용하면 JPA(Spring Data JPA)와 달리 어떤 점이 좋은지?
좋은 질문이에요! 사실 EntityManager도 JPA의 일부입니다!
JPA가 EntityManager를 사용하기 위해 추상화해둔 레벨을, 채현님이 직접 구현해보신 거예요!
다음 단계에서는 EntityManager로 직접 구현한 코드를 Spring Data JPA의 JpaRepository로 바꾸게 될 텐데요~
지금 직접 작성한 Repository 코드가 얼마나 줄어드는지 직접 비교해보시면 좋을 것 같아요!
| this.timeRepository=timeRepository; | ||
| } | ||
|
|
||
| public ReservationResponse save(ReservationRequest reservationRequest, LoginMember loginMember) { |
| @RestController | ||
| public class ThemeController { | ||
| private ThemeDao themeDao; | ||
| private final ThemeRepository timeRepository; |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
| private String value; | ||
| private String time_value; |
There was a problem hiding this comment.
Time 엔티티의 필드명이 Java 네이밍 컨벤션을 따르지 않네요
- Java 네이밍 컨벤션은 무엇일까요?
- 지금 사용한 컨벤션은 뭐로 불리울까요?
- 왜 컨벤션을 지켜야 하는 걸까요?
세가지 물음에 대해 각각의 답변을 달아주세요!
There was a problem hiding this comment.
1.네이밍 컨벤션이란 가독성을 높이고 유지보수를 용이하게 하기 위한 표준 명명 규칙입니다.
2. _는 상수 네이밍에서 대문자+ _와 함께 들어갑니다.
3. 컨벤션을 지켜야하는 이유는, 네이밍 컨벤션을 제대로 지킴으로써, 가독성을 향상 시키기 위함입니다.
| return cookies; | ||
| } | ||
|
|
||
| private String extractToken(Cookie[] cookies) { |
There was a problem hiding this comment.
extractToken() 메서드가 LoginInterceptor와 LoginMemberArgumentResolver에 중복되어 있어보여요!
- 중복되어 있으면 안될까요?
- 안되는 이유는 무엇일까요?
- 해결할 수 있는 방법을 나열해보세요
- 각 방법의 장단점을 나열해보세요
- 하나를 선택해보세요
- 선택한 이유를 작성해주세요
6가지 질문에 대한 답변을 작성해주세요~!
There was a problem hiding this comment.
1.중복되면 '안된다'라는 개념은 아닌 것 같습니다. 다만..
2. 중복되면, 유지보수에 있어서 어려움이 있을 것 같고, 이에 따른 버그 가능성이 올라갈 것 같습니다. 따라서 유지보수에 있어서 어려움이 있을 것 같습니다.
3~5
LoginMemberArgumentResolver에서는 LoginMember를 만들기 위해서 사용되었더라면,
| private String extractToken(Cookie[] cookies) { | |
| private String extractToken(Cookie[] cookies) { |
LoginInterceptor에서는 가로채는 과정에서 extractToken이 필요합니다. 두개에서 모두 제외하기 보다는 CookieManager를 만들어서 extracttoken을 구현하는 것이 맞다고 생각합니다.
6.
각 필요한 곳이 있다고 생각되었고, 그부분을 cookieManager에서 관리함으로써 편리하게 유지보수할 수있다고 생각되었습니다.
| throw new RuntimeException("Invalid email or password"); | ||
| } | ||
| Member member = memberRepository.findByEmailAndPassword(email, password) | ||
| .orElseThrow(); |
There was a problem hiding this comment.
orElseThrow()에 예외 메시지가 없네요
- 없으면 생기는 문제점은 뭐가 있을까요?
수정하게 된다면 예외 메세지가 없는 모든 코드에서 수정이 이루어지길 바래요~!
There was a problem hiding this comment.
제가 코드를 짤때는 member를 optional로 받도록 되어있습니다. 즉 null값이 존재할 수 있습니다. 그런데 이걸 orElseThrow가 없게된다면, null값일때 대처할 수 없게됩니다.
만약 제가 optional로 받지 않았더라면 orElseThrow를 없애는게 맞다고 생각합니다.
하지만, 제가 생각했을때, member를 전달하는 과정에서 오류가 생기면 그저 Member member를 썼을때는 대처할 수 없게된다고 생각되어서 optional를 사용해서 오류검증 orElseThrow가 있는 방식이 조금더 적합된다고 생각됩니다
There was a problem hiding this comment.
Optional로 처리해주신 부분 좋은 설계라고 생각해요!
findBy~라는 네이밍 자체가 "반환값이 있을 수도, 없을 수도 있다"는 걸 말해주니까요~
제가 말씀 드리고 싶은 부부능ㄴ orElseThrow() 괄호 안이 비어 있다는 것이었어요!
아래 코드와 같이 커스텀 예외나 메시지를 넣어줄 수도 있지 않을까 싶더라구요
.orElseThrow(() -> new MemberNotFoundException("적절한 멤버를 찾지 못했습니다"))지금처럼 인자 없이 orElseThrow()만 쓰면 후자가 주는 이점들을 놓치게 되지 않을까 싶은데
채현님은 어떻게 생각하시나요?
그리고 그 생각을 답변에 최대한 상세하게 작성해주세요!
There was a problem hiding this comment.
단순히 optional을 사용했을때, .orElseThrow()를 쓰면 오류가 나지 않는다는 면에서 orElseThrow를 사용하였습니다.
만약, orElseThrow를 이용해서 새로운 커스텀 에러 메세지를 넣는다면, 오류가 나는 지점을 메세지를 받음으로써 더 정확하고 쉽게 파악할 수 있다고 생각합니다!
커스텀 예외를 넣는 것이 더 좋다고 생각되네요!!
|
|
||
| public MemberResponse createMember(MemberRequest memberRequest) { | ||
| Member member = memberDao.save(new Member(memberRequest.getName(), memberRequest.getEmail(), memberRequest.getPassword(), "USER")); | ||
| Member member = |
There was a problem hiding this comment.
POST /members를 호출해 보셨나요? 회원가입이 이루어지지 않습니다. 이유는 어떤 것일까요?
There was a problem hiding this comment.
Dao를 제가 여전히 호출하고 있어서 그런 것 같습니다..!
Repository와 연결되어 있어야하는데, 이러한 부분에서 연결이 끊겨서 생기는 문제인 것 같습니다
. 다음 리뷰할때 그부분 참고해서 변경하도록 하겠습니다!
There was a problem hiding this comment.
한 번 호출해서 예외를 확인해보시면 좋을 것 같아요~!
- 어떤 예외 메시지가 발생하나요?
- Dao 호출 때문에 회원가입이 이루어지지 않는다고 판단하신 근거가 궁금해요!
- 수정 후 해결 방법도 함께 작성해주세요~~
이 부분은 다음 리뷰어분께 확인을 요청드리겠습니다!
cc.@c0mpuTurtle
| return reservation; | ||
| } | ||
|
|
||
| public Optional<Reservation> findById(Long id) { |
There was a problem hiding this comment.
현재 findAllByMemberId 하나를 사용할때 쿼리가 몇개 나가나요?
There was a problem hiding this comment.
N+1개가 나갈 것 같습니다.
사실 이 부분을 스터디때도 했는데, 쿼리가 몇개나가는지 아직 많이 헷갈리는 것 같습니다. 이부분은 따로 더 공부해보겠습니다
| @RestController | ||
| public class ThemeController { | ||
| private ThemeDao themeDao; | ||
| private final ThemeRepository themeRepository; |
There was a problem hiding this comment.
ThemeController가 ThemeRepository를 직접 사용하고 있네요
다른 도메인은 모두 Service 계층을 거치는데,
Theme만 Controller → Repository로 직접 접근하는 이유가 있나요?? 어떠한 문제점이 있을 수 있을까요??
There was a problem hiding this comment.
오 controller에서 Repository로 바로 직행하게 되면 데이터 db상의 문제가 생길 수도 있다고 생각합니다.
다음 피드백 시에 고쳐야 될 것 같네요..!
2 participants
🙋♂️인사🙋♂️
안녕하세요. 세종대학교 그리디 백엔드 4기 이채현입니다.
Spring JPA 4-5단계
entity매니저를 사용해서 repository 수정
jpa 전환 및 내 예약 구성코드를 생성함.
고민한 내용 🤔
entity 객체가 대신해주니까 그 부분이 이해가 살짝 안되고 어려웠습니다.
느낀지 그 부분이 궁금해요!