-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FEAT/#57] 위치 기반 장소 추천 알고리즘 구현 #63
Conversation
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.
고생하셨습니다 ~! 어푸 어푸 🌊🌊
@@ -59,7 +59,7 @@ private String getJwtFromRequest(HttpServletRequest request) { | |||
String bearerToken = request.getHeader("Authorization"); | |||
|
|||
if (!StringUtils.hasText(bearerToken)) { | |||
throw new BusinessException(ErrorType.UN_LOGIN_ERROR); | |||
// throw new BusinessException(ErrorType.UN_LOGIN_ERROR); |
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.
Q3: 주석하신 이유가 있나요?
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에서도 에러를 던지진 않지만 해당 에러 문구가 출력돼서 로그가 오염되는 것 같아 일단 주석 처리하고 추후에 수정하려고 했습니당
// 가변 리스트 생성 | ||
List<SpotEntity> mutableList = new ArrayList<>(filteredSpotList); | ||
|
||
// 무작위로 6개 추출 | ||
Collections.shuffle(mutableList); | ||
|
||
List<RecommendedSpot> spotList = mutableList.stream() | ||
.map(this::toRecommendedSpot) | ||
.limit(6) | ||
.toList(); | ||
|
||
return new SpotListResponse(spotList); |
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.
Q3: 이 로직 따로 함수로 분리 안하신 이유가 있을까요!?
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.
없습니다 .. 추후 리팩하겠습니다 하하 ~
if (preferenceEntity == null) { | ||
return score; | ||
} |
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.
Q1: preferenceEntity가 없을 때 위에서 처리해준 거 아닌가요!?
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.
맞아요 ㄷㄷ 수정했습니다 ㅠ
score += calcFavoriteSpotScore(optionList, preferenceEntity.getFavoriteSpotRank()); | ||
|
||
score += calcAcornScore(spotEntity); | ||
|
||
// TODO: 매직 넘버 yml로 옮기기 | ||
if (spotEntity.getCreatedAt() != null) { | ||
LocalDateTime threeMonthsAgo = LocalDateTime.now().minusMonths(3); | ||
if (spotEntity.getCreatedAt().isAfter(threeMonthsAgo)) { | ||
score += 3.0; | ||
} | ||
} |
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.
Q3: 어떤 부분은 함수로 분리되어있고 어떤 부분은 조건문 내에 존재하는데 분리 기준이 있을까요?
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.
없습니다 .. 추후 리팩하겠습니다 하하 ~
// 1) matchingRate 내림차순 | ||
int rateCompare = Integer.compare(b.matchingRate(), a.matchingRate()); | ||
if (rateCompare != 0) { | ||
return rateCompare; | ||
} | ||
|
||
// 2) matchingRate가 같은 경우 createdAt 내림차순 | ||
LocalDateTime aCreated = a.spotEntity().getCreatedAt(); | ||
LocalDateTime bCreated = b.spotEntity().getCreatedAt(); | ||
|
||
// createdAt이 null일 수도 있으니 안전 처리 | ||
if (bCreated == null && aCreated == null) { | ||
return 0; | ||
} else if (bCreated == null) { | ||
// b가 null이면 a가 더 최근이므로 a가 앞으로 | ||
return -1; | ||
} else if (aCreated == null) { | ||
// a가 null이면 b가 더 최근이므로 b가 앞으로 | ||
return 1; | ||
} |
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.
P3: 추후 comparator 함수로 분리해도 좋을 것 같아요~
if (isRecentLocal && spotEntity.getLocalAcornCount() >= 4) { | ||
acornScore += 5.0; // TODO: 매직 넘버 yml로 옮기기 |
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.
P1: isRecentLocal이 true면 도토리 개수가 4개 이상이므로 뒤에 조건문은 필요가 없을 것 같아요~!
private RecommendedSpot toRecommendedSpot(final SpotEntity spotEntity) { | ||
return new RecommendedSpot( | ||
spotEntity.getId(), | ||
fetchSpotImage(spotEntity.getId()), | ||
null, | ||
spotEntity.getSpotType().name(), | ||
spotEntity.getName(), | ||
calculateWalkingTime(spotEntity, spotEntity.getLatitude(), spotEntity.getLongitude()) | ||
); |
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.
Q3: 변환함수가 spotService에 존재하는 이유가 있을까요?
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.
없습니다 .. 추후 리팩하겠습니다 하하 ~
💡 Issue
📸 Screenshot
📄 Description
💬 To Reviewers
🔗 Reference