[feat] 다중 경로 추천 #143
[feat] 다중 경로 추천 #143
Conversation
Walkthrough다중 경로 추천을 위해 DTO 구조가 List에서 List로 변경되었고, 서비스 로직이 단일 경로 탐색에서 최대 N개의 대안 경로 생성·정렬·페이지네이션으로 확장되었습니다. 엣지 패널티 기반의 대체 경로 생성, 중복 제거, 정렬(환승 수/도보 거리) 로직이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TransportationService
participant RouteFinder as Dijkstra/RouteFinder
participant Penalty as EdgePenalty
Client->>TransportationService: findRoutes(request, page,size)
TransportationService->>RouteFinder: computeFirstRoute(start, end)
RouteFinder-->>TransportationService: firstRoute
loop up to MAX_ROUTES-1
TransportationService->>Penalty: penalizeEdges(firstRoute)
TransportationService->>RouteFinder: computeAlternativeRoute(with penalties)
RouteFinder-->>TransportationService: altRoute?
TransportationService->>TransportationService: dedupe/filter sort candidates
end
TransportationService->>Client: TransportationResponseDto(List<Route>, PageInfo)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (3)
src/main/java/com/wayble/server/direction/service/TransportationService.java (3)
90-111: 임시 노드 중복 생성(같은 ID -1/-2)로 그래프/노드 참조 혼선 가능startTmp/endTmp를 nodes에 추가(라인 104-106)한 뒤, buildGraph.addOriginDestinationWalkConnections에서 동일한 ID(-1/-2)를 가진 신규 노드를 다시 생성/추가하고 있어 동일 ID를 가진 서로 다른 Node 인스턴스가 nodes/graph에 혼재합니다. 이로 인해:
- nodes.stream().filter(id == -1/-2) 시 어떤 인스턴스를 집을지 비결정적
- runDijkstra의 break 조건(curr.equals(end))이 엔티티 equals/hashCode 구현에 따라 오동작할 수 있음
- 그래프 초기화(graph.put) 시 기존 리스트를 덮어쓰거나 예상치 못한 리스트 참조가 발생할 수 있음
중복 생성을 제거하고 하나의 임시 노드를 일관되게 사용하도록 정리해야 합니다.
다음과 같이 수정 제안합니다.
- 임시 노드 추가 제거(본 메서드):
- // 3. 임시 노드 추가 - nodes.add(startTmp); - nodes.add(endTmp); - - // 4. 그래프 빌드 및 여러 경로 찾기 + // 3. 그래프 빌드 및 여러 경로 찾기 TransportationGraphDto graphData = buildGraph(nodes, edges, startTmp, endTmp);
- buildGraph.addOriginDestinationWalkConnections에서 새 노드 생성 대신 전달받은 startTmp/endTmp 재사용(아래 별도 코드 블록 참고).
121-135: 임시 노드 탐색 방식 개선 — 전달받은 startTmp/endTmp 직접 사용현재 nodes에서 id -1/-2로 다시 탐색합니다. 중복 노드가 있을 경우 어떤 인스턴스를 선택할지 보장되지 않습니다. 전달받은 startTmp/endTmp를 그대로 사용하는 게 안전합니다.
- // 1. 임시 노드 찾기 - Node startNode = nodes.stream() - .filter(node -> node.getId().equals(-1L)) - .findFirst() - .orElse(null); - - Node endNode = nodes.stream() - .filter(node -> node.getId().equals(-2L)) - .findFirst() - .orElse(null); - - if (startNode == null || endNode == null) { - return new ArrayList<>(); - } + // 1. 임시 노드: 전달받은 노드를 그대로 사용 + Node startNode = startTmp; + Node endNode = endTmp;
351-357: addOriginDestinationWalkConnections에서 새 임시 노드 생성 제거 — 기존 startTmp/endTmp 재사용중복 노드 문제의 근원입니다. 아래와 같이 기존 임시 노드를 재사용하세요.
- // 1. 임시 노드 생성 - Node startNode = Node.createNode(-1L, startTmp.getStationName(), DirectionType.WALK, - startTmp.getLatitude(), startTmp.getLongitude()); - Node endNode = Node.createNode(-2L, endTmp.getStationName(), DirectionType.WALK, - endTmp.getLatitude(), endTmp.getLongitude()); + // 1. 전달받은 임시 노드 재사용 + Node startNode = startTmp; + Node endNode = endTmp; @@ - nodes.add(startNode); - nodes.add(endNode); + if (!nodes.contains(startNode)) nodes.add(startNode); + if (!nodes.contains(endNode)) nodes.add(endNode);참고: buildGraph의 노드 초기화(라인 315~323) 전에 startTmp/endTmp가 nodes에 없다면, 위에서 nodes에 안전하게 추가되므로 graph.put이 정상적으로 이루어집니다. 필요 시 buildGraph 호출부에서 nodes에 startTmp/endTmp를 미리 추가하는 방식으로도 일관성을 확보할 수 있습니다.
Also applies to: 391-393
🧹 Nitpick comments (3)
src/main/java/com/wayble/server/direction/service/TransportationService.java (3)
74-82: routeIndex 전역 인덱싱 로직 적절하나, 명세(1-based) 명시 권장startIndex + i + 1로 전역 1-based 인덱싱을 보장하고 있어 좋습니다. API 스펙에 1-based임을 명시해 혼동을 줄이는 것을 권장합니다.
240-256: 패널티 크기 하드코딩 — 구성/실험 가능하도록 외부화 제안routeIndex * 100000 패널티는 베이스 가중치(거리)에 비해 매우 크게 동작합니다. 상황/데이터셋에 따라 과도할 수 있으므로 설정값으로 외부화하고, WALK 엣지에는 적용하지 않도록 고려해 주세요.
279-299: 경로 필터/정렬 기준 보완 제안
- 환승 4회 이상 제거는 합리적입니다. 다만 사용성 측면에서 상수(4)는 설정값으로 외부화 권장.
- 정렬의 보행거리 계산이 상수(500/step) 기반이라 품질이 떨어집니다. 실제 보행 구간의 거리 합을 반영하면 더 나은 결과를 제공합니다. 현재 Step에는 좌표가 없어 즉시 계산이 어렵습니다. 선택지:
- Step에 from/to 좌표(또는 노드 ID) 포함
- 내부 계산용으로만 별도 구조 유지하고 DTO 변환 시 좌표 제거
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java(1 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
🔇 Additional comments (3)
src/main/java/com/wayble/server/direction/service/TransportationService.java (3)
41-41: MAX_ROUTES 상수 도입 적절상한을 명확히 하여 과도한 대안 경로 생성 방지에 도움이 됩니다. 기본 페이지 크기(5)와도 일관됩니다.
625-628: 버스 정보 조회 실패 처리 레벨/흐름 제어 확인 필요예외를 info 레벨로만 로깅하고 계속 진행합니다. 또한 특정 조건에서 mergeConsecutiveRoutes 전체가 빈 리스트를 반환(return new ArrayList<>())하여 상위 경로가 통째로 폐기됩니다. 의도된 정책인지 확인이 필요합니다. 사용자 경험 측면에서는 해당 단계만 보수적으로 정보 없이 노출하거나 기본값을 대입하는 편이 더 낫습니다.
원하시면, 실패 시 단계 단위 대체 처리(기본 BusInfo/SubwayInfo 삽입)로 전환하는 패치를 제안드릴 수 있습니다.
717-734: 환승 계산 로직 검증 요청현재 로직은 동일 모드 내 routeName 변경 또는 모드 변경 시 환승으로 카운트합니다. Step 병합 과정에서 routeName이 null로 남는 케이스(예: 데이터 누락)가 있으면 환승 미계산이 될 수 있습니다. 데이터 품질 이슈에 대비해 null-safe 보정(예: routeId 기반 비교, 또는 null이면 강제 환승 처리)을 고려해 주세요.
원하시면, routeId 우선 비교(없으면 routeName)로 견고하게 만드는 수정안을 제공하겠습니다.
| List<Route> routes, | ||
| PageInfo pageInfo | ||
| ) { | ||
| public record Route( | ||
| Integer routeIndex, // 경로 인덱스 | ||
| List<Step> steps // 해당 경로의 단계들 | ||
| ) {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중첩 타입 Route 이름 충돌/혼동 가능성 — DTO 타입명 변경 또는 스키마 설명 강화 제안
엔티티 com.wayble.server.direction.entity.transportation.Route 와 DTO 내부 중첩 타입 Route 이름이 동일해, 서비스 코드에서 동시에 사용할 때 가독성 저하와 혼동을 유발합니다. 현재 서비스는 DTO 쪽을 TransportationResponseDto.Route 로 완전 수식하여 회피하고 있으나, 유지보수성과 실수 방지를 위해 DTO 타입명을 RouteDto 또는 RouteSummary 등으로 바꾸는 것을 권장합니다. 최소한 OpenAPI 스키마 설명으로 routeIndex의 식별 의미(페이지네이션 포함 전역 1-based 인덱스)와 steps의 의미를 명확히 해 주세요.
다음과 같이 최소 주석/스키마를 보강하는 방법도 고려해 주세요:
- public record Route(
- Integer routeIndex, // 경로 인덱스
- List<Step> steps // 해당 경로의 단계들
- ) {}
+ public record Route(
+ @Schema(description = "전역 1부터 시작하는 경로 인덱스(페이지네이션 적용 시 누적 기준).")
+ Integer routeIndex,
+ @Schema(description = "해당 경로를 구성하는 이동 단계 리스트")
+ List<Step> steps
+ ) {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| List<Route> routes, | |
| PageInfo pageInfo | |
| ) { | |
| public record Route( | |
| Integer routeIndex, // 경로 인덱스 | |
| List<Step> steps // 해당 경로의 단계들 | |
| ) {} | |
| List<Route> routes, | |
| PageInfo pageInfo | |
| ) { | |
| public record Route( | |
| @Schema(description = "전역 1부터 시작하는 경로 인덱스(페이지네이션 적용 시 누적 기준).") | |
| Integer routeIndex, | |
| @Schema(description = "해당 경로를 구성하는 이동 단계 리스트") | |
| List<Step> steps | |
| ) {} |
| int pageSize = (request.size() != null) ? request.size() : 5; // 기본값 5로 설정 | ||
| int endIndex = Math.min(startIndex + pageSize, allRoutes.size()); | ||
| boolean hasNext = endIndex < allRoutes.size(); | ||
| Integer nextCursor = hasNext ? endIndex : null; |
There was a problem hiding this comment.
페이지네이션 경계값 검증 부족 — 음수/범위 초과 커서 처리 및 안전한 subList 필요
현재 cursor와 size에 대한 유효성 검증이 없어, 음수 입력 또는 cursor >= allRoutes.size() 일 때 subList에서 IndexOutOfBoundsException이 발생할 수 있습니다. 또한 size <= 0 입력도 방어해야 합니다.
다음과 같이 안전 가드를 추가해 주세요:
- int pageSize = (request.size() != null) ? request.size() : 5; // 기본값 5로 설정
- int endIndex = Math.min(startIndex + pageSize, allRoutes.size());
- boolean hasNext = endIndex < allRoutes.size();
- Integer nextCursor = hasNext ? endIndex : null;
+ int total = allRoutes.size();
+ int pageSize = (request.size() != null && request.size() > 0) ? request.size() : 5; // 기본값 5, 음수 방지
+ startIndex = Math.max(0, startIndex); // 음수 커서 방지
+ if (startIndex >= total) {
+ // 빈 페이지 반환 (또는 400 처리 선택)
+ TransportationResponseDto.PageInfo pageInfo = new TransportationResponseDto.PageInfo(null, false);
+ return new TransportationResponseDto(List.of(), pageInfo);
+ }
+ int endIndex = Math.min(startIndex + pageSize, total);
+ boolean hasNext = endIndex < total;
+ Integer nextCursor = hasNext ? endIndex : null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int pageSize = (request.size() != null) ? request.size() : 5; // 기본값 5로 설정 | |
| int endIndex = Math.min(startIndex + pageSize, allRoutes.size()); | |
| boolean hasNext = endIndex < allRoutes.size(); | |
| Integer nextCursor = hasNext ? endIndex : null; | |
| int total = allRoutes.size(); | |
| int pageSize = (request.size() != null && request.size() > 0) ? request.size() : 5; // 기본값 5, 음수 방지 | |
| startIndex = Math.max(0, startIndex); // 음수 커서 방지 | |
| if (startIndex >= total) { | |
| // 빈 페이지 반환 (또는 400 처리 선택) | |
| TransportationResponseDto.PageInfo pageInfo = new TransportationResponseDto.PageInfo(null, false); | |
| return new TransportationResponseDto(List.of(), pageInfo); | |
| } | |
| int endIndex = Math.min(startIndex + pageSize, total); | |
| boolean hasNext = endIndex < total; | |
| Integer nextCursor = hasNext ? endIndex : null; |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around lines 63 to 66, validate and sanitize the incoming cursor and size before
using them to compute a subList: treat null/negative cursor as 0, enforce size >
0 (default to 5 if null or <= 0), clamp startIndex to the range [0,
allRoutes.size()], compute endIndex = Math.min(startIndex + pageSize,
allRoutes.size()), and if startIndex >= allRoutes.size() return an empty list
and null nextCursor; otherwise return allRoutes.subList(startIndex, endIndex)
and set nextCursor = (endIndex < allRoutes.size()) ? endIndex : null to avoid
IndexOutOfBoundsException.
| private List<List<TransportationResponseDto.Step>> findAlternativeRoutesEfficiently( | ||
| Map<Long, List<Edge>> graph, | ||
| Node start, | ||
| Node end, | ||
| Map<Pair<Long, Long>, Integer> weightMap, | ||
| List<Node> nodes, | ||
| List<TransportationResponseDto.Step> firstRoute) { | ||
|
|
||
| List<List<TransportationResponseDto.Step>> alternativeRoutes = new ArrayList<>(); | ||
|
|
||
| // 첫 번째 경로에서 실제 사용된 엣지들을 추출 | ||
| Set<Pair<Long, Long>> usedEdges = extractActualEdgesFromRoute(firstRoute, graph); | ||
|
|
||
| // 최대 4개의 추가 경로 찾기 | ||
| for (int i = 0; i < 4 && alternativeRoutes.size() < MAX_ROUTES - 1; i++) { | ||
| // 실제 사용된 엣지들에만 패널티를 적용한 가중치 맵 생성 | ||
| Map<Pair<Long, Long>, Integer> penalizedWeightMap = createActualEdgePenalizedWeightMap(weightMap, usedEdges, i + 1); | ||
|
|
||
| // 다익스트라로 새로운 경로 찾기 | ||
| List<TransportationResponseDto.Step> newRoute = runDijkstra(graph, start, end, penalizedWeightMap, nodes); | ||
|
|
||
| if (!hasPublicTransport) { | ||
| return new ArrayList<>(); | ||
| if (newRoute.isEmpty()) { | ||
| break; | ||
| } | ||
|
|
||
| // 4. 환승 횟수 검증 (4회 이상 제외) | ||
| int transferCount = calculateTransferCount(route); | ||
| if (transferCount >= 4) { | ||
| return new ArrayList<>(); | ||
| // 첫 번째 경로와 동일한지 확인 | ||
| if (areRoutesIdentical(newRoute, firstRoute)) { | ||
| continue; | ||
| } | ||
|
|
||
| // 새로운 경로에서 사용된 엣지들도 추가 | ||
| Set<Pair<Long, Long>> newUsedEdges = extractActualEdgesFromRoute(newRoute, graph); | ||
| usedEdges.addAll(newUsedEdges); | ||
|
|
||
| alternativeRoutes.add(newRoute); | ||
| } | ||
|
|
||
| return route; | ||
| return alternativeRoutes; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
대체 경로 중복 검출 범위 제한 — 첫 경로만 아닌 기존 대체 경로와도 중복 비교 필요
현재 areRoutesIdentical은 firstRoute와만 비교합니다. 이미 수집된 alternativeRoutes와의 중복을 허용할 수 있어, 동일 경로가 중복 반환될 위험이 있습니다.
다음과 같이 중복 비교를 확장하세요.
- // 첫 번째 경로와 동일한지 확인
- if (areRoutesIdentical(newRoute, firstRoute)) {
- continue;
- }
+ // 기존 경로(첫 경로 및 이미 수집된 대체 경로)와 중복 여부 확인
+ boolean isDuplicate = areRoutesIdentical(newRoute, firstRoute)
+ || alternativeRoutes.stream().anyMatch(r -> areRoutesIdentical(r, newRoute));
+ if (isDuplicate) {
+ continue;
+ }또한, 가능하면 K-Shortest Paths(Yen/Eppstein) 알고리즘 도입을 검토하면 품질과 성능이 더 좋아집니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private List<List<TransportationResponseDto.Step>> findAlternativeRoutesEfficiently( | |
| Map<Long, List<Edge>> graph, | |
| Node start, | |
| Node end, | |
| Map<Pair<Long, Long>, Integer> weightMap, | |
| List<Node> nodes, | |
| List<TransportationResponseDto.Step> firstRoute) { | |
| List<List<TransportationResponseDto.Step>> alternativeRoutes = new ArrayList<>(); | |
| // 첫 번째 경로에서 실제 사용된 엣지들을 추출 | |
| Set<Pair<Long, Long>> usedEdges = extractActualEdgesFromRoute(firstRoute, graph); | |
| // 최대 4개의 추가 경로 찾기 | |
| for (int i = 0; i < 4 && alternativeRoutes.size() < MAX_ROUTES - 1; i++) { | |
| // 실제 사용된 엣지들에만 패널티를 적용한 가중치 맵 생성 | |
| Map<Pair<Long, Long>, Integer> penalizedWeightMap = createActualEdgePenalizedWeightMap(weightMap, usedEdges, i + 1); | |
| // 다익스트라로 새로운 경로 찾기 | |
| List<TransportationResponseDto.Step> newRoute = runDijkstra(graph, start, end, penalizedWeightMap, nodes); | |
| if (!hasPublicTransport) { | |
| return new ArrayList<>(); | |
| if (newRoute.isEmpty()) { | |
| break; | |
| } | |
| // 4. 환승 횟수 검증 (4회 이상 제외) | |
| int transferCount = calculateTransferCount(route); | |
| if (transferCount >= 4) { | |
| return new ArrayList<>(); | |
| // 첫 번째 경로와 동일한지 확인 | |
| if (areRoutesIdentical(newRoute, firstRoute)) { | |
| continue; | |
| } | |
| // 새로운 경로에서 사용된 엣지들도 추가 | |
| Set<Pair<Long, Long>> newUsedEdges = extractActualEdgesFromRoute(newRoute, graph); | |
| usedEdges.addAll(newUsedEdges); | |
| alternativeRoutes.add(newRoute); | |
| } | |
| return route; | |
| return alternativeRoutes; | |
| } | |
| private List<List<TransportationResponseDto.Step>> findAlternativeRoutesEfficiently( | |
| Map<Long, List<Edge>> graph, | |
| Node start, | |
| Node end, | |
| Map<Pair<Long, Long>, Integer> weightMap, | |
| List<Node> nodes, | |
| List<TransportationResponseDto.Step> firstRoute) { | |
| List<List<TransportationResponseDto.Step>> alternativeRoutes = new ArrayList<>(); | |
| // 첫 번째 경로에서 실제 사용된 엣지들을 추출 | |
| Set<Pair<Long, Long>> usedEdges = extractActualEdgesFromRoute(firstRoute, graph); | |
| // 최대 4개의 추가 경로 찾기 | |
| for (int i = 0; i < 4 && alternativeRoutes.size() < MAX_ROUTES - 1; i++) { | |
| // 실제 사용된 엣지들에만 패널티를 적용한 가중치 맵 생성 | |
| Map<Pair<Long, Long>, Integer> penalizedWeightMap = createActualEdgePenalizedWeightMap(weightMap, usedEdges, i + 1); | |
| // 다익스트라로 새로운 경로 찾기 | |
| List<TransportationResponseDto.Step> newRoute = runDijkstra(graph, start, end, penalizedWeightMap, nodes); | |
| if (newRoute.isEmpty()) { | |
| break; | |
| } | |
| // 기존 경로(첫 경로 및 이미 수집된 대체 경로)와 중복 여부 확인 | |
| boolean isDuplicate = areRoutesIdentical(newRoute, firstRoute) | |
| || alternativeRoutes.stream().anyMatch(r -> areRoutesIdentical(r, newRoute)); | |
| if (isDuplicate) { | |
| continue; | |
| } | |
| // 새로운 경로에서 사용된 엣지들도 추가 | |
| Set<Pair<Long, Long>> newUsedEdges = extractActualEdgesFromRoute(newRoute, graph); | |
| usedEdges.addAll(newUsedEdges); | |
| alternativeRoutes.add(newRoute); | |
| } | |
| return alternativeRoutes; | |
| } |
| private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) { | ||
| Set<Pair<Long, Long>> usedEdges = new HashSet<>(); | ||
|
|
||
| for (TransportationResponseDto.Step step : route) { | ||
| String fromName = step.from(); | ||
| String toName = step.to(); | ||
|
|
||
| for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) { | ||
| Long nodeId = entry.getKey(); | ||
| List<Edge> edges = entry.getValue(); | ||
|
|
||
| for (Edge edge : edges) { | ||
| Node fromNode = edge.getStartNode(); | ||
| Node toNode = edge.getEndNode(); | ||
|
|
||
| if ((fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) || | ||
| (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName))) { | ||
| usedEdges.add(Pair.of(fromNode.getId(), toNode.getId())); | ||
| usedEdges.add(Pair.of(toNode.getId(), fromNode.getId())); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return usedEdges; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
usedEdges 추출 정확도/성능 개선 필요 — 정류장명 매칭만으로는 오탐 가능
현재 from/to 정류장명만으로 그래프 전체를 스캔하며 usedEdges를 추출합니다. 동일한 정류장명이 여러 노드에 존재하거나, 모드/노선 구분이 다른 경우에도 매칭되어 과도한 패널티가 적용될 수 있습니다. WALK 구간까지 패널티에 포함되면 대체 경로 품질이 떨어질 수 있습니다.
- WALK 단계는 패널티 대상에서 제외
- 모드와 노선(routeName)까지 일치하는 엣지만 사용
- for (TransportationResponseDto.Step step : route) {
+ for (TransportationResponseDto.Step step : route) {
+ // WALK 단계는 패널티 제외
+ if (step.mode() == DirectionType.WALK) {
+ continue;
+ }
String fromName = step.from();
String toName = step.to();
for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) {
Long nodeId = entry.getKey();
List<Edge> edges = entry.getValue();
for (Edge edge : edges) {
Node fromNode = edge.getStartNode();
Node toNode = edge.getEndNode();
- if ((fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) ||
- (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName))) {
+ boolean nameMatch = (fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName))
+ || (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName));
+ boolean modeMatch = edge.getEdgeType() == step.mode();
+ boolean routeMatch = true;
+ if (step.routeName() != null && edge.getRoute() != null && edge.getRoute().getRouteName() != null) {
+ routeMatch = step.routeName().equals(edge.getRoute().getRouteName());
+ }
+ if (nameMatch && modeMatch && routeMatch) {
usedEdges.add(Pair.of(fromNode.getId(), toNode.getId()));
usedEdges.add(Pair.of(toNode.getId(), fromNode.getId()));
}
}
}
}더 나은 접근: runDijkstra에서 이미 생성한 pathEdges를 반환(예: PathResult { steps, usedEdges })하여 O(E) 전체 스캔을 없애는 구조로 리팩터링하는 것을 권장합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) { | |
| Set<Pair<Long, Long>> usedEdges = new HashSet<>(); | |
| for (TransportationResponseDto.Step step : route) { | |
| String fromName = step.from(); | |
| String toName = step.to(); | |
| for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) { | |
| Long nodeId = entry.getKey(); | |
| List<Edge> edges = entry.getValue(); | |
| for (Edge edge : edges) { | |
| Node fromNode = edge.getStartNode(); | |
| Node toNode = edge.getEndNode(); | |
| if ((fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) || | |
| (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName))) { | |
| usedEdges.add(Pair.of(fromNode.getId(), toNode.getId())); | |
| usedEdges.add(Pair.of(toNode.getId(), fromNode.getId())); | |
| } | |
| } | |
| } | |
| } | |
| return usedEdges; | |
| } | |
| private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) { | |
| Set<Pair<Long, Long>> usedEdges = new HashSet<>(); | |
| for (TransportationResponseDto.Step step : route) { | |
| // WALK 단계는 패널티 제외 | |
| if (step.mode() == DirectionType.WALK) { | |
| continue; | |
| } | |
| String fromName = step.from(); | |
| String toName = step.to(); | |
| for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) { | |
| Long nodeId = entry.getKey(); | |
| List<Edge> edges = entry.getValue(); | |
| for (Edge edge : edges) { | |
| Node fromNode = edge.getStartNode(); | |
| Node toNode = edge.getEndNode(); | |
| boolean nameMatch = (fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) | |
| || (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName)); | |
| boolean modeMatch = edge.getEdgeType() == step.mode(); | |
| boolean routeMatch = true; | |
| if (step.routeName() != null && edge.getRoute() != null && edge.getRoute().getRouteName() != null) { | |
| routeMatch = step.routeName().equals(edge.getRoute().getRouteName()); | |
| } | |
| if (nameMatch && modeMatch && routeMatch) { | |
| usedEdges.add(Pair.of(fromNode.getId(), toNode.getId())); | |
| usedEdges.add(Pair.of(toNode.getId(), fromNode.getId())); | |
| } | |
| } | |
| } | |
| } | |
| return usedEdges; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around lines 213 to 239, the current extractActualEdgesFromRoute scans the whole
graph and matches nodes only by station name, causing false positives and
performance issues; update it to (1) skip steps whose mode equals WALK so they
are not added to usedEdges, (2) when matching edges require mode and routeName
to match the step in addition to station names (compare
edge.getMode()/edge.getRouteName() or the corresponding Node/Edge attributes
against step.mode() and step.routeName()), and (3) instead of scanning the
entire graph here, refactor runDijkstra to return the computed usedEdges (e.g.,
extend its result to include usedEdges or PathResult { steps, usedEdges }) and
use that returned set to avoid O(E) full-graph scans.
There was a problem hiding this comment.
@KiSeungMin 감사합니다! 다중 경로 추천 기능 구현에 도움이 되었다면 기컐요. 코드 품질 개선과 성능 최적화를 통해 사용자들이 더 나은 경로 옵션을 받을 수 있게 되었네요!
(∩˃o˂∩)♡ 토끼가 행복해요~
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit