-
Notifications
You must be signed in to change notification settings - Fork 1
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/#129] 공고 상세 정보 및 검색 결과 화면 response body 수정 #132
Conversation
- @AuthenticationPrincipal을 통해 userId를 가져올 때 모든 API가 유저 인증이 필수이므로 Long에서 long타입으로 수정 - @PathVariable로 internshipAnnouncementId를 받는 경우 default가 required=true이므로 타입을 Long에서 long으로 수정
- Spring AOP를 사용하여 스크랩 색상 수정API - 색상(enum) 조회시 map를 활용한 캐싱과 .values()를 통한 순차 탐색의 성능 차이를 알아보기 위한 테스트 환경 구축
- InternshipDetailResponseDto 필드 수정됨에 따라 DTO 생성시의 파라미터도 수정
- 기획측 요구사항에 따라 30 -> 60일 보다 오래된 공고를 제외하도록 수정 - 오래된 인턴십 공고를 제외하기 위해 사용되던 '60일' 값을 상수화(INTERNSHIP_CREATED_WITHIN_DAYS)하여 코드 가독성 및 유지보수성을 개선하였습니다.
- 인턴공고의 총 개수를 구할 때, 인턴공고가 없을 경우, 즉 count가 null일 경우 0L을 기본값으로 설정하여 타입 언방식에서 발생할 수 있는 NullPointerException 발생 예방
|
||
public static Color findByName(String name) { | ||
return colorMap.get(name); | ||
} | ||
|
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.
정윤님,
Color enum에 findByName 메서드를 추가한 부분은 정말 좋은 개선인 것 같습니다! 다만, 메서드가 호출되는 시점에 만약 존재하지 않는 컬러 이름이 입력된다면, 그에 대한 에러 처리가 어떻게 될지 조금 더 고려해보는 게 좋을 것 같습니다. 이를 통해 예외 상황에서도 안정적으로 동작할 수 있도록 추가적인 에러 핸들링을 고민해보면 어떨까요?
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.
전체적으로 깔끔한 리펙토링인 것 같습니다!
데이터 형식이 비슷한 다른 API와 리펙토링 내용이 겹치는 부분들이 많아서 피드백 사항이 크게 없네요:)
고생하셨습니다..!
|
||
private static final int INTERNSHIP_CREATED_WITHIN_DAYS = 60; | ||
|
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.
나중의 수정을 고려해 상수를 따로 선언해주신 부분이 좋네요!
- 스크랩 수정 및 추가 시 색상 정보를 찾을 때, 잘못된 값이 들어왔을 경우에 대한 예외처리 추가
📄 Work Description
⚙️ ISSUE
📷 Screenshot
💬 To Reviewers
바뀐 수정사항은 아래와 같습니다!!
🔗 Reference
문제를 해결하면서 도움이 되었거나, 참고했던 사이트(코드링크)