[SO-182] refactor: 플래너, 예비부부의 Tag를 String으로 변경 및 매개변수 개수 감소#61
Merged
nitronium102 merged 5 commits intodevelopfrom Aug 14, 2023
Merged
Conversation
lina1919
approved these changes
Aug 14, 2023
Contributor
lina1919
left a comment
lina1919
left a comment
There was a problem hiding this comment.
네 저도 parameter를 쓰는 편이 깔끔하고 좋은 것 같습니다!
이제 tag 관련 복잡한 로직들이 없어져서 마음이 아주 편-안 합니다.
수고 많으셨어요!
nitronium102
commented
Aug 14, 2023
Comment on lines
+21
to
+31
|
|
||
| @Builder | ||
| public CustomerTagListDto(String portfolioTagList, String plannerTagList, String dressTagList, | ||
| String studioTypeTagList, String studioFocusTagList, String makeupTagList) { | ||
| this.portfolioTagList = portfolioTagList; | ||
| this.plannerTagList = plannerTagList; | ||
| this.dressTagList = dressTagList; | ||
| this.studioTypeTagList = studioTypeTagList; | ||
| this.studioFocusTagList = studioFocusTagList; | ||
| this.makeupTagList = makeupTagList; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
인자 개수가 6개라서 아슬아슬하게 이걸로 묶었습니다
만약 인자가 더 늘어난다면 화면별로 분할해서 portfolioTag, plannerTag / dressTag, StudioTypeTag, studioFocusTag, makeupTag 이런 식으로 나눌 것 같아요
Comment on lines
50
to
61
| @Builder | ||
| public Customer(String weddingDate, String budget) { | ||
| this.weddingDate = weddingDate; | ||
| public Customer(Boolean weddingDateConfirmed, String region, String budget, CustomerTagListDto customerTagList) { | ||
| this.weddingDateConfirmed = weddingDateConfirmed; | ||
| this.region = region; | ||
| this.budget = budget; | ||
| this.portfolioTagList = customerTagList.getPortfolioTagList(); | ||
| this.plannerTagList = customerTagList.getPlannerTagList(); | ||
| this.dressTagList = customerTagList.getDressTagList(); | ||
| this.studioTypeTagList = customerTagList.getStudioTypeTagList(); | ||
| this.studioFocusTagList = customerTagList.getStudioFocusTagList(); | ||
| this.makeupTagList = customerTagList.getMakeupTagList(); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
원래 customBuilder를 생성하려 했으나 필수 인자가 너무 많아 파라미터 객체를 사용하여 묶어주었습니다.
Contributor
Author
진짜 태그 관련 중간 테이블 없애니까 db 구조도 엄청 깔끔해지고 좋은 것 같아요~!👍👍👍 |
|
Kudos, SonarCloud Quality Gate passed!
|
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
작업 개요
Tag 테이블을 삭제하고 String으로 바꿈에 따라 플래너 및 예비부부의 Tag를 String으로 변경
작업 분류
작업 상세 내용
생각해볼 문제
빌더 패턴을 쓰면 좋다고는 하지만 저희처럼 필수 변수가 너무 많은 경우에는 지금처럼 파라미터 객체를 생성해서 묶어주는 편이 더 나은 것 같습니다.
빌더 패턴은 선택 변수가 많은 경우에 더 적합한 것 같아요....