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 Core] 황승준 미션 제출합니다. #390

Open
wants to merge 101 commits into
base: davidolleh
Choose a base branch
from

Conversation

davidolleh
Copy link

안녕하세요 루카님~ 리뷰 항상 감사드립니다!
프로젝트 하면서 했던 고민사항들 ReadMe에 작성해 두었습니다!
잘 부탁드립니다!

@davidolleh davidolleh closed this Nov 28, 2024
@davidolleh davidolleh reopened this Nov 28, 2024
Copy link

@dooboocookie dooboocookie left a comment

Choose a reason for hiding this comment

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

승준, 안녕하세요.
리뷰어 루카입니다. 🐳

엄청난 질문을 던져주셔서 약간 놀랐어요.
사실 이 미션을 하면서 가장 최종점에 있는 고민을 미리하신 것 같아 멋있습니다.

리뷰어에게도 가이드가 생겨서 너무 많은 맥락을 전달드리기 보단, 몇가지 주제를 집중적으로 리뷰드렸으니, 자주 티키타카해요.

🎯 리뷰 포인트

  1. 레이어 중심 vs 도메인 중심 (부제: 구조를 잘 짜야하는 이유)
  2. 엔티티란?
  3. 비즈니스 로직은 누가 관리해야할까?

👀 추가 전달

일단, 누군가에게 프로덕트를 인수할 때는
image
꼭 테스트를 돌려보고 하시면 좋겠어요.
테스트를 보고 코드의 의도를 예상하는 것도 많은 사람들이 사용하는 방법입니다.
만약 테스트가 본인 의도와 맞지 않는다면, 의도에 맞게 바꿔주세요.

그리고 질문의 질이 정말 좋아서 질문에 중심적으로 답변을 드렸으니,
그를 기반으로 여러 비즈니스 요구사항을 추가해나가면서 리팩토링해보면 좋겠네요.


조금은 더 얘기하는게 좋을 것 같아서 RC 드리겠습니다.

파이팅!

