Skip to content
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

[Spring JDBC] 망고 step8,9,10 미션 제출합니다. #428

Open
wants to merge 36 commits into
base: heehunjung
Choose a base branch
from

Conversation

heehunjung
Copy link

안녕하세요 짱수! 2주간 잘 부탁드려요 😄

PR

STEP 8,9,10

구현 사항

ReservationTIme 클래스 추가

  • 시간 관련 crd 작성

Reservation, ReservationTime 연관 관계 생성에 따른 전체적인 코드 리팩토링

레이어드 아키텍처를 적용하여 레이어별 책임과 역할에 따라 클래스 분리

고민한 부분

비즈니스 플로우 책임은 서비스가 가지도록 위임해보세요.
비즈니스 규칙 책임은 도메인이 가지도록 위임해보세요.

step 10 힌트를 보고 생각해보았을 때,

비즈니스 플로우 책임비즈니스 로직 실행 중 요구사항에 맞지않은 값이 전달되거나 생성되는 경우 ( 과거 시간을 예약하려는 경우) 과 같은 것을 생각했어요.
비즈니스 규칙 책임 에도 위에 적은 내용이 포함된다고 생각은 했었어요. 하지만, 코드를 작성하다보니 비즈니스 로직 진행 과정에서 비즈니스 로직 실행 중 요구사항에 맞지않은 값을 처리하는 경우가 많아 서비스 계층에 작성했어요. 그래서 도메인과 DTO에서는 기본적인 NULL 값이나 문자열 길이같은 최소한의 규칙만 책임지도록 했습니다.
정답은 없겠지만, 적절하게 나눈 것인지 헷갈리네요..!

아직 큰 프로젝트를 진행해보지 않아서 계층을 나누는 데, 미숙한 부분이 많은 것 같습니다.

감사합니다~~!

Copy link

@zangsu zangsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비즈니스 플로우 책임 은 비즈니스 로직 실행 중 요구사항에 맞지않은 값이 전달되거나 생성되는 경우 ( 과거 시간을 예약하려는 경우) 과 같은 것을 생각했어요.
비즈니스 규칙 책임 에도 위에 적은 내용이 포함된다고 생각은 했었어요. 하지만, 코드를 작성하다보니 비즈니스 로직 진행 과정에서 비즈니스 로직 실행 중 요구사항에 맞지않은 값을 처리하는 경우가 많아 서비스 계층에 작성했어요. 그래서 도메인과 DTO에서는 기본적인 NULL 값이나 문자열 길이같은 최소한의 규칙만 책임지도록 했습니다.
정답은 없겠지만, 적절하게 나눈 것인지 헷갈리네요..!

잘 고민해 주셨네요. 이번 미션의 핵심 주제이니만큼 리뷰를 진행하면서 관련 내용에 대해서도 차차 다뤄 보시죠!
코멘트 확인해 주세요~~

@@ -1,7 +1,10 @@
package roomescape.domain.reservation.domain;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

패키지 구조가 domain 하위에 다시 domain이 이 들어있군요??
망고는 어떤 기준으로 패키지를 구분하나요??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

domain 패키지에 우선 비즈니스 로직에 필수적인 객체들로 패키지를 구성해요.
그 내부에는 controller, service, repository 와 같은 패키지를 두고 domain 패키지는 해당 객체를 표현하기 위한 클래스를 위해 구분해두었어요.

global 패키지는 프로젝트 전체적으로 사용하는 예외에 관련한 코드라던지 configuration 관련 코드가 있으면 추가하는 편입니다!

view 패키지는 커찬과의 리뷰에서 html을 반환하는 컨트롤러와 json 을 반환하는 컨트롤러가 구분되면 좋을 것 같다라는 리뷰를 받고 동의하여 추가했어요.

사실 처음 공부할 때 봤던 예제 코드를 바탕으로 조금씩 개인적 의견을 추가하여 사용했던 것이라 패키지를 구분하는 기준을 이번 리뷰 안에 정해보고 코멘트 남겨볼게요 !

