-
Notifications
You must be signed in to change notification settings - Fork 1
[REFACTOR] 검색 리팩토링 #370
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] 검색 리팩토링 #370
Conversation
leebs0521
left a comment
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 String findNameById(UUID id) { | ||
| return findDynamicFieldByCenterId(id, userCommonAttribute.name) | ||
| .orElse(null); | ||
| } |
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.
여기그냥 Optional로 하는게 어떠신가요??
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.
감사합니다 반영했습니다!
|
|
||
| if (name == null || name.isBlank()) { | ||
| throw new BadRequestException(NOT_EXISTS_CENTER); | ||
| } |
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.
위에 Optional로 바꾸면 여기서 .orElse() 에러 핸들링 될 같아요. 그리고 NoSuchEle~ 에러클래스가 더 어울릴 것 같아요
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 final CenterQueryUseCase centerQueryUseCase; | ||
| private final NEWVolunteerQueryUseCase volunteerQueryUseCase; | ||
| private final NEWCenterQueryUseCase centerQueryUseCase; | ||
| private final LocationQueryUseCase locationQueryUseCase; |
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.
엇 다른 도메인의 메서드를 사용하는 거라 일부러 usecase로 가져와서 한 건데 repository로 하는게 맞을까요?
동작은 잘 되는데 의견 주시면 다음 리팩토링에 반영하겠습니다!
m-a-king
left a comment
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 String findNameById(UUID id) { | ||
| return findDynamicFieldByCenterId(id, userCommonAttribute.name) | ||
| .orElse(null); | ||
| } |
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.
저도 그게 좋다고 생각해용
- 불필요한 import 제거
- BadRequestException -> NoSuchElementException
b3f1c5e to
302f78e
Compare
|



📌 과제 설명
👩💻 요구 사항과 구현 내용
✅ PR 포인트 & 궁금한 점