[Spring Data JPA] 망고 5, 6단계 미션 제출합니다. #147

heehunjung · 2025-04-02T10:59:44Z

안녕하세요, 콜리!
step4 좋은 리뷰 감사했습니다 ㅎㅎ
일정이 좀 많이 겹쳐 늦었네요 ㅜㅜ

PR - STEP 5,6

구현 사항

예약 대기 요청 및 취소
- waiting 엔티티 클래스 생성
중복 예약 방지
reservationService, waitingService 테스트 추가

고려한 부분

[예약 대기에 대기 순서가 존재] ranking 컬럼을 waiting 테이블에 추가 vs 조회 시 count 하여 제공
- 후자를 선택
  - 전자를 선택하면 대기 삭제 시 모든 대기들을 조회하고 ranking을 업데이트 해줘야함
  - 따라서 후자를 선택하고 ranking을 updateAt에 따라 count해서 제공

마지막 기간 잘 부탁드립니다 🙇
리뷰에따라 잘 학습하고 개선해볼게요 !

다시 한번 감사합니다 ㅎㅎㅎ

프론트와 데이터 형태 맞춤, 동일한 인원이 예약과 대기 한번에 못하게

coli-geonwoo

망고 안녕하세요!

1차 리뷰 남겨드립니다! 아마 제가 최근에 확 바빠져서 리뷰 주기를 길게 가져가고자 최대한 보이는 부분에서 많은 리뷰 남겨드렸어요! 궁금하신 점이나 잘 감이 안오는 리뷰들은 DM 주셔도 좋습니다 ㅎㅎ

추가로 이번 리뷰는 커밋 기록을 따라가며 남겼습니다. 따라서 현재 코드에서는 수정된 부분이더라도 리팩터링의 이유나 생각을 묻는 리뷰가 포함될 수 있으니 유의해주세요!

coli-geonwoo · 2025-04-03T16:12:41Z

src/main/java/roomescape/reservation/Reservation.java

