-
Notifications
You must be signed in to change notification settings - Fork 3
refactor: 도메인 재설계 #366
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: 도메인 재설계 #366
Conversation
f1df636
to
b136096
Compare
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.
요즘 리팩토링 하느라 고생많으십니다!
일단 요 걸 짚고 가야될거 같슴당
} | ||
|
||
private BooleanExpression isContainFunctionalities(List<String> functionalityList) { | ||
if (functionalityList.isEmpty()) { | ||
private static BooleanExpression isContainPrimaryIngredients(List<String> primaryIngredientList) { |
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.
static 제거
@OneToMany(mappedBy = "petFood", cascade = {CascadeType.PERSIST, REMOVE}) | ||
private List<PetFoodFunctionality> petFoodFunctionalities = new ArrayList<>(); | ||
@OneToMany(mappedBy = "petFood", orphanRemoval = true, cascade = {PERSIST, REMOVE}) | ||
private List<PetFoodEffect> petFoodEffects = new ArrayList<>(); |
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.
일단 다 보긴했는디
요거를 짚고가야 아래꺼 추가적으로 리뷰를 할 수 있을듯용
지금 바뀐게
Functionality(기능) 하고 PrimaryIngredient(주원료)가 PetFoodEffect 하나의 엔티티로 바뀌고 컬럼한개로 구분하는거 같슴다.
근데 전 두 개가 다른 카테고리 같아서 묶는 건 조금 어색하다고 생각합니당.
그리고 후에 기능이든 주원료든 추가되는 컬럼이 있으면 공통으로 쓸 수 없을텐데 혹시 이렇게 설계하신 이유가 있을까용
📄 Summary
🙋🏻 More