-
Notifications
You must be signed in to change notification settings - Fork 76
[Spring Data JPA] 김진학 미션 제출합니다. #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: iampingu99
Are you sure you want to change the base?
Conversation
예약 생성 기능 수정
- 요청 객체 생성 로직 분리 - 요청 로직 분리
- 예약이 존재하지 않는 경우 예약을 생성할 수 없다 - 본인의 예약이 존재하는 경우 예약을 생성할 수 없다
optional 인 name 필드는 service 에서 확인하도록 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
진학님 이번에도 코드 잘 작성해주셨네요. 👍
추가로, 질문들에 대한 답변입니다.
두가지 이상의 도메인의 비즈니스로직이 결합된 기능에 대한 고민
우선, Reseravtion 도메인에 Status 를 추가해 Waiting 을 없앤다면 지금 로직에서 어떻게 달라질지 상상해서 알려줄 수 있나요? ( 엔티티적인 변화 또는 저장할때나 조회할때 어떻게 바뀌는지 )
그 뒤, 저의 의견을 말하겠습니다. 🙂
예약 목록 조회: Reservation list 에 Waiting list 도 포함되는 로직
두가지 도메인의 로직이 결합된 해당 메서드는 어디에 두는게 좋을까요?
그리고, 당장에는 이게 두가지 도메인 로직이라 할 만큼 분리가 되어야 할 도메인일지 모르겠습니다.
대기라는 엔티티와 상태는 예약이라는 도메인과 로직이 없으면 존재할 수 있나요?
물론, 극단적인 관점이긴 하지만 제 기준은 이렇습니다.
차후에는 예약이 정말 큰 의미를 가진다면
( 이미 예약이 다 끝났는데, 추가 표를 위해 대기가 필요하다 or
대기가 단순 방탈출 예약이 아닌, 테마파크 등 다른 도메인에서도 공통된 예약이 필요해진다 )
분리가 되야하지만, 지금의 예약 - 대기
관점에서 도메인이 분리되기 보단 예약
이라는 도메인 내 객체인거 같습니다.
테스트 시에 초기 데이터(data.sql) 의존에 대한 고민
해당 부분은 팀원에게 학습 및 인지시키는 노력
이 매번 코드에서 delete 하거나 insert 하는 것보다 더 좋다는 의견입니다. 물론, 이는 팀 의견이나 컨벤션일거 같아요.
우리 테스트는 해당 어노테이션을 붙여 매번 특정 데이터가 INSERT 되고, 매번 삭제되고 있어.
우리는 매번 삭제만 하고, 데이터는 직접 넣어서 다 테스트 해야해
와 같은 의견은 다양할 수 있습니다.
그리고, 이는 당장 첫번째 테스트에서 결정하기 보단
로직을 확장하고 계속 테스트를 작성하다보면 깨지지 않고 반복적인 데이터 패턴들이 보일 수 있습니다.
그러면, 그때 context.sql
과 같이 공통적인 데이터를 관리하는 스크립트를 만들면 될 거 같아요 👍
API 요청 Optional 필드 구현에 대한 고민
이부분도 사실, 어드민을 분리하는게 가장 좋다는 의의긴 한데 이게 더 복잡함을 주고 미래를 예상하기 어렵다면 서비스 단에서 처리를 하는게 더 좋은거 같아요.
핵심은, 그 순간 가장 좋고 깔끔한 코드를 짜는것 입니다. ( 레거시도 작성한 당시에는 최고의 사연있는 코드였을겁니다. )
Time time = timeRepository.findByIdOrThrow(reservationRequest.time()); | ||
Theme theme = themeRepository.findByIdOrThrow(reservationRequest.theme()); | ||
|
||
Reservation reservation = new Reservation( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분에서 Option.ofNullable 부분을 private method 로 빼도 좋을거 같네요.
비즈니스 로직의 일환이라 어쩔수 없지만, 새로운 팀원이 들어오고 이 로직을 보면 혼동을 느낄거 같네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조건식을 메서드로 추출하면 네이밍을 통해 조건의 의미를 쉽게 파악할 수 있는 것처럼, 해당 동작도 private method 로 분리하는 것이 동작의 의미를 명확하게 전달할 수 있을 것 같습니다. 수정해보겠습니다 👍
Theme theme = themeRepository.findByIdOrThrow(reservationRequest.getTime()); | ||
Reservation reservation = new Reservation(reservationRequest.getName(), reservationRequest.getDate(), time, | ||
theme); | ||
public List<MyReservationResponse> readAllByMember(String email) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 코드도 아래와 비슷하게 정답은 아니지만
public List<MyReservationResponse> readAllByMember(final String email) {
final Member member = memberRepository.findByEmailOrThrow(email);
final var reservations = getMemberReservation(member);
final var waitingWithRanks = getMemberWaiting(member);
return List.copyOf(Stream.concat(
reservations.stream().map(MyReservationResponse::from),
waitingWithRanks.stream().map(MyReservationResponse::from))
.toList());
}
이와같으면 좀 더 깔끔 해질거 같네요.
getXXX
와 같은 메소드는 재사용이 용이 + DTO 변환 로직과 분리, 중복 코드 제거
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지난번 리뷰에서 알려주신 레이어 분리 방식이 너무 마음에 들었습니다. 공유해주신 지속 성장 가능한 소프트웨어를 만들어가는 방법 글을 읽으며, 레이어를 분리하는 것만으로 해결하려다 보니 메서드 분리를 잊고 있었네요. 수정해보겠습니다. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 해당 아티클을 읽으며 영향을 받았는데
다양하게 코드를 작성하며 관점을 가져나가시면 될 거 같아요.
- 명확하게 분리해야 할 필요 & 확장될 가능성이 있는가?
- 스프링 기술적으로 ( private 메소드는 트랜잭션 불가 ) 또는 기술적으로 ( 클래스 재사용 등 ) 분리되야 하는가?
무조건 레이어 기반 클래스들을 계속 만들어 분리하다보니 1:1 과 같은 형태가 될 수도 있더라구요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순히 메서드 호출을 위임하는 형태의 분리보다는, 현재 요구사항을 명확하게 만족하는 코드가 더 중요하다는 관점을 갖게 되었습니다. 하지만 일관성에 대한 고민은 여전히 남아 있습니다.
특정 도메인에서는 분리와 확장의 필요성이 명확해 4단계 레이어 구조를 적용했지만, 다른 도메인에서는 굳이 필요하지 않아 3단계로 유지했을 때, 설계의 균형이 맞지 않는 느낌이 들었습니다. 해당 아티클에도 레이어들의 규칙이 명시되어 있는데, 이는 어떻게 균형을 맞추는게 좋을까요?
src/main/java/roomescape/reservation/MyReservationResponse.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
public Reservation(String name, String date, Time time, Theme theme) { | ||
public Reservation(String name, String date, Time time, Theme theme, Member member) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 로직은
예약이나 대기가 생성될때마다 중복적인 요소들이 발생할거 같네요.
( Date
- Time
- Theme
들이 중복 )
2월 19일 10:00 공포테마 예약
2월 19일 10:00 공포테마 대기1
2월 19일 10:00 공포테마 대기2
2월 19일 10:00 공포테마 대기3
이에 대한 의견을 들려주세용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재의 예약 테이블과 예약 테이블에서 중복적인 요소를 제거하기 위해 날짜, 시간, 테마 필드를 스케줄
로 네이밍한 테이블로 분리한 방식에 대한 차이에 대해서 고민한 결과입니다.
-
사용자의 예약 목록 조회
-
분리 하지 않는 경우:
예약 테이블에서 사용자에 대한 필터링을 통해 가져올 수 있습니다. 2번의 조인으로 예약 필드를 모두 조회가 가능하지만, 예약 테이블에날짜, 시간, 테마에 대한 중복
이 발생할 수 있습니다. -
분리하는 경우:
스케줄 테이블과 예약 테이블에서 사용자에 대한 필터링을 통해 가져올 수 있습니다. 분리를 통해 중복적인 요소가 제거 되었으나, 분리된 예약 필드를 모두 조회하기 위해 예약 테이블과의조인이 추가로 필요
하게 됩니다.
-
-
예약 생성
-
분리 하지 않는 경우:
예약 생성을모든 날짜, 시간, 테마의 조합으로 생성
하므로 날짜, 시간, 테마 테이블만 초기화되어 있다면 예약할 수 있지만, 모든 방탈출 예약이 동일한 날짜, 시간, 테마 범위를 가지게 됩니다. -
분리하는 경우:
스케줄 테이블에생성 되어있는 날짜, 시간, 테마 조합으로만 생성
할 수 있습니다. 이처럼 스케줄을 미리 초기화 해야 하지만 가능한 조합을 미리 만들어 둘 수 있으며 서비스가 확장되어 방탈출 제공자의 기능이 추가되었을 때 제공자에 맞는 방탈출 예약 조합을 만들 수 있게 됩니다.
-
-
정규화
정규화를 통해 불필요한 데이터를 없앨 수 있고, 삽입/갱신/삭제 시 발생할 수 있는 각종 이상현상 들을 방지할 수 있습니다. 하지만 정규화가 잘되더라도 불필요한 데이터나 중복되는 요소가 발생할 수 있습니다. 현재 모든 테이블은 정규화가 잘 되어 있으므로
추가적인 분리를 고려할 시에 중복과 조인 성능 중에서 선택
이 필요하다고 생각합니다.
분리를 통해 날짜, 시간, 테마를 매개변수로 받던 메서드들이 스케줄의 주키만을 받아 동작하여 가독성이 좋을 것으로 생각됩니다. 또한 날짜, 시간, 테마 조회가 많이 발생하므로 이를 복합키로 만들기 보다는 새 테이블로 분리해서 조인이 필요하지만 주키의 인덱싱을 사용하는 것이 더 좋을 것이라는 생각이 듭니다.
다른 의견도 궁금합니다 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
질문한 의도는
제가 코드를 작성하며 느낀점으론 객체지향
과 DB설계
는 조금 다른걸 느껴서 입니다.
근데, 이게 완벽하게 객체 지향을 안지켜서 발생하는 문제인가? 싶어서 질문합니다.
객체지향적으로 예약과 대기가 상태를 기반으로 엔티티라면 예약정보는 온전히 분리된 VO인거 같기도 한디? 싶기도 하더라구요.
해당 요소와 같을때는 중복적 + 커버링 인덱스로 검색을 커버 가능
하다는 생각이 들어서 분리를 시도하려고 할 거 같긴 해요.
( 물론, 예약 관련된 내용들이 더 추가될 가능성이 있다면 안할거 같긴 해요. DB 는 최대한 수비적으로 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로, 이를 엔티티로 분리하는 방법도 있고
엔티티는 분리는 하지 않되 객체를 가지게 하고 싶다면 @Embedded
를 학습해도 좋을거 같네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 코드를 작성하며 느낀점으론 객체지향과 DB설계 는 조금 다른걸 느껴서 입니다.
근데, 이게 완벽하게 객체 지향을 안지켜서 발생하는 문제인가? 싶어서 질문합니다.
객체지향적으로 예약과 대기가 상태를 기반으로 엔티티라면 예약정보는 온전히 분리된 VO인거 같기도 한디? 싶기도 하더라구요.
말씀해주신 내용이 잘 이해가 되지 않는데 조금 더 자세하게 이야기해 주실 수 있으실까요?
- 예약과 예약 대기에 대한 분리: 객체지향 관점으로 상태를 기준으로 분리될 수 있다.
- 예약 과 (시간, 날짜, 테마) 필드에 대한 분리: 예약 정보가 온전히 분리되어 두 테이블로 분리할 수 있으며 정규화와 인데스를 통해 중복을 줄이고 성능을 개선할 수도 있다.
@Embedded
를 통해서 데이터베이스는 그대로 두고 객체부분에서 응집도를 높일 수 있군요! 한번 학습해보겠습니다. 👍
@@ -21,24 +22,18 @@ public class Reservation { | |||
private Time time; | |||
@ManyToOne(fetch = FetchType.LAZY) | |||
private Theme theme; | |||
@ManyToOne(fetch = FetchType.LAZY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memberId
가 아니라, Member
라는 객체를 넣은 이유가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Member 객체를 넣게 되는 경우 Reservation 과 Member의 객체 관계를 명확히하여 그래프 탐색이나 연관 객체의 변경에 대한 감지에 용이합니다.
Long 객체로 id 만 저장하는 경우 Member 가 필요한 경우 추가 조회 쿼리가 요구됩니다.
현재는 예약 비즈니스 로직에서 Member 의 id 만을 다루기도 하고 조회 쿼리가 대부분이며 추가 쿼리가 필요하다고는 하나, 영속성 컨텍스트에서 조회될 수 있으므로 Member 객체를 가지기 보단 Long 객체를 가지는 것이 좋아보입니다.
하지만 Member 객체를 사용한다면 조회 시에 프록시를 통해 불필요한 데이터를 가져오지 않을 수도 있으며 명확하게 연관관계를 알 수 있습니다. 또한 요구사항의 추가로 예약 도메인에서 사용자의 다른 필드를 다룰 가능성이 크다고 생각하므로 사용하게 되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 답변을 작성하면서 정답이 정해져 있다기보다는 환경에 따라 달라질 수 있는 부분이 많다고 느꼈습니다. 그래서 확실한 지식보다는 부족한 제 생각과 예상이 많이 반영된 것 같아요.
우선, Reseravtion 도메인에 Status 를 추가해 Waiting 을 없앤다면 지금 로직에서 어떻게 달라질지 상상해서 알려줄 수 있나요? ( 엔티티적인 변화 또는 저장할때나 조회할때 어떻게 바뀌는지
날짜, 시간, 테마에 따른 예약을 생성하므로 편의상 날짜, 시간, 테마를 예약 일정이라고 명명하겠습니다.
-
특정 예약 일정에 확정된 예약을 생성하는 메서드
1-1. 특정 예약 일정에 확정된 예약이 존재하지 않는지 체크하는 메서드기존 방식에서 where 절에 state 가 하나 추가된 쿼리가 발생합니다.
-
특정 예약 일정에 대기된 예약을 생성하는 메서드
2-1. 특정 예약 일정에 확정된 예약이 존재하는지 체크하는 메서드
2-2. 특정 예약 일정에 확정된 본인의 예약이 존재하지 않는지 체크하는 메서드
2-3. 특정 예약 일정에 대기된 본인의 예약이 존재하지 않는지 체크하는 메서드기존 방식은 사용자를 fetch join 으로 가져오고 있지만, state 를 사용하는 경우 특정 예약 일정에 본인의 예약을 모두 가져올 수 있으므로 사용자를 가져오지 않아도 됩니다.
-
본인의 확정 또는 대기된 예약을 모두 조회하는 메서드
3-1. 사용자의 예약 목록 조회하는 메서드
3-1-A. 대기된 예약은 순위가 필요하므로 확정 또는 대기된 예약 목록을 따로 조회해서 합치는 방법
3-1-B. sql when 절을 사용해서 대기된 예약의 경우 순위를 계산하는 서브쿼리로 한번에 목록을 조회하는 방법A 방식이 더 좋아보이며 기존 방식과 다르지 않을 것 같습니다.
-
특정 예약 일정의 예약을 취소하는 메서드 + [추가] 대기된 예약을 확정된 예약으로 바꾸는 메서드
4-1.특정 예약 일정의 확정된 예약을 삭제하는 메서드
4-2. 특정 예약 일정의 대기된 예약을 확정으로 변경하는 메서드
4-2-1. 특정 예약 일정의 최신의 대기된 예약을 조회하는 메서드예약을 삭제하고 예약 대기를 생성하는 쿼리보다는, 확정된 예약을 삭제하고 대기된 예약을 수정하는 것이 성능상 좋다고 생각합니다.
분리된 두 테이블에서 또는 하나의 테이블에서 CRUD 및 비즈니스 로직으로 인한 트랜잭션 락 또는 동기화도 고민해봐야겠지만 확정된 예약을 삭제하는데 예약 대기를 생성하지 못하는게 맞을까?
와 같은 상황을 고려했을 때 예약과 예약 대기는 요청 순서대로 처리하는 것이 맞을 것 같다는 생각이 듭니다.
대기라는 엔티티와 상태는 예약이라는 도메인과 로직이 없으면 존재할 수 있나요?
도메인에 대한 개념이 확실하지 않아 분리되는 기준을 패키지, 엔티티, 테이블이라 생각했는데, 예약 없이 예약 대기가 존재할 수 없으므로 예약 대기는 예약 도메인에 존재하는 엔티티였군요. 도메인 개념부터 다시 탐구 해봐야겠습니다 👍
로직을 확장하고 계속 테스트를 작성하다보면 깨지지 않고 반복적인 데이터 패턴들이 보일 수 있습니다.
그러면, 그때 context.sql 과 같이 공통적인 데이터를 관리하는 스크립트를 만들면 될 거 같아요
의견 감사합니다. 👍 그럼 context.sql 은 테스트에서만 사용되는 데이터인거죠?
핵심은, 그 순간 가장 좋고 깔끔한 코드를 짜는것 입니다. ( 레거시도 작성한 당시에는 최고의 사연있는 코드였을겁니다. )
좋은 표현이네요. 개발자로서 확장 가능성을 고려하되, 현재 요구사항을 명확하게 만족하는 코드를 작성하는 것이 중요하네요. 너무 과도한 추상화나 확장은 현재 코드를 어지럽히는 것 같습니다. 👍
Time time = timeRepository.findByIdOrThrow(reservationRequest.time()); | ||
Theme theme = themeRepository.findByIdOrThrow(reservationRequest.theme()); | ||
|
||
Reservation reservation = new Reservation( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조건식을 메서드로 추출하면 네이밍을 통해 조건의 의미를 쉽게 파악할 수 있는 것처럼, 해당 동작도 private method 로 분리하는 것이 동작의 의미를 명확하게 전달할 수 있을 것 같습니다. 수정해보겠습니다 👍
} | ||
|
||
public Reservation(String name, String date, Time time, Theme theme) { | ||
public Reservation(String name, String date, Time time, Theme theme, Member member) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재의 예약 테이블과 예약 테이블에서 중복적인 요소를 제거하기 위해 날짜, 시간, 테마 필드를 스케줄
로 네이밍한 테이블로 분리한 방식에 대한 차이에 대해서 고민한 결과입니다.
-
사용자의 예약 목록 조회
-
분리 하지 않는 경우:
예약 테이블에서 사용자에 대한 필터링을 통해 가져올 수 있습니다. 2번의 조인으로 예약 필드를 모두 조회가 가능하지만, 예약 테이블에날짜, 시간, 테마에 대한 중복
이 발생할 수 있습니다. -
분리하는 경우:
스케줄 테이블과 예약 테이블에서 사용자에 대한 필터링을 통해 가져올 수 있습니다. 분리를 통해 중복적인 요소가 제거 되었으나, 분리된 예약 필드를 모두 조회하기 위해 예약 테이블과의조인이 추가로 필요
하게 됩니다.
-
-
예약 생성
-
분리 하지 않는 경우:
예약 생성을모든 날짜, 시간, 테마의 조합으로 생성
하므로 날짜, 시간, 테마 테이블만 초기화되어 있다면 예약할 수 있지만, 모든 방탈출 예약이 동일한 날짜, 시간, 테마 범위를 가지게 됩니다. -
분리하는 경우:
스케줄 테이블에생성 되어있는 날짜, 시간, 테마 조합으로만 생성
할 수 있습니다. 이처럼 스케줄을 미리 초기화 해야 하지만 가능한 조합을 미리 만들어 둘 수 있으며 서비스가 확장되어 방탈출 제공자의 기능이 추가되었을 때 제공자에 맞는 방탈출 예약 조합을 만들 수 있게 됩니다.
-
-
정규화
정규화를 통해 불필요한 데이터를 없앨 수 있고, 삽입/갱신/삭제 시 발생할 수 있는 각종 이상현상 들을 방지할 수 있습니다. 하지만 정규화가 잘되더라도 불필요한 데이터나 중복되는 요소가 발생할 수 있습니다. 현재 모든 테이블은 정규화가 잘 되어 있으므로
추가적인 분리를 고려할 시에 중복과 조인 성능 중에서 선택
이 필요하다고 생각합니다.
분리를 통해 날짜, 시간, 테마를 매개변수로 받던 메서드들이 스케줄의 주키만을 받아 동작하여 가독성이 좋을 것으로 생각됩니다. 또한 날짜, 시간, 테마 조회가 많이 발생하므로 이를 복합키로 만들기 보다는 새 테이블로 분리해서 조인이 필요하지만 주키의 인덱싱을 사용하는 것이 더 좋을 것이라는 생각이 듭니다.
다른 의견도 궁금합니다 😄
Theme theme = themeRepository.findByIdOrThrow(reservationRequest.getTime()); | ||
Reservation reservation = new Reservation(reservationRequest.getName(), reservationRequest.getDate(), time, | ||
theme); | ||
public List<MyReservationResponse> readAllByMember(String email) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지난번 리뷰에서 알려주신 레이어 분리 방식이 너무 마음에 들었습니다. 공유해주신 지속 성장 가능한 소프트웨어를 만들어가는 방법 글을 읽으며, 레이어를 분리하는 것만으로 해결하려다 보니 메서드 분리를 잊고 있었네요. 수정해보겠습니다. 👍
src/main/java/roomescape/reservation/MyReservationResponse.java
Outdated
Show resolved
Hide resolved
@@ -21,24 +22,18 @@ public class Reservation { | |||
private Time time; | |||
@ManyToOne(fetch = FetchType.LAZY) | |||
private Theme theme; | |||
@ManyToOne(fetch = FetchType.LAZY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Member 객체를 넣게 되는 경우 Reservation 과 Member의 객체 관계를 명확히하여 그래프 탐색이나 연관 객체의 변경에 대한 감지에 용이합니다.
Long 객체로 id 만 저장하는 경우 Member 가 필요한 경우 추가 조회 쿼리가 요구됩니다.
현재는 예약 비즈니스 로직에서 Member 의 id 만을 다루기도 하고 조회 쿼리가 대부분이며 추가 쿼리가 필요하다고는 하나, 영속성 컨텍스트에서 조회될 수 있으므로 Member 객체를 가지기 보단 Long 객체를 가지는 것이 좋아보입니다.
하지만 Member 객체를 사용한다면 조회 시에 프록시를 통해 불필요한 데이터를 가져오지 않을 수도 있으며 명확하게 연관관계를 알 수 있습니다. 또한 요구사항의 추가로 예약 도메인에서 사용자의 다른 필드를 다룰 가능성이 크다고 생각하므로 사용하게 되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드적으로는 크게 건드릴 부분이 없는거 같아용.
현재 설계 방식에서 문제가 발생한다면 어떤 부분에서 발생할 수 있는가? 어떻게 해결할 수 있는가?
에 대해서만 얘기를 나눠보면 될 거 같아요.
현재, 해당 코드에서 발생할 만한 문제는 뭐가 있을까요? ( 정답이 있기 보단 의견이 듣고 싶어서 질문합니다. )
} | ||
|
||
public Reservation(String name, String date, Time time, Theme theme) { | ||
public Reservation(String name, String date, Time time, Theme theme, Member member) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
질문한 의도는
제가 코드를 작성하며 느낀점으론 객체지향
과 DB설계
는 조금 다른걸 느껴서 입니다.
근데, 이게 완벽하게 객체 지향을 안지켜서 발생하는 문제인가? 싶어서 질문합니다.
객체지향적으로 예약과 대기가 상태를 기반으로 엔티티라면 예약정보는 온전히 분리된 VO인거 같기도 한디? 싶기도 하더라구요.
해당 요소와 같을때는 중복적 + 커버링 인덱스로 검색을 커버 가능
하다는 생각이 들어서 분리를 시도하려고 할 거 같긴 해요.
( 물론, 예약 관련된 내용들이 더 추가될 가능성이 있다면 안할거 같긴 해요. DB 는 최대한 수비적으로 )
} | ||
|
||
public Reservation(String name, String date, Time time, Theme theme) { | ||
public Reservation(String name, String date, Time time, Theme theme, Member member) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로, 이를 엔티티로 분리하는 방법도 있고
엔티티는 분리는 하지 않되 객체를 가지게 하고 싶다면 @Embedded
를 학습해도 좋을거 같네요
@@ -25,12 +25,16 @@ public List<ReservationResponse> list() { | |||
return reservationService.findAll(); | |||
} | |||
|
|||
@GetMapping("/reservations-mine") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 경로를 reservations-mine
이라고 한 이유가 있나요?
혹시나 타인의 토큰을 받아서 이를 조회하는 경우가 있다면
( 같이 가는 멤버의 토큰을 공유한다. )
이는 어떻게 되나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서비스의 역할에 대해서 고민한 결과입니다.
예약 목록의 경우 민감한 정보로 비공개 즉, 사용자가 다른 사용자의 예약 목록을 조회하지 못해야 한다고 생각했습니다. 따라서 본인의 예약 목록만 조회할 수 있다고 가정한 결과 본인의 의미가 담긴 접미사를 활용하여 /reservations-mine
로 설계하게 되었습니다.
만약에 일행의 토큰을 받아 예약을 조회하는 경우가 있다면, 조회할 사용자의 id 와 파라미터로 토큰을 받아 id 와 토큰의 정보가 일치하는지 검사한 후에 예약 목록을 반환하며, /user/{userId}/reservations
로 설계할 것 같습니다.
추가로 비회원 사용자가 회원인 일행의 토큰을 받아 조회하는 것보다는 공유하는 방법이 더 적절할 것 같습니다.
뷰에서 사용할 rank 와 일치하는 값을 조회하도록 집계연산을 수정
@@ -25,12 +25,16 @@ public List<ReservationResponse> list() { | |||
return reservationService.findAll(); | |||
} | |||
|
|||
@GetMapping("/reservations-mine") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서비스의 역할에 대해서 고민한 결과입니다.
예약 목록의 경우 민감한 정보로 비공개 즉, 사용자가 다른 사용자의 예약 목록을 조회하지 못해야 한다고 생각했습니다. 따라서 본인의 예약 목록만 조회할 수 있다고 가정한 결과 본인의 의미가 담긴 접미사를 활용하여 /reservations-mine
로 설계하게 되었습니다.
만약에 일행의 토큰을 받아 예약을 조회하는 경우가 있다면, 조회할 사용자의 id 와 파라미터로 토큰을 받아 id 와 토큰의 정보가 일치하는지 검사한 후에 예약 목록을 반환하며, /user/{userId}/reservations
로 설계할 것 같습니다.
추가로 비회원 사용자가 회원인 일행의 토큰을 받아 조회하는 것보다는 공유하는 방법이 더 적절할 것 같습니다.
Theme theme = themeRepository.findByIdOrThrow(reservationRequest.getTime()); | ||
Reservation reservation = new Reservation(reservationRequest.getName(), reservationRequest.getDate(), time, | ||
theme); | ||
public List<MyReservationResponse> readAllByMember(String email) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순히 메서드 호출을 위임하는 형태의 분리보다는, 현재 요구사항을 명확하게 만족하는 코드가 더 중요하다는 관점을 갖게 되었습니다. 하지만 일관성에 대한 고민은 여전히 남아 있습니다.
특정 도메인에서는 분리와 확장의 필요성이 명확해 4단계 레이어 구조를 적용했지만, 다른 도메인에서는 굳이 필요하지 않아 3단계로 유지했을 때, 설계의 균형이 맞지 않는 느낌이 들었습니다. 해당 아티클에도 레이어들의 규칙이 명시되어 있는데, 이는 어떻게 균형을 맞추는게 좋을까요?
} | ||
|
||
public Reservation(String name, String date, Time time, Theme theme) { | ||
public Reservation(String name, String date, Time time, Theme theme, Member member) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 코드를 작성하며 느낀점으론 객체지향과 DB설계 는 조금 다른걸 느껴서 입니다.
근데, 이게 완벽하게 객체 지향을 안지켜서 발생하는 문제인가? 싶어서 질문합니다.
객체지향적으로 예약과 대기가 상태를 기반으로 엔티티라면 예약정보는 온전히 분리된 VO인거 같기도 한디? 싶기도 하더라구요.
말씀해주신 내용이 잘 이해가 되지 않는데 조금 더 자세하게 이야기해 주실 수 있으실까요?
- 예약과 예약 대기에 대한 분리: 객체지향 관점으로 상태를 기준으로 분리될 수 있다.
- 예약 과 (시간, 날짜, 테마) 필드에 대한 분리: 예약 정보가 온전히 분리되어 두 테이블로 분리할 수 있으며 정규화와 인데스를 통해 중복을 줄이고 성능을 개선할 수도 있다.
@Embedded
를 통해서 데이터베이스는 그대로 두고 객체부분에서 응집도를 높일 수 있군요! 한번 학습해보겠습니다. 👍
throw new IllegalArgumentException( | ||
String.format("Reservation already exist %s, %s, %s", date, time.getValue(), theme.getName())); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 메서드를 분리하고 싶은데 마땅한 방법이 떠오르지 않네요,,
🏃♂️ 과정
요구사항을 다시 해석해서 구현하게 되었습니다. 정답을 찾기 어려웠거나 없는 부분이 많아서 추가로 의견을 여쭙게 되었습니다.
이번 리뷰도 잘 부탁드립니다. 😄
📃 요구사항
내 예약 목록 조회
이전 3단계 요구사항으로 예약 생성 시에 요청 객체에 이름 필드 값이 존재한다면 해당 이름을, 존재하지 않는다면 로그인한 사용자의 이름을 넣어주었습니다. 이때 해당 예약의 누구의 예약인지 중점을 두었고,
이름과 관계없이 현재 로그인 되어 있는 유저로 연관관계를 맺어주었습니다.
처음에는
어드민이 브라운으로 예약을 한다면 이것은 브라운의 예약이 아닐까
하는 생각도 들었지만, 택배를 시킬 때 배송자 명은 자유롭게 수정 가능하고, 해당 택배 건과 관계가 없으므로 이름은 키가 아니라고 생각하고 구현하게 되었습니다.로직
예외
예약 대기 기능
로직
예외
🔎 궁금한점
두가지 이상의 도메인의 비즈니스로직이 결합된 기능에 대한 고민
예약 대기 생성: Waiting 을 생성하기 위해서 Reservation 을 검증하는 로직
Waiting 도메인에서 Reserviaton 테이블에 대해서 검증을 수행하는데 분리가 필요해보입니다.
WaitingService::validateReservationExistsAndNotOwnedByMember
예약 목록 조회: Reservation list 에 Waiting list 도 포함되는 로직
두가지 도메인의 로직이 결합된 해당 메서드는 어디에 두는게 좋을까요?
ReservationService::readAllByMember
추가로 두 도메인의 성격이 매우 비슷하여 Reservation 도메인에 state 필드를 사용해서 Waiting 도메인의 역할을 하면 좋겠다는 생각을 했습니다. 하지만 Reservation 의 테이블과 비즈니스 로직이 많이 커지게 되었는데 레이어를 추가하거나 Reservation 엔티티를 사용하는 WaitingService 와 같이 서비스를 하나 더 두는 방식이 있을 것 같습니다.
테스트 시에 초기 데이터(data.sql) 의존에 대한 고민
초기 데이터들을 활용해서 조회 테스트를 작성했을 때 많은 도움을 받았습니다. 하지만 초기 데이터(data.sql)를 모르는 사람이 보았을 때를 생각해보니 가독성이 좋지 않았습니다. (ex) 초기 데이터로 인해 예약 목록을 조회했을 때 예약 데이터가 3개 반환 된다는 확신)
따라서 테스트시에
repository.save()
,entityManager.persist()
,.flush()
등을 사용해서 조회할 데이터를 넣어주는 방식을 사용하게 되었습니다.API 통합 테스트에 사용할 데이터 입력
통합 테스트는 데이터베이스에 영향을 받아야 한다는 생각이 들어서 초기 데이터(data.sql)에 의존한 결과
WaitingControllerTest
는 어떤 입력이 검증에서 실패하는지 잘 나타내지 못했습니다.이에 필요한 데이터를 넣어주기 위해 API를 호출하는 방식을 사용하게 되었는데, 이는 테스트 코드도 매우 길어지게 되었고 많은 시간을 요구하게 되었습니다. 테스트가 초기 데이터(data.sql)에 어느정도 의존하는게 좋을까요?
API 요청 Optional 필드 구현에 대한 고민
비즈니스 로직에서 Optional.ofNullable() 을 통해 처리하게 되었는데 서비스 단의 dto 를 만드는게 좋을까요?
ReservationService::save
추가로 이름에 따라서 사용자의 기능과 어드민의 기능으로 분리해볼 수도 있을 것 같은데 거의 비슷한 동작이라 중복되는 로직이 많이 생기기도 했고 다른 엔드포인트를 사용해야 한다는 점으로 인해 분리하지 않게 되었습니다. 요구사항에 따라서 달라지는 부분이겠죠?