@@ -38,6 +39,9 @@ public class Reservation {
    @ManyToOne(fetch = FetchType.LAZY)
    private Theme theme;

+    @ManyToOne(fetch = FetchType.LAZY)
+    private Member member;
+
    @OneToMany(fetch = FetchType.LAZY, mappedBy = "reservation")
    private List<ForStudy> forStudies;


열심히 공부하셨네요 망고 👍

다만 지금은 필요하지 않은 필드로 보이니 제거해줍시다 😄

반영할게요 ㅎㅎ

coli-geonwoo · 2025-04-03T16:19:54Z

src/main/java/roomescape/reservation/Reservation.java

@@ -64,6 +64,14 @@ public Reservation(String name, LocalDate date, ReservationTime reservationTime,
        this.theme = theme;
    }

+    public Reservation(Member member, String name, LocalDate date, ReservationTime reservationTime, Theme theme) {


[질문 ✋ ]

이건 순수한 궁금증인데 혹시 member가 없는 예약 객체의 생성을 허용해 놓은 이유는 무엇인가요?
member 엔티티의 연관관계 추가에 따라 모든 예약에는 특정 member에게 귀속되어야 하는 것 아닌가요?

어드민 페이지에서 이름을 추가하는 형태로 구현되어 있고 어드민이라면 임의로 예약을 추가할 수 있을 것 같다고 생각했어요. (아이디 없는 지인 예약 또는 전화 예약)

따라서 member 없이도 이름만으로 예약을 만들 수 있도록 구현했어요 !

[의견 🔈 ]

도메인을 현실상황에 비추어 다양한 상황에 대비하는 자세는 좋습니다.

다만 member 없는 예약이 과연 가능한지에 대해서는 의문이 있으며, 그 멤버의 식별이 이름으로 판단되는 것 또한 오류가능성을 내포한다고 생각합니다.

예약에 정말 대상 회원이 없다면, 추후 예약자의 세부정보를 확인할 때(생년 월일, 가입일자, 성별 등등) 어떻게 확인하나요?

이름으로 회원을 식별한다고 하면 동명이인의 경우 예약자가 누구인지 어떻게 아나요?

외래키에 null이 들어가면 상황에 따라 inner join의 경우 조회되지 않을 수 있습니다. 고려하셨나요?

단순히 이런 상황을 대비했다기보다, 이 상황에 대비할 수 있는 여러 대안을 생각하고, 그 중에 가장 합리적인 대안을 선택하는 게 좋을 것 같아요. 만약 저라면 회원이 없는 예약이라는 데이터 정규성을 해치는 정책보다 어드민이 새로운 회원을 생성하는 걸 허용해주고 예약은 어느 상황에서든 회원이 있는 방식으로 정책을 정했을 것 같습니다.

이외에 망고가 다른 생각이 있다면 설득해주어도 좋습니다.

외래키에 null이 들어가면 상황에 따라 inner join의 경우 조회되지 않을 수 있습니다. 고려하셨나요?

전체 조회는 어드민 페이지에서만 하는 데, member 엔티티를 조회하고 있지 않아서 단순히 findAll로 괜찮다고 생각했었어요. (reservation의 필드에 name(유저네임)이 포함되어 있습니다!)

예약에 정말 대상 회원이 없다면, 추후 예약자의 세부정보를 확인할 때(생년 월일, 가입일자, 성별 등등) 어떻게 확인하나요?

대상 회원이 없는 예약은 어드민이 만든 예약이라 따로 세부정보를 확인할 필요가 없다고 생각했었어요.

이름으로 회원을 식별한다고 하면 동명이인의 경우 예약자가 누구인지 어떻게 아나요?

이 부분은 멤버간 확인은 가능하고 회원이 없는 예약은 위에 설명했던 것 처럼 어드민이 만든 예약이라 어드민 입장에서 식별 가능하다고 생각했어요.

멤버 입장에서는 본인 예약은 스스로 알거라 생각했구요 !

하지만 코멘트를 보고 생각해보니 member를 null로 처리한다면NullPointException의 위험성과 memberId(외래 키)만 명시적으로 쓰는 게 낫겠다라는 생각이 들었습니다.

따라서 멤버에 어드민 예약용 데이터를 미리 한 개 만들어두고 어드민 예약 시 해당 멤버를 사용하도록 변경했습니다 !

이름으로 존재하는 멤버를 조회해서 예약 생성하는 방식으로 하려면 이름을 유니크하게 설정 해야합니다.
저는 name을 실제 이름으로 생각하고 구현해서 중복을 허용하는 게 맞다 판단하여 해당 방법은 선택하지 않았어요.

coli-geonwoo · 2025-04-03T16:20:39Z

src/main/java/roomescape/reservation/ReservationController.java

+    private ReservationResponse getReservationResponse(Member member,
+                                                       ReservationRequest reservationRequest,
+                                                       boolean isAdmin) {
+        if (isAdmin) {


[질문 ✋ ]

해당 로직을 컨트롤러에 둔 이유가 궁금합니다!

멤버 요청인지 관리자 요청인지 구분하는 것이 처음에는 요청에 관련된 작업이라 컨트롤러에 뒀었어요.

그런데 생각해보니, 추출한 정보를 바탕으로 판단을 내리는 건 비즈니스 로직에 포함된다고 볼 수 있고, 컨트롤러는 요청 파싱과 응답 처리 역할과 같이 뷰 관련 로직들을 담당하는 게 좋다고 생각해서 변경하겠습니다 !

coli-geonwoo · 2025-04-03T16:21:23Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -37,20 +37,28 @@ public ResponseEntity<List<MyReservationResponse>> mine() {
    public ResponseEntity create(@AuthMember Member member
            , @RequestBody ReservationRequest reservationRequest) {
        Role role = member.getRole();


멤버에게 어드민인지 물어보는 과정을 조금 더 객체지향적으로 바꾸어볼 수 있을 것 같아요! 망고의 생각은 어떤까요?

동의합니다 ! 위 코멘트할 때 같이 수정했어요 ㅎㅎ

coli-geonwoo · 2025-04-03T16:23:54Z

src/main/java/roomescape/reservation/ReservationService.java

+        return new ReservationResponse(reservation);
+    }
+
+    private ReservationTime getReservationTime(final ReservationRequest reservationRequest) {


[제안 🔈 ]
dto는 변경이 많은 객체에요. 서비스에서 메서드를 분리할 땐, 최대한 재활용이 가능하도록 변경의 여지가 적은 것들을 기준으로 구성되어야 한다고 생각해요. ReservationRequest가 이렇게 깊게 침투하게 되면 dto 변경 시 변경의 여파는 어떻게 될까요?

현재 dto를 파라미터로 전달하고 있었는 데, long xxxId를 전달받는 함수로 변경하여 변경의 범위를 해당 함수의 파라미터로 줄였습니다 !

또 이렇게 구현하면 다른 서비스 메서드에서 해당 객체 조회할 때 재사용하기 편할 것 같아요.

좋은 의견 감사합니다 ㅎㅎ

coli-geonwoo · 2025-04-03T16:57:34Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -29,10 +34,13 @@ public List<ReservationResponse> list() {
    }

    @GetMapping("/reservations-mine")
-    public ResponseEntity<List<MemberReservationResponse>> mine(@AuthMember Member member) {
-        List<MemberReservationResponse> result = reservationService.getMyReservations(member.getId());
+    public ResponseEntity<MemberReservationResponses> getMemberReservations(


[질문 ✋ ]

비즈니스 로직이 controller에 조금 튀어나와있는 느낌이 드네요. ReservationService 내에 WaitingService가 있지 않고 Controller가 ReservatonService와 WaitingService 둘다 가지게 한 이유는 무엇일까요?

예약과 예약 대기라는 개념은 어떤게 상위 개념이고, 어떤게 파생된 개념일까요?

예약과 예약 대기라는 개념은 어떤게 상위 개념이고, 어떤게 파생된 개념일까요?

예약이 있어야 예약 대기가 있으니깐 예약이 상위 개념이라고 생각해요.

비즈니스 로직이 controller에 조금 튀어나와있는 느낌이 드네요. ReservationService 내에 WaitingService가 있지 않고 Controller가 ReservatonService와 WaitingService 둘다 가지게 한 이유는 무엇일까요?

단순히 controller가 service보다 상위 계층이라 포함시켰었어요.

상위 개념 하위 개념에 대해 생각하다보니 ReservationService 내부에서 WaitingService를 가지고있는 것이 자연스러워보이네요 !

비즈니스 로직 순서도 그에 따라 예약을 찾고 대기를 찾고 있어서 ReservationService에서 WaitingService를 포함해서 비즈니스 로직을 수행하는 게 더 적절해보이네요. 반영하겠습니다 ㅎㅎ

위처럼 적고 수정하려고 보니까, 대기와 예약은 개념적으로는 상위/하위 개념이 맞긴하지만,

한 사람의 예약과 대기를 놓고 보면 꼭 상위 개념과 파생 개념이라고 보긴 애매

실제 비즈니스 로직도 각자 테이블에서 memberId 기준으로 독립적으로 데이터 가져오고 있음

위 2가지와 컨트롤러 쪽도 살펴보면,
응답을 조립하기 위해 일급 컬렉션에서 값을 합치는 메서드만 호출하는 정도라 비즈니스 로직이 컨트롤러까지 새어 나온 느낌은 아닌 것 같기도 해서 콜리의 의견이 궁금합니다 ! ㅎㅎ 🆘 🆘

[답변]

바로 답변 드리면 재미없으니 조금 힌트를 드리겠습니다.

어느 레이어에 해당 로직이 들어가야 하는지 판단이 잘 서지 않는다면 해당 정책이 서비스 정책인지 생각해보시면 좋습니다.

예약과 예약 대기를 한 화면에 같이 보여준다 <- 해당 로직은 서비스 로직인가요?

맞다면 서비스 계층에서 처리되어야 할 것이고, 아니라면 지금처럼 컨트롤러 계층에 있어도 좋을 것 같습니다. 컨트롤러가 담당해야 하는 역할과 서비스 계층이 담당해야 하는 역할을 다시한번 복기해보는 것도 좋은 경험이 될 것 같네요.

두번째로 상위 개념과 하위 개념에 대한 구분 방식에 대한 것입니다. 꼭 두 객체가 한 곳에 하나가 포함되어 있어야 상위-하위 객체라고 부르는 것은 아닙니다. 제가 정의하는 도메인의 상하위 개념은 충분조건과 필요조건의 개념과 유사합니다.
그 관점에서 상위 개념이 존재해야 하위 개념이 존재할 수 있습니다. 그럼 물어보면 다음과 같습니다.

예약이 존재해야 예약 대기가 존재할 수 있나요? 반대로 예약 대기가 존재해야 예약이 존재할 수 있나요?

그럼 이 질문으로 부터 api 관점까지 나아가면 이렇습니다. 현재 API 에서는 "예약에 관한" 나의 정보를 모두 가져와달라고 하고 있지요. 그곳에 예약과 예약대기를 모두 반환하고 있습니다. 그럼 어떤 것이 상위 개념일까요?

힌트 감사합니다 ㅎㅎ

예약과 예약 대기를 한 화면에 같이 보여준다 <- 해당 로직은 서비스 로직인가요?

같이 보여준다라는 점에서 view와 관련되었다고 생각합니다. 따라서 컨트롤러에서 처리해도 될 것 같아요 ! 명확히 나누어서 어디의 책임인지 확인하는 게 좋은 방법인 것 같네요 감사합니다. ㅎㅎ

그럼 어떤 것이 상위 개념일까요?

예약이 상위 개념이라고 생각해요.

하지만 각 도메인 별 단순 조회는 각자의 레포지토리를 사용하는 각 각의 서비스 클래스에서 수행하는 게 좋아보이기도 해요.

예약만 조회하거나 예약 대기만 조회하는 경우 새로운 서비스클래스 메서드를 만들거나 필터링 해줘야할 것 같아요.

그럴거면 나눠서 조회하는 게 코드의 중복을 줄이고 불필요한 코드를 만들지 않아도 된다는 점에서 좋은 것 같습니다 !

하지만 특정 날짜,시간과 테마에 관련된 예약, 예약 대기를 보여달라고 하면 명백하게 예약과 예약 대기가 부모 자식과 같은 관계로 연관되어 있으니
WaitingService를 ReservationService에서 호출하여 구현할 것 같기도 합니다..!

예약과 예약 대기를 한 화면에 같이 보여준다 -> 해당 로직은 서비스로직입니다.

왜인지에 대해 생각해보죠. 아마 망고는 '보여준다'라는 키워드가 들어갔기에 뷰라고 판단한 것 같아요. 다만 무엇을 보여주냐?와 어떻게 보여주나? 는 다른 개념입니다. 그리고 무엇을 보여주냐? 는 서비스 정책인 반면, 그렇게 서비스적으로 보여주기로 결정된 것을 어떻게 보여주나? 라는 개념이 뷰와 가깝다고 생각하면 되겠습니다.

예를 들어 보겠습니다. 예약과 예약대기를 같이 보여준다는 서비스 정책이 결정되면, 해당 정책을 뷰에 그릴 때 시간은 어떤 형식으로 보여줄지 결정해야 합니다. 이런 형식, 모양 등이 뷰에 대한 논의입니다.

그럴거면 나눠서 조회하는 게 코드의 중복을 줄이고 불필요한 코드를 만들지 않아도 된다는 점에서 좋은 것 같습니다 !

이부분은 합리적이라 생각되어 납득되었습니다 👍

오.. 단순히 보여준다라고 생각해서 뷰 관련 로직이라고 생각했었는 데, 합치는 행위는 (무엇을 보여주는지)는 서비스 정책으로 보면 되는군요 ㅎㅎ 헷갈리는 부분이 좀 명확해진 것 같아요.

해당 코드들이 서비스 로직이여서 컨트롤러에 노출하는 것을 막기위해
예약, 대기를 각 각 조회하는 서비스클래스 메서드들을 유지하고, ReservationService에 두 메서드를 호출하여 합치는 메서드를 만들습니다 !

coli-geonwoo · 2025-04-03T17:02:05Z

src/main/resources/static/js/user-reservation.js

@@ -179,7 +179,7 @@ function onWaitButtonClick() {
          return response.json();
        })
        .then(data => {
-          alert("대기 순서" + data.waitingNumber + "번째");
+          alert("대기 순서" + data.ranking + "번째");


js 코드를 만진 김에 하나만 제안해보고 싶어요. 대기순서에 +1 하는거 여기서 하면 안되나요? ㅋㅋㅋㅋ +1L이 조금 불편해서...

앗 수정했어요 ! 디테일을 진짜 잘보시는 것 같습니다..ㅎㅎ 👍

coli-geonwoo · 2025-04-03T17:05:18Z

src/main/java/roomescape/reservation/ReservationService.java

@@ -35,6 +37,7 @@ public ReservationResponse save(ReservationRequest reservationRequest) {

    public ReservationResponse saveWithMember(ReservationRequest reservationRequest,
                                              Member member) {
+        validatedRequest(reservationRequest);


앞선 commit에서 중복예약 검증 리뷰남기려다가 고려해주셨네요 👍 좋은 도메인적 고민입니다.

coli-geonwoo · 2025-04-03T17:08:08Z

src/main/java/roomescape/reservation/Reservation.java

@@ -110,4 +110,8 @@ public List<ForStudy> getForStudies() {
    public boolean isOwner(Member member) {
        return this.member.equals(member);
    }
+
+    public boolean isMadeByAdmin() {


너무 서비스 로직(어드민이 생성할 경우 null로 초기화한다)의 관점이 들어가있는 메서드명이 아닌가 싶어요.그리고 그 정보를 reservation이 아는 것이 부자연스러워 보입니다.

또 admin인 경우에는 member를 null로 처리하는 이유가 궁금합니다. 역할이 admin인 멤버의 예약은 존재할 수 없는건가요?

너무 서비스 로직(어드민이 생성할 경우 null로 초기화한다)의 관점이 들어가있는 메서드명이 아닌가 싶어요.그리고 그 정보를 reservation이 아는 것이 부자연스러워 보입니다.

완전 동의합니다 !
예약이 member가 어떤 사람인지 알 필요가 없는 데 이상하게 구현했네요.. ㅎㅎ

if (member.isAdmin()) { return reservation; } if (reservation.isOwner(member)) { throw new RoomescapeBadRequestException("본인이 예약한 방에는 대기를 할 수 없습니다."); }

현재는 원래 member에서 있던 isAdmin 메서드로 변경했습니다.
사실 이런 코드 구조가 아래 답변의 답이랑 중복되네요 ㅠㅠ

또 admin인 경우에는 member를 null로 처리하는 이유가 궁금합니다. 역할이 admin인 멤버의 예약은 존재할 수 없는건가요?

프론트 상에서 예약을 추가할 수 있는 방법은 관리자 페이지, 기본 페이지에서 추가하는 방법 2가지 입니다.
우선 admin이 관리자 페이지를 두고 기본 페이지에서 예약을 생성하는 게 좀 어색하다고 생각해서 관리자 페이지에서만 예약을 하게 구현했습니다.

관리자 페이지에서는 예약자 명을 직접 입력하는 데, 1. 이름으로 멤버를 찾아서 예약을 생성 2. 이름으로 예약을 바로 생성 (멤버 필드를 채우지 않고) 에서 저는 후자를 선택해서 관리자는 이름으로 예약을 할 수 있도록 만들었습니다 ..!

관리자가 지인 예약이나 본인 예약을 잡고싶을 때 아무 이름으로나 만들 수 있다고 생각해서 그렇게 했습니다. (이러면 멤버가 어드민에게 예약을 직접 잡아달라고 요청하면 좀 힘들어지긴 합니다 ㅠㅠ)

적다보니 빈틈이 좀 많은 것 같은데 이 부분은 나중에 시간날 때 리팩토링 해도 될까요 ? ㅠㅠ

나중에 리팩터링하는 것은 망고의 자유입니다!

다만 여기서 논하고 싶은 것은 반영을 해주세요- 적인 차원보다 망고가 이야기한 내용들이 합리적인지, 저를 설득하기 힘들었던 부분들은 무엇인지에 대해 논하고 싶습니다.

저는 DB에 null이 저장되는 것을 좋아하지 않습니다. null은 오류 가능성을 내포하며 정규화가 제대로 되어 있지 않음을 나타내는 신호와도 같습니다. 같은 맥락에서 예약에 member가 비어있는 것을 생각해보았을 때, 외래키라는 주요한 제약조건에 null 값을 채워 넣는 것이 바람직한 것인지에 대한 의문이 있습니다.

물론 유저의 편의성을 위해 멤버가 없는 예약을 한다 는 것을 허용해준 것은 좋습니다. 다만 주인이 없는 예약은 부자연스러워요. 그 관점에서 주인을 특정하지 못하는 상황이 내포된 외래키 null 삽입이 좋은 판단일지 의문이 듭니다.

당연히 당연히 제 개인적인 의견이고요. 의견을 납득하기보다 망고가 조금 더 생각을 어필해주는 것도 좋은 리뷰 티키타카가 될 것 같습니다.

주인이 없는 예약은 없는 게 맞긴 하지만, 저는 어드민이 특정 시간, 타입의 예약을 막을 방법도 필요하다고 생각했어요.

현재 주어진 프론트 상에서 가능한 방법이 임의 예약 추가라는 방법밖에 없다고 생각하여 멤버를 null 삽입하는 방식으로 구현했습니다.

하지만, null 삽입에 대한 부작용에 대해서는 크게 생각하지 않고 로직을 작성했고, 외래키에 null 삽입이 좋지 않는 판단이라는 것에 동의 합니다 !

위의 코멘트 진행하면서 현재는 어드민 예약 시 어플리케이션 생성 시 생성 된 멤버(어드민 예약용 멤버)를 사용해서 만들도록 했어요.

그리고 실제 서비스 흐름에서는 사용자가 예약 조회 시 사용자 본인의 memberId를 기준으로 조회하고 있기 때문에,
이 '어드민 예약용 멤버'로 생성된 예약이 일반 사용자에게 노출되거나 혼동을 줄 가능성은 낮다고 판단했습니다!

coli-geonwoo · 2025-04-03T17:11:43Z

src/main/java/roomescape/waiting/WaitingRepository.java

                WHERE w.member.id = :memberId
+                ORDER BY r.date ASC, rt.timeValue ASC


[질문 ✋ ]

시간 순으로 정렬하여 보여준다는 서비스 로직일까요? 아니면 쿼리 자체에 고정되어도 되는 로직일까요?

반대로 findWaitingRankingByMemberId 라는 메서드명을 보았을 때 시간순 정렬을 유추할 수 있을까요?

유추할수없다고 생각합니다!
id로 바꾸니 저 문제는 사라졌지만, update를 사용한다고 했어도 콜리 말처럼 쿼리 자체에서 빼거나 꼭 필요하다면 메서드 명에 AscByTime 처럼 추가할 것 같아요 ㅎㅎ

기존에는 reservaionId로 조회하면서 Optional로 받아서 List로 변경하고 받은 데이터로 대기 내역 조회 후 예외 처리 또는 새로운 대기 생성으로 변경

heehunjung

안녕하세요 콜리! 고봉밥 리뷰 감사합니다 ㅎㅎ
항상 공부할 부분을 잘 지적해주셔서 많이 배워가요 !

코멘트 및 반영했습니다 🙇

heehunjung · 2025-04-04T07:59:05Z

src/main/java/roomescape/reservation/MemberReservationResponse.java

        , String time, String status) {

+    public static final String WAITING_STATUS_MESSAGE = "%d번째 예약대기";


없습니다 ㅜㅜ 해당 클래스에서만 사용하므로 private 으로 변경할게요 !

heehunjung · 2025-04-04T12:36:29Z

src/main/java/roomescape/reservation/Reservation.java

@@ -64,6 +64,14 @@ public Reservation(String name, LocalDate date, ReservationTime reservationTime,
        this.theme = theme;
    }

+    public Reservation(Member member, String name, LocalDate date, ReservationTime reservationTime, Theme theme) {


어드민 페이지에서 이름을 추가하는 형태로 구현되어 있고 어드민이라면 임의로 예약을 추가할 수 있을 것 같다고 생각했어요. (아이디 없는 지인 예약 또는 전화 예약)

따라서 member 없이도 이름만으로 예약을 만들 수 있도록 구현했어요 !

heehunjung · 2025-04-04T18:15:12Z

src/main/java/roomescape/member/Member.java

@@ -62,4 +63,23 @@ public String getPassword() {
    public Role getRole() {
        return role;
    }
+
+    @Override


처음에는 직접 만든 객체의 동등성을 비교하려면 equals()와 hashCode()를 모두 오버라이딩해야 equals()로 동등성 비교가 가능하다고 단순하게 이해하고 사용했어요.

이번 기회에 eq & hc를 다시 학습했습니다 !

equals

기본 구현은 Object 클래스의 equals()로, 객체가 동일한지 검사하기 위해 사용, 객체의 동일성을 비교

메모리 상에 같은 위치인지 비교하는 것이기 때문에 동일한 객체가 메모리 상에 여러 개 띄어져있는 경우 동일한 객체로 인식하지 못함

equals ()를 오버라이딩 해서 동등성 비교를 비교할 때 사용

hashCode

기본적으로 실행 중에 객체의 유일한 integer 값을 반환

객체를 해시 기반 Collection(HashSet, HashMap 등)에 사용할 때, 객체를 식별하는 정수값을 제공

함께 Overriding

equal 만 오버라이링을 통해 재정의해서 사용하더라도 객체의 동등성은 판단 가능합니다.

HashSet, HashMap, HashTable같이 hash값을 사용하는 collection을 사용할 때 collection은 객체가 논리적으로 같은 지 비교할 때 hashCode() -> equals() 순서로 호출하여 같은 객체인지 비교합니다.

해당 collection을 쓰지않으면 무조건 hashCode를 재정의 해야하는 건 아닙니다.

하지만 보통 eq, hc를 같이 재정의하는 경우가 많기 때문에 협업 상황이나 코드의 재사용성을 고려할 때 두 메서드를 함께 오버라이딩하는 것이 좋은 것 같습니다.

여기서는 id로 비교하는 것으로 변경해서 삭제해도 될 것 같습니다 ㅎㅎ 반영할게요 !

heehunjung · 2025-04-05T02:13:24Z

src/main/java/roomescape/reservation/Reservation.java

@@ -38,6 +39,9 @@ public class Reservation {
    @ManyToOne(fetch = FetchType.LAZY)
    private Theme theme;

+    @ManyToOne(fetch = FetchType.LAZY)
+    private Member member;
+
    @OneToMany(fetch = FetchType.LAZY, mappedBy = "reservation")
    private List<ForStudy> forStudies;


반영할게요 ㅎㅎ

heehunjung · 2025-04-05T05:57:50Z

src/main/java/roomescape/reservation/ReservationController.java

+    private ReservationResponse getReservationResponse(Member member,
+                                                       ReservationRequest reservationRequest,
+                                                       boolean isAdmin) {
+        if (isAdmin) {


멤버 요청인지 관리자 요청인지 구분하는 것이 처음에는 요청에 관련된 작업이라 컨트롤러에 뒀었어요.

그런데 생각해보니, 추출한 정보를 바탕으로 판단을 내리는 건 비즈니스 로직에 포함된다고 볼 수 있고, 컨트롤러는 요청 파싱과 응답 처리 역할과 같이 뷰 관련 로직들을 담당하는 게 좋다고 생각해서 변경하겠습니다 !

heehunjung · 2025-04-05T13:43:00Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -29,10 +34,13 @@ public List<ReservationResponse> list() {
    }

    @GetMapping("/reservations-mine")
-    public ResponseEntity<List<MemberReservationResponse>> mine(@AuthMember Member member) {
-        List<MemberReservationResponse> result = reservationService.getMyReservations(member.getId());
+    public ResponseEntity<MemberReservationResponses> getMemberReservations(


위처럼 적고 수정하려고 보니까, 대기와 예약은 개념적으로는 상위/하위 개념이 맞긴하지만,

한 사람의 예약과 대기를 놓고 보면 꼭 상위 개념과 파생 개념이라고 보긴 애매

실제 비즈니스 로직도 각자 테이블에서 memberId 기준으로 독립적으로 데이터 가져오고 있음

위 2가지와 컨트롤러 쪽도 살펴보면,
응답을 조립하기 위해 일급 컬렉션에서 값을 합치는 메서드만 호출하는 정도라 비즈니스 로직이 컨트롤러까지 새어 나온 느낌은 아닌 것 같기도 해서 콜리의 의견이 궁금합니다 ! ㅎㅎ 🆘 🆘

heehunjung · 2025-04-05T14:19:51Z

src/main/java/roomescape/waiting/WaitingService.java

+        Optional<Waiting> existedWaiting = waitingRepository.findByMemberId(member.getId());
+
+        if (existedWaiting.isPresent()) {
+            existedWaiting.get().refreshTimestamp();


여기 반영하면서 이전에는 완전 잘못 구현했던 것을 알게되었습니다..

기존에는 reservationId로 조회할 때 Optional을 사용해 단일 데이터를 받아오던 로직을, List로 변경하여 여러 데이터를 처리하도록 수정했습니다. 이후 받은 데이터를 기반으로 대기 내역을 조회하고, 예외 처리를 하거나 새로운 대기 항목을 생성하는 방식으로 변경했습니다.

heehunjung · 2025-04-05T14:24:25Z

src/main/resources/static/js/user-reservation.js

@@ -179,7 +179,7 @@ function onWaitButtonClick() {
          return response.json();
        })
        .then(data => {
-          alert("대기 순서" + data.waitingNumber + "번째");
+          alert("대기 순서" + data.ranking + "번째");


앗 수정했어요 ! 디테일을 진짜 잘보시는 것 같습니다..ㅎㅎ 👍

heehunjung · 2025-04-05T14:48:07Z

src/main/java/roomescape/reservation/Reservation.java

@@ -110,4 +110,8 @@ public List<ForStudy> getForStudies() {
    public boolean isOwner(Member member) {
        return this.member.equals(member);
    }
+
+    public boolean isMadeByAdmin() {


너무 서비스 로직(어드민이 생성할 경우 null로 초기화한다)의 관점이 들어가있는 메서드명이 아닌가 싶어요.그리고 그 정보를 reservation이 아는 것이 부자연스러워 보입니다.

완전 동의합니다 !
예약이 member가 어떤 사람인지 알 필요가 없는 데 이상하게 구현했네요.. ㅎㅎ

if (member.isAdmin()) { return reservation; } if (reservation.isOwner(member)) { throw new RoomescapeBadRequestException("본인이 예약한 방에는 대기를 할 수 없습니다."); }

현재는 원래 member에서 있던 isAdmin 메서드로 변경했습니다.
사실 이런 코드 구조가 아래 답변의 답이랑 중복되네요 ㅠㅠ

또 admin인 경우에는 member를 null로 처리하는 이유가 궁금합니다. 역할이 admin인 멤버의 예약은 존재할 수 없는건가요?

프론트 상에서 예약을 추가할 수 있는 방법은 관리자 페이지, 기본 페이지에서 추가하는 방법 2가지 입니다.
우선 admin이 관리자 페이지를 두고 기본 페이지에서 예약을 생성하는 게 좀 어색하다고 생각해서 관리자 페이지에서만 예약을 하게 구현했습니다.

관리자 페이지에서는 예약자 명을 직접 입력하는 데, 1. 이름으로 멤버를 찾아서 예약을 생성 2. 이름으로 예약을 바로 생성 (멤버 필드를 채우지 않고) 에서 저는 후자를 선택해서 관리자는 이름으로 예약을 할 수 있도록 만들었습니다 ..!

관리자가 지인 예약이나 본인 예약을 잡고싶을 때 아무 이름으로나 만들 수 있다고 생각해서 그렇게 했습니다. (이러면 멤버가 어드민에게 예약을 직접 잡아달라고 요청하면 좀 힘들어지긴 합니다 ㅠㅠ)

적다보니 빈틈이 좀 많은 것 같은데 이 부분은 나중에 시간날 때 리팩토링 해도 될까요 ? ㅠㅠ

heehunjung · 2025-04-05T14:50:26Z

src/main/java/roomescape/waiting/WaitingRepository.java

                WHERE w.member.id = :memberId
+                ORDER BY r.date ASC, rt.timeValue ASC


유추할수없다고 생각합니다!
id로 바꾸니 저 문제는 사라졌지만, update를 사용한다고 했어도 콜리 말처럼 쿼리 자체에서 빼거나 꼭 필요하다면 메서드 명에 AscByTime 처럼 추가할 것 같아요 ㅎㅎ

coli-geonwoo

망고! 2번째 리뷰 남겼습니다.

한번 더 티키타카 가보시죠!

coli-geonwoo · 2025-04-06T09:53:13Z

src/main/java/roomescape/reservation/Reservation.java

@@ -64,6 +64,14 @@ public Reservation(String name, LocalDate date, ReservationTime reservationTime,
        this.theme = theme;
    }

+    public Reservation(Member member, String name, LocalDate date, ReservationTime reservationTime, Theme theme) {


[의견 🔈 ]

도메인을 현실상황에 비추어 다양한 상황에 대비하는 자세는 좋습니다.

다만 member 없는 예약이 과연 가능한지에 대해서는 의문이 있으며, 그 멤버의 식별이 이름으로 판단되는 것 또한 오류가능성을 내포한다고 생각합니다.

예약에 정말 대상 회원이 없다면, 추후 예약자의 세부정보를 확인할 때(생년 월일, 가입일자, 성별 등등) 어떻게 확인하나요?

이름으로 회원을 식별한다고 하면 동명이인의 경우 예약자가 누구인지 어떻게 아나요?

외래키에 null이 들어가면 상황에 따라 inner join의 경우 조회되지 않을 수 있습니다. 고려하셨나요?

단순히 이런 상황을 대비했다기보다, 이 상황에 대비할 수 있는 여러 대안을 생각하고, 그 중에 가장 합리적인 대안을 선택하는 게 좋을 것 같아요. 만약 저라면 회원이 없는 예약이라는 데이터 정규성을 해치는 정책보다 어드민이 새로운 회원을 생성하는 걸 허용해주고 예약은 어느 상황에서든 회원이 있는 방식으로 정책을 정했을 것 같습니다.

이외에 망고가 다른 생각이 있다면 설득해주어도 좋습니다.

coli-geonwoo · 2025-04-06T10:00:20Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -29,10 +34,13 @@ public List<ReservationResponse> list() {
    }

    @GetMapping("/reservations-mine")
-    public ResponseEntity<List<MemberReservationResponse>> mine(@AuthMember Member member) {
-        List<MemberReservationResponse> result = reservationService.getMyReservations(member.getId());
+    public ResponseEntity<MemberReservationResponses> getMemberReservations(


[답변]

바로 답변 드리면 재미없으니 조금 힌트를 드리겠습니다.

어느 레이어에 해당 로직이 들어가야 하는지 판단이 잘 서지 않는다면 해당 정책이 서비스 정책인지 생각해보시면 좋습니다.

예약과 예약 대기를 한 화면에 같이 보여준다 <- 해당 로직은 서비스 로직인가요?

맞다면 서비스 계층에서 처리되어야 할 것이고, 아니라면 지금처럼 컨트롤러 계층에 있어도 좋을 것 같습니다. 컨트롤러가 담당해야 하는 역할과 서비스 계층이 담당해야 하는 역할을 다시한번 복기해보는 것도 좋은 경험이 될 것 같네요.

두번째로 상위 개념과 하위 개념에 대한 구분 방식에 대한 것입니다. 꼭 두 객체가 한 곳에 하나가 포함되어 있어야 상위-하위 객체라고 부르는 것은 아닙니다. 제가 정의하는 도메인의 상하위 개념은 충분조건과 필요조건의 개념과 유사합니다.
그 관점에서 상위 개념이 존재해야 하위 개념이 존재할 수 있습니다. 그럼 물어보면 다음과 같습니다.

예약이 존재해야 예약 대기가 존재할 수 있나요? 반대로 예약 대기가 존재해야 예약이 존재할 수 있나요?

그럼 이 질문으로 부터 api 관점까지 나아가면 이렇습니다. 현재 API 에서는 "예약에 관한" 나의 정보를 모두 가져와달라고 하고 있지요. 그곳에 예약과 예약대기를 모두 반환하고 있습니다. 그럼 어떤 것이 상위 개념일까요?

coli-geonwoo · 2025-04-06T10:01:29Z

src/main/java/roomescape/reservation/ReservationRepository.java

-    @Query("SELECT r FROM Reservation r join fetch r.member m WHERE m = :member")
-    List<Reservation> findAllByMember(@Param("member") Member member);
+    @Query("SELECT r FROM Reservation r join fetch r.member m WHERE m.id = :memberId")
+    List<Reservation> findAllByMemberId(long memberId);


[질문 🗣️ ]

망고가 한측의 관점을 이야기해주었으니 (합리적인 관점입니다.) 저는 이를 반대하는 측면의 관점에서 이야기해보겠습니다.

먼저, dao와 repository의 차이를 설명한 글을 통해 repository와 dao가 어떤 관점에서 다른지 생각해봅시다.

ref) https://www.baeldung.com/java-dao-vs-repository

그리고 해당 질문에 대해 답변을 내려보아요.
레포지토리는 도메인에 가까운 개념인가요? DB에 가까운 개념인가요?
id라는 것은 도메인에 가까운 개념일까요? DB 에 가까운 개념인가요?

coli-geonwoo · 2025-04-06T10:44:10Z

src/main/java/roomescape/reservation/ReservationResponse.java

    public ReservationResponse(Reservation reservation) {
        this(reservation.getId(), reservation.getName(), reservation.getThemeValue(),
-                reservation.getDate(), reservation.getTimeValue().format(TIME_FORMATTER));
+                reservation.getDate(), reservation.getTimeValue().format(Formatter.TIME_FORMATTER));


[질문 ✋ ]

그럼 다른 도메인, 예를 들어 예약자 이름의 포맷이 추가되면 모든 포맷 관련 포매터가 Formatter로 모이게 되는 건가요?

coli-geonwoo · 2025-04-06T10:47:19Z

src/test/java/roomescape/waiting/WaitingServiceTest.java

+            Waiting waiting1 = insertWaiting(brown, reservation);   // rank: 1
+            insertWaiting(manggo, reservation);                     // rank: 2
+
+            List<WaitingRankingResponse> before = waitingService.getMemberWaitings(manggo);


[제안]
private 메서드를 통해 테스트 가독성을 챙겼으나 개인적으로 given-when-then 구조가 명확히 드러나지 않는 것 같아요. 특히 중간에 들어간 assertThat 절이 그 경계를 흐릿하게 만들고 있는 것 같습니다.

assertions 를 중간에 넣어 검증한 이유는 무엇인가요?

처음에는 이전 랭킹과 그 후 랭킹을 보여주는 형태를 원했었어요.
다시보니 주석에 의존하게 되는 이상한 테스트 코드이었네요.. ㅠㅠ

현재는 불필요한 size 검증은 없애고, beforeRank를 저장한 후 현재 랭크와 비교하는 방식의 검증형태로 변경했습니다 !

coli-geonwoo · 2025-04-06T10:55:02Z

src/main/java/roomescape/reservation/Reservation.java

@@ -110,4 +110,8 @@ public List<ForStudy> getForStudies() {
    public boolean isOwner(Member member) {
        return this.member.equals(member);
    }
+
+    public boolean isMadeByAdmin() {


나중에 리팩터링하는 것은 망고의 자유입니다!

다만 여기서 논하고 싶은 것은 반영을 해주세요- 적인 차원보다 망고가 이야기한 내용들이 합리적인지, 저를 설득하기 힘들었던 부분들은 무엇인지에 대해 논하고 싶습니다.

저는 DB에 null이 저장되는 것을 좋아하지 않습니다. null은 오류 가능성을 내포하며 정규화가 제대로 되어 있지 않음을 나타내는 신호와도 같습니다. 같은 맥락에서 예약에 member가 비어있는 것을 생각해보았을 때, 외래키라는 주요한 제약조건에 null 값을 채워 넣는 것이 바람직한 것인지에 대한 의문이 있습니다.

물론 유저의 편의성을 위해 멤버가 없는 예약을 한다 는 것을 허용해준 것은 좋습니다. 다만 주인이 없는 예약은 부자연스러워요. 그 관점에서 주인을 특정하지 못하는 상황이 내포된 외래키 null 삽입이 좋은 판단일지 의문이 듭니다.

당연히 당연히 제 개인적인 의견이고요. 의견을 납득하기보다 망고가 조금 더 생각을 어필해주는 것도 좋은 리뷰 티키타카가 될 것 같습니다.

heehunjung

안녕하세요 콜리!
리뷰 및 코멘트 남겼어요 ㅎㅎ

좋은 관점을 많이 제시해주셔서 생각할 부분이 많았던 것 같아요 감사합니다 🙇

heehunjung · 2025-04-07T11:45:36Z

src/main/java/roomescape/reservation/ReservationResponse.java

    public ReservationResponse(Reservation reservation) {
        this(reservation.getId(), reservation.getName(), reservation.getThemeValue(),
-                reservation.getDate(), reservation.getTimeValue().format(TIME_FORMATTER));
+                reservation.getDate(), reservation.getTimeValue().format(Formatter.TIME_FORMATTER));


그런 의도였습니다 !

그렇게 사용할려면 여러 도메인에서 사용되기 때문에 view 패키지 위치를 roomescape 또는 global에 두는 게 좋은 방안일까요,,?

heehunjung · 2025-04-07T12:06:08Z

src/main/java/roomescape/reservation/Reservation.java

@@ -64,6 +64,14 @@ public Reservation(String name, LocalDate date, ReservationTime reservationTime,
        this.theme = theme;
    }

+    public Reservation(Member member, String name, LocalDate date, ReservationTime reservationTime, Theme theme) {


외래키에 null이 들어가면 상황에 따라 inner join의 경우 조회되지 않을 수 있습니다. 고려하셨나요?

전체 조회는 어드민 페이지에서만 하는 데, member 엔티티를 조회하고 있지 않아서 단순히 findAll로 괜찮다고 생각했었어요. (reservation의 필드에 name(유저네임)이 포함되어 있습니다!)

예약에 정말 대상 회원이 없다면, 추후 예약자의 세부정보를 확인할 때(생년 월일, 가입일자, 성별 등등) 어떻게 확인하나요?

대상 회원이 없는 예약은 어드민이 만든 예약이라 따로 세부정보를 확인할 필요가 없다고 생각했었어요.

이름으로 회원을 식별한다고 하면 동명이인의 경우 예약자가 누구인지 어떻게 아나요?

이 부분은 멤버간 확인은 가능하고 회원이 없는 예약은 위에 설명했던 것 처럼 어드민이 만든 예약이라 어드민 입장에서 식별 가능하다고 생각했어요.

멤버 입장에서는 본인 예약은 스스로 알거라 생각했구요 !

하지만 코멘트를 보고 생각해보니 member를 null로 처리한다면NullPointException의 위험성과 memberId(외래 키)만 명시적으로 쓰는 게 낫겠다라는 생각이 들었습니다.

따라서 멤버에 어드민 예약용 데이터를 미리 한 개 만들어두고 어드민 예약 시 해당 멤버를 사용하도록 변경했습니다 !

이름으로 존재하는 멤버를 조회해서 예약 생성하는 방식으로 하려면 이름을 유니크하게 설정 해야합니다.
저는 name을 실제 이름으로 생각하고 구현해서 중복을 허용하는 게 맞다 판단하여 해당 방법은 선택하지 않았어요.

heehunjung · 2025-04-07T13:13:49Z

src/main/java/roomescape/reservation/ReservationRepository.java

-    @Query("SELECT r FROM Reservation r join fetch r.member m WHERE m = :member")
-    List<Reservation> findAllByMember(@Param("member") Member member);
+    @Query("SELECT r FROM Reservation r join fetch r.member m WHERE m.id = :memberId")
+    List<Reservation> findAllByMemberId(long memberId);


레포지토리는 도메인에 가까운 개념인가요? DB에 가까운 개념인가요?

레포지토리는 여러 도메인의 DAO를 사용해 원하는 도메인 객체를 생성한다는 점에서 도메인에 가까운 개념이라고 생각해요 !

id라는 것은 도메인에 가까운 개념일까요? DB 에 가까운 개념인가요?

id는 데이터베이스에서 테이블 내 데이터를 구분하기위한 식별자로 사용하는 것이기 때문에 DB에 가깝다고 생각해요.

repository가 도메인에 가까운 계층이니 memberId보다 member 처럼 도메인 객체를 사용해서 조회하는 것이 자연스러울 수 있을 수도 있을 것 같아요.
하지만 findByMemberId, findByMemberName처럼 구현하면 특정 식별자를 기반으로 데이터를 조회한다처럼 레포지토리의 책임을 구체적으로 명시할 수 있다는 점과 repository 사용자가 어떤 것으로 db를 조회하는 지 확실히 알 수 있어 더 좋은 것 같아요 !

heehunjung · 2025-04-07T13:43:00Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -29,10 +34,13 @@ public List<ReservationResponse> list() {
    }

    @GetMapping("/reservations-mine")
-    public ResponseEntity<List<MemberReservationResponse>> mine(@AuthMember Member member) {
-        List<MemberReservationResponse> result = reservationService.getMyReservations(member.getId());
+    public ResponseEntity<MemberReservationResponses> getMemberReservations(


힌트 감사합니다 ㅎㅎ

예약과 예약 대기를 한 화면에 같이 보여준다 <- 해당 로직은 서비스 로직인가요?

같이 보여준다라는 점에서 view와 관련되었다고 생각합니다. 따라서 컨트롤러에서 처리해도 될 것 같아요 ! 명확히 나누어서 어디의 책임인지 확인하는 게 좋은 방법인 것 같네요 감사합니다. ㅎㅎ

그럼 어떤 것이 상위 개념일까요?

예약이 상위 개념이라고 생각해요.

하지만 각 도메인 별 단순 조회는 각자의 레포지토리를 사용하는 각 각의 서비스 클래스에서 수행하는 게 좋아보이기도 해요.

예약만 조회하거나 예약 대기만 조회하는 경우 새로운 서비스클래스 메서드를 만들거나 필터링 해줘야할 것 같아요.

그럴거면 나눠서 조회하는 게 코드의 중복을 줄이고 불필요한 코드를 만들지 않아도 된다는 점에서 좋은 것 같습니다 !

하지만 특정 날짜,시간과 테마에 관련된 예약, 예약 대기를 보여달라고 하면 명백하게 예약과 예약 대기가 부모 자식과 같은 관계로 연관되어 있으니
WaitingService를 ReservationService에서 호출하여 구현할 것 같기도 합니다..!

heehunjung · 2025-04-07T14:20:43Z

src/main/java/roomescape/reservation/Reservation.java

@@ -110,4 +110,8 @@ public List<ForStudy> getForStudies() {
    public boolean isOwner(Member member) {
        return this.member.equals(member);
    }
+
+    public boolean isMadeByAdmin() {


주인이 없는 예약은 없는 게 맞긴 하지만, 저는 어드민이 특정 시간, 타입의 예약을 막을 방법도 필요하다고 생각했어요.

현재 주어진 프론트 상에서 가능한 방법이 임의 예약 추가라는 방법밖에 없다고 생각하여 멤버를 null 삽입하는 방식으로 구현했습니다.

하지만, null 삽입에 대한 부작용에 대해서는 크게 생각하지 않고 로직을 작성했고, 외래키에 null 삽입이 좋지 않는 판단이라는 것에 동의 합니다 !

위의 코멘트 진행하면서 현재는 어드민 예약 시 어플리케이션 생성 시 생성 된 멤버(어드민 예약용 멤버)를 사용해서 만들도록 했어요.

그리고 실제 서비스 흐름에서는 사용자가 예약 조회 시 사용자 본인의 memberId를 기준으로 조회하고 있기 때문에,
이 '어드민 예약용 멤버'로 생성된 예약이 일반 사용자에게 노출되거나 혼동을 줄 가능성은 낮다고 판단했습니다!

heehunjung · 2025-04-07T14:30:56Z

src/test/java/roomescape/waiting/WaitingServiceTest.java

+            Waiting waiting1 = insertWaiting(brown, reservation);   // rank: 1
+            insertWaiting(manggo, reservation);                     // rank: 2
+
+            List<WaitingRankingResponse> before = waitingService.getMemberWaitings(manggo);


처음에는 이전 랭킹과 그 후 랭킹을 보여주는 형태를 원했었어요.
다시보니 주석에 의존하게 되는 이상한 테스트 코드이었네요.. ㅠㅠ

현재는 불필요한 size 검증은 없애고, beforeRank를 저장한 후 현재 랭크와 비교하는 방식의 검증형태로 변경했습니다 !

coli-geonwoo

망고! 마지막 티키타카가 되겠네요. 궁금한점 있으면 남겨주시면 감사하겠습니다!

coli-geonwoo · 2025-04-08T18:30:43Z

src/main/java/roomescape/reservation/ReservationService.java

-                .orElseThrow(() -> new RoomescapeNotFoundException("예약 시간을 찾을 수 없습니다."));
+    @Transactional
+    public ReservationResponse create(ReservationRequest reservationRequest, Member member) {
+        validatedRequest(reservationRequest);


지금이 4월 9일 새벽 3시 30분인데 과거의 예약에 성공했습니다.

앗 validateRequest 메서드에 현재 시각과 비교하는 메서드를 추가해서 해결했습니다 !

coli-geonwoo · 2025-04-08T18:34:18Z

src/main/java/roomescape/reservationTime/ReservationTimeService.java

+        return reservationTimeRepository.findAll()
+                .stream()
+                .map(ReservationTimeResponse::new)
+                .toList();
    }

    public ReservationTime save(ReservationTime reservationTime) {


시간 추가가 되지 않고 있으니 한번 확인 부탁드립니다. 개인적으로 ReservationTim을 controller request 에서도 계속 쓰시고 + timeValue라는 필드값이 API 명세와 맞지 않는 부분이 있어 binding 문제가 있으리라 생각됩니다.

맞습니다 ! 프론트쪽 json 필드 명과 api 명세가 맞지 않아서 발생하는 문제라 통일하여 해결했습니다.

작업하면서 중복 시간 추가, 잘못된 형식 입력에 대한 처리도 했습니다 !

coli-geonwoo · 2025-04-08T18:40:26Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -37,20 +37,28 @@ public ResponseEntity<List<MyReservationResponse>> mine() {
    public ResponseEntity create(@AuthMember Member member


ResponseEntity가 raw type이네요! 제네릭을 통해 명시적으로 반환값을 명시해줍시다!

앗 반영할게요 ㅎㅎ

coli-geonwoo · 2025-04-08T18:43:32Z

src/main/java/roomescape/reservation/ReservationController.java

-        ReservationResponse result = reservationService.save(reservationRequest);
+
+        ReservationResponse result = getReservationResponse(member,
+                reservationRequest, isAdmin);


[질문 ✋ ]
여기에서도 의문이 있는데 꼭 name이 reservationRequest로부터 나와야 하는건가요? name의 nullable을 조사하고 없을 경우 member의 name으로 update 하는게 아니라, 그냥 member로 부터만 name을 가져다쓰고 request의 name은 사용하지 않으면 되지 않을까 싶은데 update를 통해 request로부터 name을 가져오도록 강제한 이유가 있나요?

아...
미션 요구사항에 request의 name이 없는 경우에 member의 name을 사용하라 가 있었는 데 거기에 빠져서 불필요한 로직을 작성했네요.

리뷰대로 member의 name을 사용하는 방식으로 수정했습니다 ~ !

coli-geonwoo · 2025-04-08T19:06:52Z

src/main/java/roomescape/reservation/ReservationResponse.java

    public ReservationResponse(Reservation reservation) {
        this(reservation.getId(), reservation.getName(), reservation.getThemeValue(),
-                reservation.getDate(), reservation.getTimeValue().format(TIME_FORMATTER));
+                reservation.getDate(), reservation.getTimeValue().format(Formatter.TIME_FORMATTER));


그렇기보다 그 의도가 좋은 방향인지 재고를 요청드리고 싶습니다.

전역 상수화는 정말 도메인 곳곳에서 사용될만큼 범용적이고 명확한 도메인룰을 규정하는데 사용됩니다. 다만 특정 로직에서만 사용되는 포맷팅이나 변경 사항이 많은 경우, 해당 도메인에 두고 사용하는 것이 일반적입니다.

예를 들어 이 경우에는 여러 곳에서 시간이 같은 형식으로 포맷팅되기 때문에 전역 상수화를 생각하신 것 같습니다. 다만 응답 중 한 곳에서 시간 포맷팅을 다르게 해야 한다면, 그 포매터는 상수화하실 건가요, 아니면 해당 dto 객체에 선언하실건가요?

만약 꼭 중복부분을 없애고 싶다면, 직접 역직렬화를 커스텀하는 Jsondeserialzier를 선언해보는 것도 방법이 될 수 있습니다.

@JsonComponent public class TimeSerializer extends JsonSerializer<LocalTime> { private static final DateTimeFormatter TIME_FORMATTER = new DateTimeFormatterBuilder() .appendPattern("HH:mm") .toFormatter() .withZone(ZoneId.of("Asia/Seoul")); @Override public void serialize( LocalTime time, JsonGenerator jsonGenerator, SerializerProvider serializerProvider ) { try { jsonGenerator.writeString(TIME_FORMATTER.format(time)); } catch (IOException exception) { throw new RuntimeException("시간 변환 과정에서 문제가 발생했습니다."); } } }

coli-geonwoo · 2025-04-08T19:11:40Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -29,10 +34,13 @@ public List<ReservationResponse> list() {
    }

    @GetMapping("/reservations-mine")
-    public ResponseEntity<List<MemberReservationResponse>> mine(@AuthMember Member member) {
-        List<MemberReservationResponse> result = reservationService.getMyReservations(member.getId());
+    public ResponseEntity<MemberReservationResponses> getMemberReservations(


예약과 예약 대기를 한 화면에 같이 보여준다 -> 해당 로직은 서비스로직입니다.

왜인지에 대해 생각해보죠. 아마 망고는 '보여준다'라는 키워드가 들어갔기에 뷰라고 판단한 것 같아요. 다만 무엇을 보여주냐?와 어떻게 보여주나? 는 다른 개념입니다. 그리고 무엇을 보여주냐? 는 서비스 정책인 반면, 그렇게 서비스적으로 보여주기로 결정된 것을 어떻게 보여주나? 라는 개념이 뷰와 가깝다고 생각하면 되겠습니다.

예를 들어 보겠습니다. 예약과 예약대기를 같이 보여준다는 서비스 정책이 결정되면, 해당 정책을 뷰에 그릴 때 시간은 어떤 형식으로 보여줄지 결정해야 합니다. 이런 형식, 모양 등이 뷰에 대한 논의입니다.

그럴거면 나눠서 조회하는 게 코드의 중복을 줄이고 불필요한 코드를 만들지 않아도 된다는 점에서 좋은 것 같습니다 !

이부분은 합리적이라 생각되어 납득되었습니다 👍

heehunjung

안녕하세요 콜리 !

마지막까지 좋은 리뷰 감사합니다 🙇
생각할만한 것들을 계속 질문해주셔서 도움이 많이되었던 것 같아요 ㅎㅎ

코멘트 및 반영했습니다 !

heehunjung · 2025-04-10T14:55:54Z

src/main/java/roomescape/reservation/ReservationService.java

-                .orElseThrow(() -> new RoomescapeNotFoundException("예약 시간을 찾을 수 없습니다."));
+    @Transactional
+    public ReservationResponse create(ReservationRequest reservationRequest, Member member) {
+        validatedRequest(reservationRequest);


앗 validateRequest 메서드에 현재 시각과 비교하는 메서드를 추가해서 해결했습니다 !

heehunjung · 2025-04-11T11:10:15Z

src/main/java/roomescape/reservation/ReservationResponse.java

    public ReservationResponse(Reservation reservation) {
        this(reservation.getId(), reservation.getName(), reservation.getThemeValue(),
-                reservation.getDate(), reservation.getTimeValue().format(TIME_FORMATTER));
+                reservation.getDate(), reservation.getTimeValue().format(Formatter.TIME_FORMATTER));


예를 들어 이 경우에는 여러 곳에서 시간이 같은 형식으로 포맷팅되기 때문에 전역 상수화를 생각하신 것 같습니다.

맞습니다..!

다만 응답 중 한 곳에서 시간 포맷팅을 다르게 해야 한다면, 그 포매터는 상수화하실 건가요, 아니면 해당 dto 객체에 선언하실건가요?

변경에 대해 생각하지 않고 단순히 중복되서 같이 쓰고자 전역변수화를 했어요.
위와 같이 요구사항에 따라 많이 변경될 수 있고 특정 도메인에서만 사용하기때문에 dto 마다 필요한 formatter를 위치하여 사용하는 것이 좋아보입니다 !
그렇게 수정할게요 ㅎㅎ

dto 별로 두는 것이 인지적 측면에서도 좋아보여요 !

heehunjung · 2025-04-11T11:47:00Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -29,10 +34,13 @@ public List<ReservationResponse> list() {
    }

    @GetMapping("/reservations-mine")
-    public ResponseEntity<List<MemberReservationResponse>> mine(@AuthMember Member member) {
-        List<MemberReservationResponse> result = reservationService.getMyReservations(member.getId());
+    public ResponseEntity<MemberReservationResponses> getMemberReservations(


오.. 단순히 보여준다라고 생각해서 뷰 관련 로직이라고 생각했었는 데, 합치는 행위는 (무엇을 보여주는지)는 서비스 정책으로 보면 되는군요 ㅎㅎ 헷갈리는 부분이 좀 명확해진 것 같아요.

해당 코드들이 서비스 로직이여서 컨트롤러에 노출하는 것을 막기위해
예약, 대기를 각 각 조회하는 서비스클래스 메서드들을 유지하고, ReservationService에 두 메서드를 호출하여 합치는 메서드를 만들습니다 !

heehunjung · 2025-04-11T12:13:09Z

src/main/java/roomescape/reservationTime/ReservationTimeService.java

+        return reservationTimeRepository.findAll()
+                .stream()
+                .map(ReservationTimeResponse::new)
+                .toList();
    }

    public ReservationTime save(ReservationTime reservationTime) {


맞습니다 ! 프론트쪽 json 필드 명과 api 명세가 맞지 않아서 발생하는 문제라 통일하여 해결했습니다.

작업하면서 중복 시간 추가, 잘못된 형식 입력에 대한 처리도 했습니다 !

heehunjung · 2025-04-11T12:13:20Z

src/main/java/roomescape/reservation/ReservationController.java

@@ -37,20 +37,28 @@ public ResponseEntity<List<MyReservationResponse>> mine() {
    public ResponseEntity create(@AuthMember Member member


앗 반영할게요 ㅎㅎ

heehunjung · 2025-04-11T12:23:29Z

src/main/java/roomescape/reservation/ReservationController.java

-        ReservationResponse result = reservationService.save(reservationRequest);
+
+        ReservationResponse result = getReservationResponse(member,
+                reservationRequest, isAdmin);


아...
미션 요구사항에 request의 name이 없는 경우에 member의 name을 사용하라 가 있었는 데 거기에 빠져서 불필요한 로직을 작성했네요.

리뷰대로 member의 name을 사용하는 방식으로 수정했습니다 ~ !

heehunjung added 17 commits April 1, 2025 00:01

refactor: reservation에 member n:1 연관 관계 추가

74cd2f8

refactor: ReservationTimeResponse(dto) 추가

9d34605

refactor: 요구사항에 따라 예약 생성 변경

8714a9e

refactor: dto, record로 변경

fd60ee1

feat: 내 예약 조회 기능 구현

9d2994a

test: step6 테스트 코드 추가

de76662

refactor: memberId 조회로 변경

744b73b

feat: waiting 엔티티 클래스 생성

a797ab1

feat: 대기 생성 기능 추가

92a1bbb

refactor: my -> member 이름 변경

3f9478c

refactor: 대기 기능 수정

74441f6

프론트와 데이터 형태 맞춤, 동일한 인원이 예약과 대기 한번에 못하게

refactor: 중복 예약 금지

3b0b697

refactor: admin이 추가한 예약(member == null) 인 경우 사용자가 대기를 만들 수 있도록 변경

27edf22

refactor: 시간순 정렬 + fetch join 추가 및 create에 transaction 추가

ff2ddfe

test: waitingService 관련 테스트 작성

8f8e068

test: reservationService, create 관련 테스트 추가

230d27e

refactor: waiting 생성 시 대기 순서 관련 수정

84895ce

coli-geonwoo suggested changes Apr 3, 2025

View reviewed changes

heehunjung added 9 commits April 5, 2025 11:38

chore: 학습용 코드 제거

1f34029

refactor: 비즈니스 로직 위치 service로 이동

4e3438e

refactor: 서비스 클래스 내부 private 메서드 파라미터의 범위를 줄여 재사용성과 변경의 여파를 최소화

7d94f89

refactor: 불필요한 Long -> long으로 변경

b9a6fd0

study: sql 확인

8fc2a6d

refactor: fetchType = lazy 추가

b02cdc7

chore: 조금 더 의미있는 변수명으로 변경

1707d48

refactor: 예약 대기 관련 로직 수정

e2b2720

기존에는 reservaionId로 조회하면서 Optional로 받아서 List로 변경하고 받은 데이터로 대기 내역 조회 후 예외 처리 또는 새로운 대기 생성으로 변경

refactor: 1L 제거 -> FRONT로 던짐

82989ca

heehunjung commented Apr 5, 2025

View reviewed changes

coli-geonwoo suggested changes Apr 6, 2025

View reviewed changes

feat: 커스텀 서버 에러 추가

81db9c9

heehunjung added 2 commits April 7, 2025 21:12

refactor: admin 예약 방식 변경

63a161f

refactor: test 가독성을 위한 수정

04f7dcf

heehunjung commented Apr 7, 2025

View reviewed changes

coli-geonwoo suggested changes Apr 8, 2025

View reviewed changes

heehunjung and others added 7 commits April 10, 2025 23:54

refactor: 현재 시간 이전 예약 금지

520c54d

refactor: day까지 고려

8df0824

refactor: 전역상수화된 formatter -> 각 dto 내부로 이동

253b2ff

refactor: 컨트롤러에 작성된 서비스로직 코드 위치 변경

c5e4a00

refactor: ReservationTime 생성 관련 오류 처리 및 중복 예약 시간 추가, 이상한 시간 추가 처리 추가

a4e6d87

chore: 타입 명시

82141f1

refactor: 불필요한 로직 제거

8dd6439

heehunjung commented Apr 11, 2025

View reviewed changes

boorownie merged commit 6d1e2ac into next-step:heehunjung Apr 14, 2025

		WHERE w.member.id = :memberId
		ORDER BY r.date ASC, rt.timeValue ASC

		, String time, String status) {

		public static final String WAITING_STATUS_MESSAGE = "%d번째 예약대기";

		@@ -37,20 +37,28 @@ public ResponseEntity<List<MyReservationResponse>> mine() {
		public ResponseEntity create(@AuthMember Member member

[Spring Data JPA] 망고 5, 6단계 미션 제출합니다. #147

[Spring Data JPA] 망고 5, 6단계 미션 제출합니다. #147

Conversation

heehunjung commented Apr 2, 2025 • edited Loading

PR - STEP 5,6

구현 사항

고려한 부분

coli-geonwoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heehunjung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equals

hashCode

함께 Overriding

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coli-geonwoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heehunjung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coli-geonwoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heehunjung commented Apr 2, 2025 •

edited

Loading