fix: 승인 과정에서 발생하는 npe 처리 #108
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
uwoobeat
requested changes
Feb 22, 2024
| @@ -110,10 +112,15 @@ public Page<Member> findAllByPaymentStatus(RequirementStatus paymentStatus, Page | |||
|
|
|||
| @Override | |||
| public Map<Boolean, List<Member>> groupByVerified(MemberGrantRequest request) { | |||
There was a problem hiding this comment.
파라미터 DTO로 안받도록 수정해주세요. 아까 체크 못했네요
Comment on lines
119
to
123
|
|
||
| Map<Boolean, List<Member>> classifiedMember = new HashMap<>(); | ||
| classifiedMember.put(true, groupByVerified.getOrDefault(true, new ArrayList<>())); | ||
| classifiedMember.put(false, groupByVerified.getOrDefault(false, new ArrayList<>())); | ||
| return classifiedMember; |
There was a problem hiding this comment.
Suggested change
| Map<Boolean, List<Member>> classifiedMember = new HashMap<>(); | |
| classifiedMember.put(true, groupByVerified.getOrDefault(true, new ArrayList<>())); | |
| classifiedMember.put(false, groupByVerified.getOrDefault(false, new ArrayList<>())); | |
| return classifiedMember; | |
| if (groupByVerified != null) { | |
| return groupByVerified; | |
| } | |
| Map<Boolean, List<Member>> emptyMap = new HashMap<>(); | |
| emptyMap.put(true, new ArrayList<>()); | |
| emptyMap.put(false, new ArrayList<>()); | |
| return emptyMap; |
There was a problem hiding this comment.
null 로직을 명확히 분리하면 좋을 것 같습니다 + early return
There was a problem hiding this comment.
이게 true와 false 둘 중 하나만 null이 반환될 수 있어서 둘 다 if문으로 확인하느니 getOrDefault로 처리하는게 좋을 것 같습니다.
null 로직은 분리했습니다
There was a problem hiding this comment.
if (groupByVerified.get(true) != null && groupByVerified.get(false) != null) {
return groupByVerified;
}
return ensureNonNull(groupByVerified);
아니면 이렇게 되어야 할 것 같은데 뭔가 두번 확인하는 느낌이 나는 것 같지 않나요?
There was a problem hiding this comment.
getOrDefault 좋습니다.
메서드 이름이 뭔가 검증하는 메서드 이름 같아서 딴걸로 한번 바꿔보시면 좋을 것 같아요
저도 마땅히 생각나는 건 없는데 한번 gpt한테 물어보고 정히 없으면 그냥 패스하는 걸로 하죠
일단 어푸드릴게요
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.
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타