Skip to content
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

Merged
merged 13 commits into from
Jan 24, 2025

Conversation

ckkim817
Copy link
Member

💡 Issue

📸 Screenshot

📄 Description

💬 To Reviewers

🔗 Reference

@ckkim817 ckkim817 added ✨ FEAT 새로운 기능 추가 🐈‍⬛ 창균 labels Jan 24, 2025
@ckkim817 ckkim817 self-assigned this Jan 24, 2025
@ckkim817 ckkim817 requested a review from gahyuun as a code owner January 24, 2025 07:28
@gahyuun gahyuun added the size/L label Jan 24, 2025
Copy link
Member

@gahyuun gahyuun left a 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q3: 주석하신 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

토큰이 필요 없는 API에서도 에러를 던지진 않지만 해당 에러 문구가 출력돼서 로그가 오염되는 것 같아 일단 주석 처리하고 추후에 수정하려고 했습니당

Comment on lines +146 to +157
// 가변 리스트 생성
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);
Copy link
Member

@gahyuun gahyuun Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q3: 이 로직 따로 함수로 분리 안하신 이유가 있을까요!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

없습니다 .. 추후 리팩하겠습니다 하하 ~

Comment on lines 260 to 262
if (preferenceEntity == null) {
return score;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q1: preferenceEntity가 없을 때 위에서 처리해준 거 아닌가요!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞아요 ㄷㄷ 수정했습니다 ㅠ

Comment on lines 277 to 287
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;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q3: 어떤 부분은 함수로 분리되어있고 어떤 부분은 조건문 내에 존재하는데 분리 기준이 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

없습니다 .. 추후 리팩하겠습니다 하하 ~

Comment on lines 178 to 197
// 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;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: 추후 comparator 함수로 분리해도 좋을 것 같아요~

Comment on lines +414 to +415
if (isRecentLocal && spotEntity.getLocalAcornCount() >= 4) {
acornScore += 5.0; // TODO: 매직 넘버 yml로 옮기기
Copy link
Member

@gahyuun gahyuun Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: isRecentLocal이 true면 도토리 개수가 4개 이상이므로 뒤에 조건문은 필요가 없을 것 같아요~!

Comment on lines 426 to 434
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())
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q3: 변환함수가 spotService에 존재하는 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

없습니다 .. 추후 리팩하겠습니다 하하 ~

@ckkim817 ckkim817 merged commit abdd001 into develop Jan 24, 2025
1 check passed
@ckkim817 ckkim817 deleted the feat/#57 branch January 24, 2025 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 위치 기반 장소 추천 알고리즘 구현
2 participants