Comment on lines 19 to 21
public static Reservation newWithoutId(final String name, final LocalDate date,
final ReservationTime reservationTime) {
return new Reservation(null, name, date, reservationTime);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 메서드를 만든 이유는 무엇인가요?

+) 해당 정적 팩토리 메서드와 id 를 가지지 않는 생성자의 차이점은 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 정적 팩토리 메서드와 id 를 가지지 않는 생성자의 차이점은 무엇인가요?

메서드 명에 의도를 제공할 수 있다는 것 말고는 현재 정적 팩토리 메서드에서는 차이점이 없습니다..

해당 메서드를 만든 이유는 무엇인가요?

위 이유로 만들었었습니다.
하지만 커찬과 리뷰를 나누는 과정에서 객체 생성할 때 보통 new 키워드를 사용하여 객체를 생성하는 데, 정적 팩토리 메서드를 통해 객체를 생성하면 모든 개발자가 인지를 해야한다는 큰 단점이 있는 것을 알게되었어요.
계속 고민해봤는데, id를 포함하지 않는 생성자를 사용하는 것이 좋아보여 수정할게요 !

@@ -11,6 +11,6 @@ public record ReservationResponse(Long id, String name, LocalDate date, String t
public static ReservationResponse fromReservation(final Reservation reservation) {

return new ReservationResponse(reservation.getId(), reservation.getName(), reservation.getDate(),
TIME_FORMATTER.format(reservation.getTime()));
TIME_FORMATTER.format(reservation.getReservationTime().getTime()));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디미터 법칙에 대해 알아볼까요??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디미터의 법칙 은 객체의 노출을 최대한 줄이기 위해 객체를전달하기보다는 해당 객체에 메세지를 보내 동작을 하도록 하게 하는 것입니다!
디미터의 법칙을 위반하게되면 객체간의 결합도가 높아지고 객체의 내부 구조가 외부로 노출된다는 단점이 있습니다.

그에따라 코드 수정해뒀습니다~!

if (name == null || name.isBlank() || date == null || time == null) {
throw new RoomescapeBadRequestException("잘못된 예약 데이터입니다: name/date/time이 null이거나 빈 값입니다.");
if (name == null || name.isBlank() || date == null) {
throw new RoomescapeBadRequestException("잘못된 예약 데이터입니다.");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외 메시지가 수정되었네요? 어떤 이유로 수정하였을까요??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외 메세지가 너무 구체적이면 해당 API를 악용하여 호출할 가능성이 있다는 리뷰를 받고 수정해봤는 데, 다시 생각해보니.. name/date/time이 null이거나 빈 값입니다. 은 악용할 여지가 없고 클라이언트 요청이 어떤 문제가 있는 지 잘 제공하는 것 같아 다시 수정할게요 !

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 고민해 주셨네요!
+) 우리의 클라이언트는 개발자가 아닐 경우가 더 많아요. 일반 사용자에게 해당 예외 메시지는 어떤 의미를 전달할 수 있을까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음.. 일반 사용자 입장에서는 모호할 것 같아요.
지금은 @NotNull 같은 애노테이션을 사용해서 request의 검증을 하기 때문에 해당 예외 메세지는 사라졌지만, 만약 예외 메세지를 작성한다면

잘못된 예약 데이터입니다. 이름 날짜 시간 항목은 필수입니다. 다 정도로 null 같은 키워드는 빼고 적을 것 같아요 !

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금은 @NotNull 같은 애노테이션을 사용해서 request의 검증을 하기 때문에 해당 예외 메세지는 사라졌지만

그럼 지금은 어떤 예외 메시지가 사용되고 있나요?? @NotNull 을 사용하면 예외메시지를 사용하지 않아도 되나요?

+ " r.date, \n"
+ " t.id as time_id, \n"
+ " t.time as time_value \n"
+ "FROM reservation as r inner join reservationTime as t on r.time_id = t.id";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

join 의 종류에는 어떤 것들이 있고, 어떤 차이가 있나요?
이번 조회 쿼리에서 inner join을 사용한 이유는 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inner join

  • join 하는 테이블에서 조인 조건을 만족하는 행만 반환
    left join
  • 왼쪽 테이블의 모든 행을 반환하고 조건을 만족하는 오른쪽 테이블의 행만 반환 (일치하는 값이 없으면 null)
    right join
  • 오른쪽 테이블의 모든 행을 반환하고 조건을 만족하는 왼쪽 테이블의 행만 반환 (일치하는 값이 없으면 null)
    full outer join
  • left join과 right join을 같이 사용하는 형태, 두 테이블의 모든 행을 반환 (일치하는 값이 없으면 null), 대부분의 dbms에서 제공하지 않아 left join과 right join을 이용하여 사용

이번 조회 쿼리에서 inner join을 사용한 이유는 무엇인가요?

이번 조회 쿼리에서 t on r.time_id = t.id 조건이 있고, 코드상에서 reservationTime이 reservation 생성 시 반드시 포함되도록 구현했기 때문에 left join이나 right join을 사용해도 결과는 같습니다.
하지만 left join을 사용한다고 하면 **"reservationTime이 없는 reservation도 조회하는가?"**라는 오해를 일으킬 수 있습니다(right join도 동일).
이에따라 inner join을 사용했어요 !

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 학습해 주셨네요~ 💯

return ReservationTimeResponse.fromReservationTime(newReservationTime);
}

@Transactional(readOnly = true)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 메서드에서 Transactional 어노테이션은 왜 필요한가요?
readOnly = true 어노테이션은 왜 필요한가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단일 select 쿼리만 사용하는 조회용 메서드라서 Transactional 기능이 꼭 필요하지는 않는 것 같아요.

readOnly = true 어노테이션은 왜 필요한가요?

데이터베이스에 따라 조회 시 최적화를 해주고 CUD 동작을 막아주는데, H2 데이터베이스에서는 최적화도 미미하고 readOnly = true 어노테이션를 무시한다고 확인했어요. 하지만 해당 애노테이션이 붙어있으면 단순 조회용 메서드라고 인식하기 쉬울 것 같아 붙였어요 !

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단순 조회용 메서드라는 것은 getXXX() 메서드 명 만으로도 충분히 알 수 있지 않나요??
다른 누군가 조회 메서드 내부에서 데이터를 변경하지 않기를 원한다면 꼭 코드에 명시하기보다 팀 내부 컨벤션을 만드는 것도 방법이 될 수 있어요.

readOnly는 DB 부하 분산을 위해 사용되기도 하는데, 지금 단계에서 다루기엔 조금 깊은 내용같네요.
아무튼 제 개인적인 의견은 @Transactional도, readOnly 속성도 불필요하게 느껴지긴 합니다.
아직은 꼭 지워야 하는 정도의 코드는 아니기에 선택은 망고가 해주세요~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메소드 명으로 충분히 알 수 있는 부분 동의합니다 !
제가 찾아본 내용도 h2 db에서는 @Transactional (readOnly = True) 가 필요없는 것 같아 지우겠습니당.

return reservationTimeRepository.findAll()
.stream()
.map(ReservationTimeResponse::fromReservationTime)
.collect(Collectors.toList());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JDK 16부터는 아래와 같이 사용이 가능해요!

Suggested change
.collect(Collectors.toList());
.toList();

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 ! 감사합니다
반영했어요 ~~

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여전히 반영이 안된 부분이 있어보이네요~~ 👀
mac + intelliJ 를 사용한다면 cmd + shift + R 단축키로 변경해야 하는 부분을 모두 탐색하거나 일괄 변경할 수 있답니다~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 ㅠㅠ 명령어 잘 사용할게요 !!

안된 부분도 반영했습니다 ~!

}

