[feat] 개별 프로필 조회 API 구현 #139
Merged
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
SunwoongH
requested changes
Mar 3, 2024
Comment on lines
20
to
30
| return new TripParticipantProfileResponse( | ||
| user.getName(), | ||
| user.getIntro(), | ||
| validatedResult, | ||
| participant.getStyleA(), | ||
| participant.getStyleB(), | ||
| participant.getStyleC(), | ||
| participant.getStyleD(), | ||
| participant.getStyleE(), | ||
| isOwner | ||
| ); |
There was a problem hiding this comment.
Builder 어노테이션을 적용해봐도 좋을 것 같습니다~
Comment on lines
211
to
217
| private int getvalidatedResult(User user) { | ||
| try { | ||
| return user.getResult().getNumResult(); | ||
| } catch (NullPointerException e) { | ||
| return -1; | ||
| } | ||
| } |
There was a problem hiding this comment.
메서드명 Camel Case 확인 한 번 부탁드립니다. 그리고 NPE의 경우,
numResult가 int 기본 자료형으로 선언되어 있어서 발생하는 것 같습니다. 해당 자료형을 Integer 래퍼 클래스로 변경하면 null 값을 가질 수 있어서 해결될 것 같은데 확인 가능할까요?
There was a problem hiding this comment.
변경해보니 response builder에서 user.getResult().getNumResult()를 사용하는데 getResult() 자체가 null 이기 때문에 getNumResult()를 꺼내오면 NPE가 발생하는것으로 확인됩니다. null을 그대로 반환하려면 다시 user.getResult()를 null인지 아닌지 확인하는 메서드가 필요한데 이 메서드를 추가해서 null을 반환하는 것으로 변경하는게 나을까요?! 좋은 의견있으면 알려주세요!
There was a problem hiding this comment.
제가 잘못 이해했었네요! 저도 조금 더 고민이 필요할 것 같습니다. 우선 2차 스프린트 기간 내에 API가 배포되어야 해서 현재 기능적인 문제는 없으니 현재 PR 머지 후 이슈를 새롭게 생성해서 리팩터링을 진행해 보면 좋을 것 같습니다. 같이 고민해 보겠습니다!
SunwoongH
approved these changes
Mar 4, 2024
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.
Related Issue 📌
close #134
Description ✔️