-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor : 애그리거트 재 설계로 인한 패키지 조정 #83
Conversation
레포지토리 위치 변경
위와 같은 형태는 어떨까요? https://incheol-jung.gitbook.io/docs/study/ddd-start/4-jpa 패키지 네이밍 변경
ddd에서 레이어를 위와 같이 나누는거같은데 패키지 네이밍을 위와 같이 하는것은 어떨까요? https://github.com/woowacourse/service-apply/tree/master/src/main/kotlin/apply DTO 패키지저는 개인적으로 DTO 패키지 위치는 사용하는 Layer에 있어야한다고 생각해요 https://github.com/woowacourse/service-apply/tree/master/src/main/kotlin/apply/application |
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.
고생 많았어 👍
@@ -1,4 +1,4 @@ | |||
package mafia.mafiatogether.domain; | |||
package mafia.mafiatogether.room.util; |
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.
개인적인 생각으로는 CodeGenerator는 우리 비즈니스 로직이 들어가있다고생각해서 domain 패키지에 넣는것이 어떻까요?
private final static int TARGET_STRING_LENGTH = 10;
클래스명이나 해당 부분에서도 저희가 정한 코드 10자가 비즈니스 규칙이라고 생각하는데
해당 클래스가 유틸클래스가되려면 숫자를 입력받고 거기에 해당하는 랜덤 문자열을 뽑아내야하지않을까요? (추가로 이런 느낌의 클래스명)
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.
저도 CodeGenerator는 도메인에 포함되어야 한다고 생각해요!
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.
PR 타이틀에 조금 더 해당 PR의 의도가 드러나면 좋을 것 같아요! 가령 애그리거트 재 설계로 인한 패키지 조정 이런 식으로요?! (그렇게 중요한 것은 아니라고 생각하지만, 나중에 해당 PR을 볼 때 이게 뭐였지?! 라고 생각할 수도 있을 것 같아서 말씀드려요!)
그리고 항상 앞장서서 솔선수범 해주시는 것 항상 감사하게 생각합니다. 🥰
제 의견도 파워랑 비슷한 것 같아요! ui, application, 이런 식으로 나누는 것은 거기서 거기인 것 같긴하지만, 현재 Repository가 Domain에 위치해야 한다는 점이요.
DDD 책을 읽어보면, Service쪽에서 infra쪽을 의존하지 않기 위해, 의존 역전을 활용하여 domain쪽에 repository interface를 두고, infra 딴에서 이를 구현을 많이 하고는 하는데, 이를 저희의 상황에 대입해서 생각해보면, 사실 구현체는 JPA가 구현해주고 있으니, 따로 infra 패키지를 둘 필요도 service와 repository 패키지를 구분할 필요도 없다고 봅니다.
Dto도 비슷하게 생각해요, Presentation 영역에 두게 되면, Service에서 Presentation 영역에 의존하게 되니, Service 딴에 두어야 한다고 생각하고, 위에서 말씀드렸던 repository와 비슷한 논리로 Service와 Dto를 구분할 필요는 없다고 생각해요!
말씀드린 모든 사항은 개인적인 생각이니 한번 생각해보시고 다시 의견주시면 감사할 것 같아요!
다시 한번 고생했습니다. 달달달리~~~
|
||
public record RoomStatusResponse( | ||
StatusType statusType | ||
|
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.
@@ -1,4 +1,4 @@ | |||
package mafia.mafiatogether.domain; | |||
package mafia.mafiatogether.room.util; |
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.
저도 CodeGenerator는 도메인에 포함되어야 한다고 생각해요!
@thdwoqor @kpeel5839 이렇게 하니까 더 깔끔해 보이네요! 계층별로 나뉘어서 한눈에 보기 쉬운것 같습니다! 역시 뭘 더하기 전에 패키지 정리를 해본것이 좋았던거 같네요! |
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.
close #82
작업단위가 너무 커져서 우선적으로 의존성 분리전 패키지 분리 pr부터 올려봅니다.
의존성 분리의 경우 레디스 도입과 같이 진행을 해야할 것 같아 우선적으로 분리된 패키지부터 같이 보자는 의미에서 올려봅니다.
변경 사항
어그리거트 별 패키지 분리
패키지 구조
이렇게 구현되어있습니다.