@ExceptionHandler(Exception.class)
public ResponseEntity handleException(Exception exception) {
public ResponseEntity handleException(final Exception exception) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인텔리제이에는 파라미터에 final을 자동으로 붙여주는 기능이 있다고 해요~
저는 파라미터에 final을 붙이는 것을 선호하는 편은 아니라 잘 모르지만 망고에게는 도움될 것 같네요~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다 ! 참고해서 사용할게요 ㅎㅎ

name VARCHAR(20) NOT NULL,
date DATE NOT NULL,
time TIME NOT NULL,
CREATE TABLE reservationTime
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sql 파일도 포맷팅을 해 주면 더 깔끔하게 볼 수 있어요~
인텔리제이를 사용한다면 mac 기준 cmd + opt + L 로 포매팅이 가능합니다

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다!

@@ -1,11 +1,14 @@
package roomescape;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

실패하는 테스트가 몇 존재하는군요.
테스트는 모두 성공시키고 다시 미션을 제출해주세요. 실패하는 테스트를 리뷰하기는 어렵겠네요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아.. 수정 시 오류가 있었는데, 테스트 코드를 확인하지 못했네요 죄송합니다..

수정했습니다 !

Copy link
Author

@heehunjung heehunjung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 짱수 ! 리뷰 감사합니다 🙇
코멘트 및 반영 했어요 !


테스트 코드에 실수가 있었네요 ..
변명이지만 이유를 적어보자면
if (name == null || name.isEmpty() || name.getBytes(StandardCharsets.UTF_8).length < 256 || reservationTime == null)

reservation 생성자에 해당 제약조건을 추가할 때 부등호 방향을 반대로하고 테스트를 안돌리고 push를 했어요. 테스트 코드의 중요성을 한번 더 알게된 것 같아요 ..

시간써서 리뷰해주시는 데, 좋은 코드 리뷰작업이 될 수 있도록 노력할게요 !
감사합니다.

@@ -1,7 +1,10 @@
package roomescape.domain.reservation.domain;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

domain 패키지에 우선 비즈니스 로직에 필수적인 객체들로 패키지를 구성해요.
그 내부에는 controller, service, repository 와 같은 패키지를 두고 domain 패키지는 해당 객체를 표현하기 위한 클래스를 위해 구분해두었어요.

global 패키지는 프로젝트 전체적으로 사용하는 예외에 관련한 코드라던지 configuration 관련 코드가 있으면 추가하는 편입니다!

view 패키지는 커찬과의 리뷰에서 html을 반환하는 컨트롤러와 json 을 반환하는 컨트롤러가 구분되면 좋을 것 같다라는 리뷰를 받고 동의하여 추가했어요.

사실 처음 공부할 때 봤던 예제 코드를 바탕으로 조금씩 개인적 의견을 추가하여 사용했던 것이라 패키지를 구분하는 기준을 이번 리뷰 안에 정해보고 코멘트 남겨볼게요 !

import roomescape.domain.reservation.dto.ReservationRequest;
import roomescape.domain.reservation.dto.ReservationResponse;
import roomescape.domain.reservation.service.ReservationService;

@Controller
@RestController
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • html을 전달하는 컨트롤러와 json을 반환하는 컨트롤러를 분리해두었고, json 이외의 값을 반환하는 컨트롤러가 필요하다면 html, json 을 반환하는 컨트롤러를 분리했던 것처럼 분리해서 사용할 가능성이 높다 생각했습니다.
  • ResponseEntity를 사용하고 있지만 @RestController 를 사용하면 해당 컨트롤러의 목적(json을 전달하는)이 더 잘보일 것이라 생각했습니다.

위 의견으로 변경했어요 !

@@ -11,6 +11,6 @@ public record ReservationResponse(Long id, String name, LocalDate date, String t
public static ReservationResponse fromReservation(final Reservation reservation) {

return new ReservationResponse(reservation.getId(), reservation.getName(), reservation.getDate(),
TIME_FORMATTER.format(reservation.getTime()));
TIME_FORMATTER.format(reservation.getReservationTime().getTime()));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디미터의 법칙 은 객체의 노출을 최대한 줄이기 위해 객체를전달하기보다는 해당 객체에 메세지를 보내 동작을 하도록 하게 하는 것입니다!
디미터의 법칙을 위반하게되면 객체간의 결합도가 높아지고 객체의 내부 구조가 외부로 노출된다는 단점이 있습니다.

그에따라 코드 수정해뒀습니다~!

+ " r.date, \n"
+ " t.id as time_id, \n"
+ " t.time as time_value \n"
+ "FROM reservation as r inner join reservationTime as t on r.time_id = t.id";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inner join

  • join 하는 테이블에서 조인 조건을 만족하는 행만 반환
    left join
  • 왼쪽 테이블의 모든 행을 반환하고 조건을 만족하는 오른쪽 테이블의 행만 반환 (일치하는 값이 없으면 null)
    right join
  • 오른쪽 테이블의 모든 행을 반환하고 조건을 만족하는 왼쪽 테이블의 행만 반환 (일치하는 값이 없으면 null)
    full outer join
  • left join과 right join을 같이 사용하는 형태, 두 테이블의 모든 행을 반환 (일치하는 값이 없으면 null), 대부분의 dbms에서 제공하지 않아 left join과 right join을 이용하여 사용

이번 조회 쿼리에서 inner join을 사용한 이유는 무엇인가요?

이번 조회 쿼리에서 t on r.time_id = t.id 조건이 있고, 코드상에서 reservationTime이 reservation 생성 시 반드시 포함되도록 구현했기 때문에 left join이나 right join을 사용해도 결과는 같습니다.
하지만 left join을 사용한다고 하면 **"reservationTime이 없는 reservation도 조회하는가?"**라는 오해를 일으킬 수 있습니다(right join도 동일).
이에따라 inner join을 사용했어요 !

Comment on lines 61 to 63
if (!reservationRepository.remove(id)) {
throw new RoomescapeServerException();
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

첫번째 요청은 성공하지만 그 뒤 요청들은 계속 예외가 발생합니다. 그래서 사용자가 삭제가 되었음에도 예외 메세지를 받아 혼란이 올 것 같아요.
따라서 삭제하려는 예약이 없는 경우에도 정상적으로 처리하면 이런 문제점이 사라지네요!
반영하겠습니다.

}
}

public ReservationTime toReservationTime() {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다 !
네이밍 컨벤션 주의할게요. 👍

}

@ExceptionHandler(Exception.class)
public ResponseEntity handleException(Exception exception) {
public ResponseEntity handleException(final Exception exception) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다 ! 참고해서 사용할게요 ㅎㅎ

return reservationTimeRepository.findAll()
.stream()
.map(ReservationTimeResponse::fromReservationTime)
.collect(Collectors.toList());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 ! 감사합니다
반영했어요 ~~

return ReservationTimeResponse.fromReservationTime(newReservationTime);
}

@Transactional(readOnly = true)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단일 select 쿼리만 사용하는 조회용 메서드라서 Transactional 기능이 꼭 필요하지는 않는 것 같아요.

readOnly = true 어노테이션은 왜 필요한가요?

데이터베이스에 따라 조회 시 최적화를 해주고 CUD 동작을 막아주는데, H2 데이터베이스에서는 최적화도 미미하고 readOnly = true 어노테이션를 무시한다고 확인했어요. 하지만 해당 애노테이션이 붙어있으면 단순 조회용 메서드라고 인식하기 쉬울 것 같아 붙였어요 !

@@ -1,11 +1,14 @@
package roomescape;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아.. 수정 시 오류가 있었는데, 테스트 코드를 확인하지 못했네요 죄송합니다..

수정했습니다 !


public class ReservationTime {

private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("HH:mm");
Copy link

@zangsu zangsu Mar 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금의 구조는 domain 이 출력 형식을 알고 있는 상황이네요.

  1. MVC 패턴에서 domain 은 출력 형식 등 view 와 관련된 정보를 몰라야 해요. 왜 그런지 학습 & 고민을 해 보고 이유를 알려주세요.
  2. 디미터 법칙을 조금 더 깊이 학습해 보고, 디미터 법칙이 무엇인지를 좀 더 자세히 설명해 주세요.
    • 객체간의 결합도가 높아지고 객체의 내부 구조가 외부로 노출되는 것이 왜 단점인가요? 어떤 문제가 있나요?

Copy link
Author

@heehunjung heehunjung Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MVC 패턴에서 domain 은 출력 형식 등 view 와 관련된 정보를 몰라야 해요. 왜 그런지 학습 & 고민을 해 보고 이유를 알려주세요.

MVC 패턴은 관심사의 분리를 통해 구성 요소들을 분리해요. 이를 통해 분리된 요소가 자신의 역할에 집중하도록 하여 코드의 유지보수성, 확장성, 재사용성을 높이는 것이 목적이에요.
domain에서 view와 관련된 정보를 알게된다면 view와 결합도가 증가하게 됩니다. view 관련 수정 시 domain 쪽 코드도 수정해야 되고, 해당 domain 의 다양한 형태를 view에서 요구하면 domain 코드가 비대해지는 단점이 있어요.
반면, 역할을 분리하면 domain이 비즈니스 로직에만 집중할 수 있어 코드가 간결해지고, View는 출력 형식을 자유롭게 변경할 수 있는 장점이 생기기 때문에, domain이 View와 관련된 정보를 몰라야 하는 것이 좋습니다 !

디미터 법칙을 조금 더 깊이 학습해 보고, 디미터 법칙이 무엇인지를 좀 더 자세히 설명해 주세요.

디미터 법칙의 핵심 개념은 객체 구조의 경로를 따라 멀리 떨어져 있는 객체에 메세지를 보내는 설계를 피하라는 것입니다. 따라서 디미터 법칙은 최소 지식 원칙이라고도 불립니다!
디미터 법칙에 따르면, 한 객체는 다음에 해당하는 메서드를 호출함으로써 객체 간 결합도를 효과적으로 낮출 수 있습니다.

  1. 객체 자신의 메서드
  2. 메서드의 파라미터로 전달된 객체의 메서드
  3. 메서드 내에서 생성하거나 초기화한 객체의 메서드
  4. 객체가 인스턴스 변수로 직접 소유한 객체의 메서드
    이러한 범위를 준수함으로써 디미터의 법칙을 지킬 수 있습니다 !

객체간의 결합도가 높아지고 객체의 내부 구조가 외부로 노출되는 것이 왜 단점인가요? 어떤 문제가 있나요?

객체간의 결합도가 높아지면 하나의 객체에서 변경이 발생했을 때
다른 객체에도 영향을 끼치게 되고, 다른 상황에서 재사용하기 어려워집니다. 또한 객체에 독립적인 테스트 작성이 어려워집니다.

객체의 내부 구조가 외부로 노출된다면 다른 객체가 해당 객체의 내부 구현에 의존하게 될 가능성이 높아집니다. 따라서 해당 객체의 내부 구현이 변경되면 이를 사용하는 다른 코드도 다 변경해야 됩니다. 객체 내부 상태가 외부로 노출되어 외부에서 직접 변경될 가능성도 생겨 불변성 보장에도 어려움이 생기는 문제가 있습니다!

Copy link

@zangsu zangsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 반영하느라 수고하셨어요!!
조금 더 학습, 또는 개선할 부분을 남겨 두었으니 확인해 주세요~
+) 어렵거나 이해가 안되는 부분들이 있다면 편하게 DM 주세요~

Copy link
Author

@heehunjung heehunjung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 리뷰 감사합니다 !
덕분에 조금 더 생각해볼 지점이 많았던 것 같아요.
코멘트 및 반영했습니다 😄

import roomescape.domain.reservation.dto.ReservationRequest;
import roomescape.domain.reservation.dto.ReservationResponse;
import roomescape.domain.reservation.service.ReservationService;

@Controller
@RestController
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResponseEntityHttpEntity 를 상속받은 클래스로 빌더를 사용하여 간단하게 body이외에도 status나 header를 쉽게 추가할 수 있어서 사용했어요 !


public class ReservationTime {

private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("HH:mm");
Copy link
Author

@heehunjung heehunjung Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MVC 패턴에서 domain 은 출력 형식 등 view 와 관련된 정보를 몰라야 해요. 왜 그런지 학습 & 고민을 해 보고 이유를 알려주세요.

MVC 패턴은 관심사의 분리를 통해 구성 요소들을 분리해요. 이를 통해 분리된 요소가 자신의 역할에 집중하도록 하여 코드의 유지보수성, 확장성, 재사용성을 높이는 것이 목적이에요.
domain에서 view와 관련된 정보를 알게된다면 view와 결합도가 증가하게 됩니다. view 관련 수정 시 domain 쪽 코드도 수정해야 되고, 해당 domain 의 다양한 형태를 view에서 요구하면 domain 코드가 비대해지는 단점이 있어요.
반면, 역할을 분리하면 domain이 비즈니스 로직에만 집중할 수 있어 코드가 간결해지고, View는 출력 형식을 자유롭게 변경할 수 있는 장점이 생기기 때문에, domain이 View와 관련된 정보를 몰라야 하는 것이 좋습니다 !

디미터 법칙을 조금 더 깊이 학습해 보고, 디미터 법칙이 무엇인지를 좀 더 자세히 설명해 주세요.

디미터 법칙의 핵심 개념은 객체 구조의 경로를 따라 멀리 떨어져 있는 객체에 메세지를 보내는 설계를 피하라는 것입니다. 따라서 디미터 법칙은 최소 지식 원칙이라고도 불립니다!
디미터 법칙에 따르면, 한 객체는 다음에 해당하는 메서드를 호출함으로써 객체 간 결합도를 효과적으로 낮출 수 있습니다.

  1. 객체 자신의 메서드
  2. 메서드의 파라미터로 전달된 객체의 메서드
  3. 메서드 내에서 생성하거나 초기화한 객체의 메서드
  4. 객체가 인스턴스 변수로 직접 소유한 객체의 메서드
    이러한 범위를 준수함으로써 디미터의 법칙을 지킬 수 있습니다 !

객체간의 결합도가 높아지고 객체의 내부 구조가 외부로 노출되는 것이 왜 단점인가요? 어떤 문제가 있나요?

객체간의 결합도가 높아지면 하나의 객체에서 변경이 발생했을 때
다른 객체에도 영향을 끼치게 되고, 다른 상황에서 재사용하기 어려워집니다. 또한 객체에 독립적인 테스트 작성이 어려워집니다.

객체의 내부 구조가 외부로 노출된다면 다른 객체가 해당 객체의 내부 구현에 의존하게 될 가능성이 높아집니다. 따라서 해당 객체의 내부 구현이 변경되면 이를 사용하는 다른 코드도 다 변경해야 됩니다. 객체 내부 상태가 외부로 노출되어 외부에서 직접 변경될 가능성도 생겨 불변성 보장에도 어려움이 생기는 문제가 있습니다!

return reservationTimeRepository.findAll()
.stream()
.map(ReservationTimeResponse::fromReservationTime)
.collect(Collectors.toList());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 ㅠㅠ 명령어 잘 사용할게요 !!

안된 부분도 반영했습니다 ~!

return ReservationTimeResponse.fromReservationTime(newReservationTime);
}

@Transactional(readOnly = true)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메소드 명으로 충분히 알 수 있는 부분 동의합니다 !
제가 찾아본 내용도 h2 db에서는 @Transactional (readOnly = True) 가 필요없는 것 같아 지우겠습니당.

if (name == null || name.isBlank() || date == null || time == null) {
throw new RoomescapeBadRequestException("잘못된 예약 데이터입니다: name/date/time이 null이거나 빈 값입니다.");
if (name == null || name.isBlank() || date == null) {
throw new RoomescapeBadRequestException("잘못된 예약 데이터입니다.");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음.. 일반 사용자 입장에서는 모호할 것 같아요.
지금은 @NotNull 같은 애노테이션을 사용해서 request의 검증을 하기 때문에 해당 예외 메세지는 사라졌지만, 만약 예외 메세지를 작성한다면

잘못된 예약 데이터입니다. 이름 날짜 시간 항목은 필수입니다. 다 정도로 null 같은 키워드는 빼고 적을 것 같아요 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants