Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
카테고리 순서 변경 #988
카테고리 순서 변경 #988
Changes from 54 commits
2e7fd59
fc4681a
2d61a2c
8e1037d
2b966cd
c4f283f
d90802b
339286a
52810d3
f8dfd92
ddcb834
64096db
bc199d8
0102194
fa04a1d
d61ac47
25e0e47
11ab595
9eea751
5a2e765
a760937
d795cbe
f94b833
ff27d42
ff3acb9
92794bd
29be344
1f93dc0
85e87cb
f96d279
39d0e0c
26a0fab
c4a6ffc
ec7a539
74fc71c
9757b17
771d391
f79c1c3
601ff62
bb06d03
fa429ec
6d27648
347eb85
051e800
73023b3
85bf564
3753a32
dcc09b5
d9da7e5
f83d47a
d4d756f
bc8f98a
cc681f9
0802f93
008eceb
2aace81
6c4431a
9e84fe5
3e870f6
1ca4fcf
ae70fa9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 메서드로 외부에서 호출하게 한 이유가 궁금해요!
생성자에서 처리하지 않는다면 외부에서 따로 검증 메서드를 호출해야 하는데, 휴먼 에러로 이 검증을 호출하지 않아버릴 수 있을 것 같아요 🤔 또 도메인 규칙을 지키지 않는 도메인 객체가 생성되어 남게 될 수도 있겠네요!
이것에 대해 어떻게 생각하는지 궁금해요!
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.
제가 이해한 몰리의 "생성 시 즉시 검증"은 다음과 같이 코드를 작성하는 것을 의미하는 것 같아요.
저도 이런 경우의 도메인 검증 로직은 외부에서 호출되는 게 아니라 생성자에서 바로 이루어져야 한다고 생각해요.
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.
그런데... 코드 리뷰를 하며 곰곰이 생각해보니 저는 ids라는 클래스를 도메인으로 바라보는 게 맞지 않는 것 같아요.
우리가 검증을 하는 이유는 DB에 저장될 데이터의 생성, 수정, 삭제라는 서비스 로직이 실행되기 위한 입력이 정상적인지 확인하기 위해서에요.
우리가 "요청을 받는 방식"을 정책적으로 결정함에 따라 생겨난 요구사항이에요.
그 과정에서 사용되는 ID라는 속성은 RDBMS와 JPA에 의존하기 때문에 생겨났을 뿐이고, 카테고리라는 도메인과 무관해요.
본 클래스는 컬렉션에서의 중복 검증 로직일 뿐이에요.
로직이 도메인의 특성에서 비롯되지 않았어요.
카테고리가 아닌 다른 도메인에서도 컬렉션의 중복 검증 로직은 얼마든지 이루어질 수 있어요.
실제로 SourceCodeService에서도 동일한 검증 로직을 수행하고 있죠.
그런 의미에서 본 ID 중복 검증 클래스가 하는 일은 JPA Entity의 ID의 컬렉션을 이용한 서비스 로직으로 간주해야 한다고 생각해요.
SourceCodeService에서 같은 로직을 사용하고 있으니, global 패키지의 util 로 분리해도 되겠군요.
Ordinals, Names 클래스도 같은 의견이에요.
두 클래스는 서비스 레이어에서 DTO를 사용하는 방식 때문에 만들어졌을 뿐이고, 다른 도메인 객체와 상호작용하지 않아요.
Ordinals, Names가 도메인 영역에서 의미를 갖고 다른 도메인 객체와 상호작용이 있다면 도메인으로 볼 수 있겠지만, 그렇지 않으므로 categoryValidationService로 옮겨져야 한다고 생각해요.
저는 몰리가 일급컬렉션을 이야기했을때, Categories를 만드는 것을 생각했어요.
그렇게되면 Categories 객체가 생성될 때 각 원소의 ordinal과 name 필드의 중복을 검사할 수 있죠.
다만 중복 검증 로직은 여전히 util로 분리할 여지가 있다고 생각하고, Categories 클래스를 만든다면 생성자에서 util 클래스의 정적 메서드를 호출해서 검증하는 게 좋을 것 같아요.
핵심은 검증이라고 해서 다 같은 검증이 아니라는 거에요. 도메인의 검증과, 서비스의 검증을 구분하자는 이야기를 하고 싶었어요. 예를 들어 Default 카테고리가 있다는 정책은 도메인 영역으로 봐도 좋다고 생각해요. 카테고리를 수정할 수 있다는 것도 도메인 영역이에요. 그래서 카테고리를 수정할 때, Default 카테고리를 수정하려 할 경우 도메인 객체에서 예외를 발생시켜도 된다고 생각해요. 그래서 위 Category 클래스에 같은 의도로 의견을 남겼답니다.
너무 길게 써서 죄송합니다... 😭
그러나 제 생각을 가능한 그대로 전달하는 게 서로에게 더 도움이 된다고 생각했어요.
혹시 제가 모르는 게 있거나, 다른 의견이 있다면 얼마든지 부탁해요!!