[♻️ refactor/#96] 스크랩 관련 API -> path variable 수정 및 color enum 필드 수정 #125
Conversation
This file contains 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
junggyo1020
reviewed
Sep 5, 2024
Comment on lines
+161
to
166
| private Color findColor(String color) { | ||
| return Arrays.stream(Color.values()) | ||
| .filter(c-> c.getKey() == color) | ||
| .filter(c-> c.getName().equals(color)) | ||
| .findAny().get(); | ||
| } | ||
|
|
There was a problem hiding this comment.
- .findAny().get() 을 사용하는 경우, Invalid한 color값이 요청으로 들어오면 오류가 발생할 수 있어 이부분에 대한 예외처리가 있으면 좋을 것 같습니다..!
- 색상 변경은 빈번하게 일어나는 기능이라고 생각이 드는데, Color.values()를 매번 순회하는 대신, Map을 사용해 캐싱을 하는 방식도 생각해보면 좋을 것 같아요!
There was a problem hiding this comment.
-
1번에 대해서는
request dto에서 유효성 검증을 하면 좋을 것 같다는 생각인데요!
저희가 아직validation exception을GlobalExceptionHandler에서 전역적으로 처리하지 않고 있어서 추후에 해당 처리가 된 후에 바로 적용하겠습니다!👉더 좋은 의견있다면 달아주세요 ㅎㅎ
-
2번은 저도 생각못했는데 한 번 찾아보고 적용해볼게요 감사합니다!
|
전체적으로 코드가 깔끔하게 리팩토링된 것 같습니다. 수고하셨습니다. |
JungYoonShin
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
📄 Work Description
⚙️ ISSUE
📷 Screenshot
- 스크랩 추가
💬 To Reviewers
리팩할 부분이나 보완할 부분은 편하게 코리 달아주세요~!
🔗 Reference
문제를 해결하면서 도움이 되었거나, 참고했던 사이트(코드링크)