Comment on lines +14 to +20
- 프로젝트 구조:<br/>
[도메인 우선 vs 레이어 우선](https://codewithandrea.com/articles/flutter-project-structure/)
<br/>
우선 프로젝트 구조를 잘 가져가면 가져올 수 있는 효과가 무엇이 있는지 궁금합니다
미션처럼 크기가 작은 프로젝트에서는 레이어 구조 또한 괜찮은 방향 같지만 서비스 크기가
큰 앱을 개발하는데 있어서는 도메인 단위로 나누는 것이 프로젝트으 복잡성을 줄일 수 있게 되는거 같습니다.
그러나 도메인 단위로 나뉘게 된다면 중복된 Entity(테이블이 서로 연관되어 있을 가능성이 크기 )에 대한 관리를 어떻게 하는지 궁금합니다!

Choose a reason for hiding this comment

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

저번에 이야기 나눴을 때, 아키텍쳐를 많이 고민했다고 애기하셨었는데 이 질문에서 딥하게 고민하고 계신 것이 느껴지네요. 👍 👍 👍

Choose a reason for hiding this comment

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

1️⃣ 질문

우선 프로젝트 구조를 잘 가져가면 가져올 수 있는 효과가 무엇이 있는지 궁금합니다

프로젝트 구조를 도메인 중심으로 가져갈지 레이어 중심으로 가져갈지 보다, 이 질문을 우선적으로 해주셔서 정말 감사합니다. 정말 훌륭합니다. 🎉
그쵸. 효과에 공감하고 느껴야 동기가 생기겠죠. 구조, 아키텍쳐 같은 최소 약속을 지키면서 코딩을 하는 것은 매우 번거로운 일이니까요.

그런 말이 있습니다 (진짜 있는지 모름) 개발자는 움직이는 기차의 바퀴를 바꾸는 것이다
모든 것을 허물고 다시 구조를 내 맘대로 올리는 것은 그리 어려운 일은 아니겠어요.
하지만, 우리가 개발자로서 하는 일은 대부분 다른 사람(혹은 과거의 나)이 만든 코드를 이해하고 기존의 비즈니스가 잘 유지된 상태로 개선을 해나가야합니다.
작은 토이 프로젝트도 몇주 몇달만 되어도 엄청난 복잡도가 생길거에요.

저는 구조를 잘 가져간다 는 것은 의존성을 잘 설정한다 인 것 같은데요.
의존성영향(변경)이 전파된다는 의미와 또 같은 의미입니다.

예를 들어 이런 의존 관계가 있겠죠.

  • Service가 Repository를 의존
  • Reservation이 Time을 의존

이는,
Repository의 문제가 생기면 Service에도 문제가 생긴다는 의미입니다.
Time에게 변경이 생기면 Reservation에도 자연스레 변경이 생기겠죠.
(예를들면 Time의 클래스명을 ReservationTime으로 바꾼다고 생각해보면 되겠어요.)

구조를 잘 가져가게 되면 유지보수 시, 변경사항의 전파를 예측하기 좋겠죠.


(영상자료)toss SLASH 22 - 김재민님 / 지속 성장 가능한 코드를 만들어가는 방법

Copy link

@dooboocookie dooboocookie Nov 28, 2024

Choose a reason for hiding this comment

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

2️⃣ 질문

미션처럼 크기가 작은 프로젝트에서는 레이어 구조 또한 괜찮은 방향 같지만 서비스 크기가
큰 앱을 개발하는데 있어서는 도메인 단위로 나누는 것이 프로젝트으 복잡성을 줄일 수 있게 되는거 같습니다.

(구조를 패키지로 많이 나타내므로 이하 패키지라는 표현을 많이 사용 예정)

오! 레이어 중심으로 구조를 가져가는 것도 정말 직관적이고 좋죠!
꼭 규모가 큰 도메인이라 해도 도메인 중심으로 패키지를 나누지 않을 수 있습니다.
레이어 별 나누고 그 안에서 도메인을 따로 나눌 수 있어요. (정답은 없는 것이니까요)

승준님이 그렇게 느끼신 것은 합리적이라 생각합니다.
아마도 규모가 클 수록 팀별로 도메인을 따로 관리할 가능성이 있겠죠.
그렇다 보면,
A팀은 예약 시스템,
B팀은 채팅 시스템,
C팀은 결제 시스템,
...
이런식으로 팀별로 도메인이 나눠질 수 있습니다
서비스 전체로 보면 예약을 하면서 채팅을하고 결제를 하겠지만, 그를 관리하는 사람(팀)이 달라지면 경계가 필요합니다.

이때부턴 도메인 중심적인 사고를 하면서 구조를 짜볼 필요가 있겠죠.

Choose a reason for hiding this comment

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

3️⃣ 질문

그러나 도메인 단위로 나뉘게 된다면 중복된 Entity(테이블이 서로 연관되어 있을 가능성이 크기 )에 대한 관리를 어떻게 하는지 궁금합니다!

답변에 앞서 중복된 Entity(테이블이 서로 연관되어 있을 가능성이 크기)라는 표현이 어떤 의미인지 정확하게 이해하지 못했습니다.
제가 이해한 질문은 도메인 중심 설게에서 다른 패키지로 나뉘어버린 Entity간 연관관게는 어떻게 핸들링하냐입니다.
이 질문이라고 생각하고 답변하겠습니다.

일단, 대부분의 조직에서는
예약 팀결제 팀 엔티티 자체직접 참조하지 않을 것입니다.
결제팀 코드 수정에 예약팀 코드가 영향을 받는 등의 의존성 문제로 참조를 끊는 것이죠.

다만, 예약에서 어떤 결제건이 연관되었는지 알아야한다면, 참조를 하긴 해야겠죠.
직접 참조하는 것보단 약한 참조인 id만을 갖고 있을 수 있겠네요. 이를 간접 참조라고도 합니다.

프로젝트 구조를 도메인 중심적으로 가져가기로 했고,
그 도메인 모델들이 경계가 구분되어서,
다른 패키지에 속했다면,
그 둘은 크게 서로 신경 쓸 필요가 없다는 의미입니다.
이 정도의 질의(메시지)는 던져볼 수 있겠네요. 👍
나 아이디가 1인 예약인데, 내 결제는 아이디가 a래. a결제 결제 상태좀 알려줄래?

이렇게 고민을 깊게 해보는 자세 정말 좋습니다.

질문하신 내용이 이 미션에서 고민할 수 있는 영역 중 꽤 고수준의 영역이라고 생각합니다.
특히 3번 질문답에는 너무 깊게 매몰되기 보다는 그렇구나 정도로 넘어가도 좋겠어요.

Choose a reason for hiding this comment

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

모두가 봤으면 하는 훌륭한 리뷰에요.
좋은 질문과 좋은 답변입니다..👍 감사합니다

Comment on lines +22 to +36
[Repository 계층, 도메인과 영속성 엔티티 사이의 간극](https://kokodakadokok.tistory.com/entry/Repository-%EA%B3%84%EC%B8%B5-%EB%8F%84%EB%A9%94%EC%9D%B8%EA%B3%BC-%EC%97%94%ED%8B%B0%ED%8B%B0-%EC%82%AC%EC%9D%B4%EC%9D%98-%EA%B0%84%EA%B7%B9-%EB%A7%A4%EA%BE%B8%EA%B8%B0)
<br/>
스프링은 기술적으로 편의를 위해서?
데이터베이스 테이블과 Java의 class를 매핑해준 Jpa 기술을 사용하는 것으로 알고 있습니다.
Jpa의 @Entity라는 annotation을 사용하여 정의하는 Class는 Domain과의 간극이 존재한다고
생각합니다. 아직 이 간극에 대해 완벽히 알지 못하여서 Entity와 Domain을 혼돈해서 사용
하게 되는거 같습니다. 최소한 Service를 나눌때 Domain기준으로 나누고 싶은데
Entity(Table)단위로 나뉘게 되는 경향이 강한거 같습니다!
지금은 TimeService와 ReservationService가 나뉘어져 있지만
TimeService와 ReservationService가 하고 있는 역할은 결국 예약이라는 하나의 도메인에 속한다는 생각이 들어
이 둘을 하나의 ReservationService로 합할까 고민하게 되었습니다.

- Entity 패키지를 따로 둔 이유 <br/>
제가 따로 Entity 패키지를 분리한 이유는 Service, Repository, Contoller(controller에서 dto를 entity로 변환하기에) 모두다
Entity를 바라보기 때문에 분리하게 되었습니다.

Choose a reason for hiding this comment

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

1️⃣ 질문

Repository 계층, 도메인과 영속성 엔티티 사이의 간극
스프링은 기술적으로 편의를 위해서?
데이터베이스 테이블과 Java의 class를 매핑해준 Jpa 기술을 사용하는 것으로 알고 있습니다.
Jpa의 @entity라는 annotation을 사용하여 정의하는 Class는 Domain과의 간극이 존재한다고
생각합니다. 아직 이 간극에 대해 완벽히 알지 못하여서 Entity와 Domain을 혼돈해서 사용
하게 되는거 같습니다.

뭐죠? 도대체 저 글은 어떤 애송이가 쓴 글이죠?

저희가 Reservation 같은 아이들을 부를 때 아래와 같이 정말 여러 이름으로 부를 수 있겠네요.

  • Domain
  • Entity
  • Table
  • Resource
  • ...

저 용어들이 사용되는 때에 따라서 의미가 약간씩은 다르게 사용되는데요.
저 나름대로 해당 단어들을 정의해보겠습니다. (틀릴 수 있음)

  • Domain
    • 해당 비즈니스(프로덕트)가 해결하고자 하는 영역을 의미합니다. 현재는 예약이라고 할 수 있겠네요. 좀 큰 범위네요.
  • Entity
    • Domain에서 의미있는(구분할 필요가 있는) 대상(실체, 객체, ...) 라고 할 수 있겠어요. Domain 보다는 조금 더 code에 가까운 개념이네요.
  • Table
    • 보통 RDBMS에서 엔티티를 다룰 때 사용하는 것이네요. 조금 DB쪽에 가까운 개념이네요.
  • Resource
    • CRUD API 에서 조회, 생성, 수정하려는 대상을 보통 resource라고 지칭합니다.
    • GET /api/v1/reservations/1 는 reservations라는 예약 리스트 리소스에서 1번 리소스를 조회한 것이겠네요

이런 관점에서 Time과 Reservation이 같은 도메인 문제를 해결하기 위한 엔티티들이라면 한 팀(패키지)에서 관리할 수 있겠네요.
근데 그렇다고 서비스 자체가 합쳐지는 것은 또 다른 문제입니다.
그럼 예약 도메인이 복잡해질 수록 ReservationService는 한없이 비대해질테니까요.

지금 제가 생각했을 때는, Service들의 문제라기 보단,
Reservation과 Time을 너무 테이블과 일치시켜 생각하시는 것 같습니다.

RDBMS가 아니라 인메모리에 저장한다고 생각하고 table 개념을 제거하고 설계해보는 것도 방법이겠어요.
그 후에 Repository만 RDB로 갈아끼면되니까요! (오... 갓프링)

Choose a reason for hiding this comment

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

2️⃣ 질문

Entity 패키지를 따로 둔 이유
제가 따로 Entity 패키지를 분리한 이유는 Service, Repository, Contoller(controller에서 dto를 entity로 변환하기에) 모두다
Entity를 바라보기 때문에 분리하게 되었습니다.

그렇군요. 이견 없습니다.

image

위에서 레이어드 아키텍처의 가치에도 크게 벗어나지 않습니다.
Entity는 Domain Model로
영속-어플리케이션-표현 레이어에서 약간은 떨어져 온전히 비즈니스 규칙을 담고있는 아이니까요.
제일 안쪽에 있다고 할 수 있습니다. 모든 레이어들이 entity를 의존하고있죠.

조금 더 자세한 리뷰는 상희님 리뷰 참고하면 좋겠어요.

Comment on lines +40 to +62
public Time save(Time reservationTime) {
try {
String query = "INSERT INTO time (time) VALUES (?)";
KeyHolder keyHolder = new GeneratedKeyHolder();

jdbcTemplate.update(connection -> {
PreparedStatement ps = connection.prepareStatement(
query,
new String[]{"id"});
ps.setString(1, reservationTime.getTime().format(CustomDateTimeFormat.timeFormatter));
return ps;
}, keyHolder);

Long id = keyHolder.getKey().longValue();

return new Time(
id,
reservationTime.getTime()
);
} catch (DuplicateKeyException e) {
throw new EntityAlreadyExistException(e.getMessage());
}
}

Choose a reason for hiding this comment

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

[Comment]

👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏
정말 좋습니다. 기립박수

그쵸. 이 방탈출카페는 아무리 생각해도 테마 선택하는 곳을 없는 것 보니 방탈 테마는 하나인가봐요.

그럼 중복의 예약시간을 만들면 안되겠죠.

그럼 여기서 고려를 해볼게 있겠네요.

중복한 예약시간은 존재하면 안된다.

이 요구사항을 지키기 위해 이를 막는 방법은 여러가지가 있을텐데요.

  1. 서비스? 도메인? 어디선가...?
  2. DB에 저장할 때 에러(지금처럼)
  3. ...

현재 방법의 장단점과 다른 방법으로는 커버가 안되는 영역이 있을까요?

만약, 해당 방탈출은 러닝 타임이 1시간이고 청소시간이 30분이라 예약시간 1시간 30분 이후에는 예약 시간이 생기면 안된다는 비즈니스 규칙은 어디에 구현하면 좋을지 고려해주세요.

Comment on lines +17 to 23
public ReservationService(
@Autowired ReservationDao reservationRepository,
@Autowired TimeDao timeDao
) {
this.reservationDao = reservationRepository;
this.timeDao = timeDao;
}
Copy link

@dooboocookie dooboocookie Nov 28, 2024

Choose a reason for hiding this comment

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

[Request Change]

image

헐... 중복 예약 되는데요;;;;

그리고 저 지금 29일인데 27일게 예약돼요....

@davidolleh
Copy link
Author

리뷰에 대한 답변을 너무 늦게 달아서 죄송합니다. 너무 성심성의껏 달아주셨는데 개인적인(신체) 문제로 리뷰에 대한 답변과 수정이 늦었습니다. 리뷰 너무 감사드립니다 :)

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.

3 participants