improve: 산책 저장 시 부여되는 뱃지 확인 로직 #368
Open
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
Update CI/CD infra (Docker,GitHub Action) & Swagger 적용
Fix: gradle.yml
Update gradleDeployMaster.yml
Update and rename gradle.yml to gradleDev.yml
Update gradleDeployMaster.yml and gradleDeployDev.yml
Update gradleDeployMaster.yml
Update gradleDeployMaster.yml and gradleDeployDev.yml
fix: Security-Server yml version
Chore : Update Security YAML
improve: 산책 저장 시 부여되는 뱃지 확인 로직
dlxortmd987
approved these changes
Jul 13, 2023
| public List<Integer> getAcquiredBadgeIdxList(int userIdx, Double totalDistance, Integer totalWalkCount) { | ||
| // 현재 누적 거리, 횟수로 얻을 수 있는 뱃지 계산 - 획득한 뱃지가 없을 때 어떤 동작을 해야 하지? | ||
| TotalDistanceStatus distanceStatus = TotalDistanceStatus.getDistanceBadge(totalDistance).orElseThrow(); | ||
| TotalRecordStatus recordStatus = TotalRecordStatus.getWalkCountBadge(totalWalkCount).orElseThrow(); |
There was a problem hiding this comment.
음.. 현재 생각나는건 enum이 현재는 2개로 분리되어 있는데(record, distance) 이것들을 하나로 합치고(뭐 DefaultBadgeStatus 등등..) 여기에 start 뱃지를 넣어서 이걸 반환하는 방법이 있을 것 같아요 (근데 이거는 좀 별로인 것 같긴 하네요.)
아니면 그냥 Optional로 놔두다가 274번째줄 이후에 둘 다 empty면 바로 반환하는 방법도 있을 것 같아요.
처음이면 1만 넣어서 반환, 아니면 빈 리스트로 반환
| List<Integer> acquiredBadgeIdxList = new ArrayList<>(); | ||
|
|
||
| // 현재 갖고 있는 뱃지 조회 | ||
| List<Integer> beforeSavingWalkBadgeList = userBadgeRepository.findAllByUserIdxAndStatus(userIdx, "ACTIVE") |
There was a problem hiding this comment.
findAllByUserIdxAndStatus 반환 값이 Optional<List<>> 형태인데
Optional을 빼고 그냥 List<> 형태로 반환하면 어떨까요?
반환 값이 없으면 빈 리스트로 나오는게 자연스러울 것 같아서요!
| } | ||
| } | ||
|
|
||
| return acquiredBadgeIdxList; |
There was a problem hiding this comment.
Suggested change
| return acquiredBadgeIdxList; | |
| return Collections.unmodifiableList(acquiredBadgeIdxList); |
메서드 밖에서 리스트에 추가 못하게 불변 리스트로 반환하는 것도 좋을 것 같아요
| int originMaxRecordBadgeIdx = 1; // 원래 갖고 있던 뱃지(6~8)의 가장 큰 값 | ||
|
|
||
| for (Integer originBadgeIdx : beforeSavingWalkBadgeList) { | ||
| if (originBadgeIdx >= TotalDistanceStatus.minBadgeIdx() && originBadgeIdx <= TotalDistanceStatus.maxBadgeIdx()) { |
There was a problem hiding this comment.
if 문 안에 있는 boolean 값을 TotalDistanceStatus 안에 넣는건 어떻게 생각하시나요?
| List<Integer> beforeSavingWalkBadgeList = userBadgeRepository.findAllByUserIdxAndStatus(userIdx, "ACTIVE") | ||
| .map(userBadgeList -> userBadgeList.stream() | ||
| .map(UserBadge::getBadgeIdx) | ||
| .sorted() |
Comment on lines
+278
to
+285
| for (Integer originBadgeIdx : beforeSavingWalkBadgeList) { | ||
| if (originBadgeIdx >= TotalDistanceStatus.minBadgeIdx() && originBadgeIdx <= TotalDistanceStatus.maxBadgeIdx()) { | ||
| originMaxDistanceBadgeIdx = originBadgeIdx; | ||
| } | ||
| if (originBadgeIdx >= TotalRecordStatus.minBadgeIdx() && originBadgeIdx <= TotalRecordStatus.maxBadgeIdx()) { | ||
| originMaxRecordBadgeIdx = originBadgeIdx; | ||
| } | ||
| } |
There was a problem hiding this comment.
앞에서 beforeSavingWalkBadgeList을 만들 떄 내림차순으로 정렬하고
originMaxDistanceBadgeIdx, originMaxRecordBadgeIdx 이게 초기화 되면
바로 for 문을 나가는건 어떤가요?
Suggested change
| for (Integer originBadgeIdx : beforeSavingWalkBadgeList) { | |
| if (originBadgeIdx >= TotalDistanceStatus.minBadgeIdx() && originBadgeIdx <= TotalDistanceStatus.maxBadgeIdx()) { | |
| originMaxDistanceBadgeIdx = originBadgeIdx; | |
| } | |
| if (originBadgeIdx >= TotalRecordStatus.minBadgeIdx() && originBadgeIdx <= TotalRecordStatus.maxBadgeIdx()) { | |
| originMaxRecordBadgeIdx = originBadgeIdx; | |
| } | |
| } | |
| for (Integer originBadgeIdx : beforeSavingWalkBadgeList) { | |
| if (originBadgeIdx >= TotalDistanceStatus.minBadgeIdx() && originBadgeIdx <= TotalDistanceStatus.maxBadgeIdx()) { | |
| originMaxDistanceBadgeIdx = originBadgeIdx; | |
| } | |
| else if (originBadgeIdx >= TotalRecordStatus.minBadgeIdx() && originBadgeIdx <= TotalRecordStatus.maxBadgeIdx()) { | |
| originMaxRecordBadgeIdx = originBadgeIdx; | |
| } | |
| if (originMaxDistanceBadgeIdx != 1 && originMaxRecordBadgeIdx != 1) break; | |
| } |
Comment on lines
+288
to
+299
| if(distanceStatus.getBadgeIdx() > originMaxDistanceBadgeIdx) { | ||
| for (int i = originMaxDistanceBadgeIdx + 1; i <= distanceStatus.getBadgeIdx(); i++) { | ||
| acquiredBadgeIdxList.add(i); | ||
| } | ||
| } | ||
|
|
||
| //거리 뱃지 확인 로직 - 2~5 | ||
| if(recordStatus.getBadgeIdx() > originMaxRecordBadgeIdx) { | ||
| for (int i = originMaxRecordBadgeIdx + 1; i <= recordStatus.getBadgeIdx(); i++) { | ||
| acquiredBadgeIdxList.add(i); | ||
| } | ||
| } |
There was a problem hiding this comment.
안드한테 순서대로 보내려면 두 개 순서 바꿔야 할 것 같아요
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.
변경한 부분
코드리뷰 시 함께 고민해줬으면 좋겠는 부분 (?)
완벽하진 않지만 우선 지금까지 한 부분만 올릴게용!
그리고 추가적으로 더 할 수 있으면 계속 수정하겠습니다 :)