-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 성공한 루트미션에 맞는 문제 추천 기능 구현 #52
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
Conversation
|
Claude finished @eric-jy-park's task —— View job 코드 리뷰 완료리뷰 요약성공한 루트미션을 기반으로 한 문제 추천 기능이 구현되었습니다. AttemptController에 새로운 추천 API 엔드포인트가 추가되고, RecommendationServiceImpl에서 특정 도전 기록을 기반으로 유사한 난이도의 문제들을 추천하는 로직이 구현되었습니다. 새로운 예외 클래스와 에러 코드도 함께 추가되어 에러 처리가 강화되었습니다. 추천 로직은 기존 세션의 추천 리스트에서 도전한 문제의 순서를 기준으로 앞뒤 문제들을 추천하는 방식으로 구현되었습니다. 코드 리뷰 결과RecommendationNotFoundException.java:8 - issue: 잘못된 ErrorCode 사용 - RecommendationNotFoundException이 ATTEMPT_NOT_FOUND 에러 코드를 사용하고 있어 예외 의미와 불일치합니다. RECOMMENDATION_NOT_FOUND로 변경해야 합니다. RecommendationServiceImpl.java:102-120 - suggestion: 무한 루프 위험 및 성능 문제 - while 루프가 recommendations.size()를 초과할 경우 무한 루프가 발생할 수 있고, 매번 .get(startingIndex)로 접근하는 것은 비효율적입니다. for 루프를 사용하고 범위 체크를 추가하는 것이 안전합니다. |
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.
Pull Request Overview
This PR implements a route mission recommendation feature based on successful route mission attempts. The system now provides 3 similar difficulty route missions as recommendations when a user completes a route mission attempt.
- Added
getRecommendationsByUserAttemptmethod to provide recommendations based on completed attempts - Created custom exception handling for recommendation-related errors
- Integrated the recommendation service with the attempt controller endpoint
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| RecommendationServiceImpl.java | Implements core logic to find and return 3 recommendations based on attempt's recommended order |
| RecommendationService.java | Adds interface method for attempt-based recommendations |
| RecommendationNotFoundException.java | Creates custom exception for missing recommendations |
| ErrorCode.java | Adds RECOMMENDATION_NOT_FOUND error code |
| ErrorCategory.java | Adds RECOMMENDATION error category |
| AttemptController.java | Connects recommendation service to existing endpoint |
|
|
||
| public class RecommendationNotFoundException extends BusinessException { | ||
| public RecommendationNotFoundException() { | ||
| super(ErrorCode.ATTEMPT_NOT_FOUND); |
Copilot
AI
Aug 1, 2025
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.
The exception is using the wrong error code. It should use ErrorCode.RECOMMENDATION_NOT_FOUND instead of ErrorCode.ATTEMPT_NOT_FOUND since this is a RecommendationNotFoundException.
| super(ErrorCode.ATTEMPT_NOT_FOUND); | |
| super(ErrorCode.RECOMMENDATION_NOT_FOUND); |
|
|
||
| Integer targetRecommendationRecommendedOrder = targetRecommendation.getRecommendedOrder(); | ||
|
|
||
| int startingIndex = Math.max(targetRecommendationRecommendedOrder - 1, 0); |
Copilot
AI
Aug 1, 2025
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.
The logic assumes recommendations are sorted by recommendedOrder, but there's no explicit sorting. If recommendations are not naturally ordered, this could return incorrect recommendations. Consider sorting the recommendations list by recommendedOrder before processing.
| Sector sector = routeMission.getSector(); | ||
|
|
||
| recommendationResponseList.add( | ||
| RouteMissionRecommendationResponse.toDto(recommendation, routeMission, gym, attempts, sector, recommendation.getRecommendedOrder()) |
Copilot
AI
Aug 1, 2025
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.
The DTO creation logic is duplicated between this method and getRecommendationsByUserActiveSession. Consider extracting this into a private helper method to reduce code duplication.
| RouteMissionRecommendationResponse.toDto(recommendation, routeMission, gym, attempts, sector, recommendation.getRecommendedOrder()) | |
| createRouteMissionRecommendationResponse(recommendation, routeMission, gym, attempts, sector) |
|
Claude finished @eric-jy-park's task —— View job 변경사항 개요성공한 루트미션 시도를 기반으로 한 추천 기능이 구현되었습니다. AttemptController에 새로운 GET 엔드포인트 코드 리뷰 결과RecommendationServiceImpl.java:98 - issue: 정렬되지 않은 리스트 사용으로 잘못된 추천 가능 - recommendations 리스트가 recommendedOrder로 정렬되지 않은 상태에서 인덱스 기반 탐색을 수행하여 잘못된 추천이 반환될 수 있습니다. RecommendationServiceImpl.java:102-120 - suggestion: 코드 중복 및 성능 문제 - DTO 생성 로직이 getRecommendationsByUserActiveSession과 중복되고, 반복문 내에서 매번 동일한 변환 작업을 수행하여 비효율적입니다. 공통 메서드로 추출하는 것이 좋습니다. LGTM |
|
Claude finished @eric-jy-park's task —— View job 변경사항 개요성공한 루트미션 도전을 기반으로 한 추천 기능이 구현되었습니다. AttemptController에 새로운 GET 엔드포인트 코드 리뷰 결과RecommendationServiceImpl.java:98 - issue: 정렬되지 않은 리스트 사용으로 잘못된 추천 가능 - recommendations 리스트가 recommendedOrder로 정렬되지 않은 상태에서 인덱스 기반 탐색을 수행하여 잘못된 추천이 반환될 수 있습니다. RecommendationServiceImpl.java:102-120 - suggestion: 코드 중복 및 성능 문제 - DTO 생성 로직이 getRecommendationsByUserActiveSession과 중복되고, 반복문 내에서 매번 동일한 변환 작업을 수행하여 비효율적입니다. 공통 메서드로 추출하는 것이 좋습니다. 할 일 목록
|
📌 PR 제목
📋 작업 내용
✅ 체크리스트
🚨 주의사항