[REFACTOR] 웨이블존 추천 경로 리팩토링 #126
Conversation
WalkthroughGraphInit에 램프 판별용 필드와 램프/마커 기반 거리 스케일링 로직이 추가되고, WaybleDijkstraService에 폴리라인 중복 좌표 제거 및 되돌림(backtracking) 처리 헬퍼가 도입되었습니다. 일부 마커 타입이 RAMP로 변경되고 WalkingService용 단위 테스트가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant WalkingService
participant GraphInit
participant Dijkstra as WaybleDijkstraService
Client->>WalkingService: findWayblePath(startCoord, endCoord)
WalkingService->>GraphInit: getNodeMap() / getMarkerMap()
WalkingService->>Dijkstra: createWayblePath(startNodeId, endNodeId)
Dijkstra->>Dijkstra: 경로 탐색 (거리 계산: GraphInit의 rampMarkers/RAMP_PENALTY 영향)
Dijkstra->>Dijkstra: deleteDuplicateCoords()로 폴리라인 중복 제거 및 되돌림 처리
Dijkstra-->>WalkingService: WayblePathResponse(points, polyline)
WalkingService-->>Client: WayblePathResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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
🧹 Nitpick comments (5)
src/main/java/com/wayble/server/direction/init/GraphInit.java (1)
68-74: 경사로 가중치 적용 로직: 변수명 정정 및 가중치 상호작용 재검토
- 변수명:
rampMakers→rampMarkers정정 필요.- 현재 로직은 “웨이블 마커 인접 시 0.5배” 후 “램프면 5배”를 추가로 적용하여, 램프+마커 동시일 때 결과적으로 2.5배가 됩니다. 의도된 정책인지 확인이 필요합니다. 일반적으로는 램프 패널티만 적용하고, 기타 마커 혜택(예: 엘리베이터)은 별도로 적용하는 방식이 명확합니다.
최소한의 수정(변수명 정정):
- boolean isRamp = rampMakers.contains(edge.from()) || rampMakers.contains(edge.to()); + boolean isRamp = rampMarkers.contains(edge.from()) || rampMarkers.contains(edge.to());정책 확인 필요 사항:
- 램프에 대해서도 마커 0.5배 혜택을 먼저 적용한 뒤 5배 패널티를 곱하는 것이 의도인지, 혹은 램프일 경우 마커 혜택을 배제해야 하는지 명확화 바랍니다. 필요 시 아래처럼 분기하는 방식을 고려하세요.
double distance = edge.length(); if (isRamp) { distance *= 5; } else if (isWaybleMarker) { distance *= 0.5; }src/test/java/com/wayble/server/direction/service/WalkingServiceTest.java (2)
45-56: 미사용 테스트 데이터 정리 또는 검증 강화 권장
adjacencyList,markerMap을 준비하지만 실제로는 사용하지 않습니다(해당 스텁도 주석 처리). 테스트 의도를 명확히:
- 단순히
findWayblePath가 최근접 노드 계산 후 Dijkstra 서비스로 위임하는 흐름만 검증한다면 불필요한 데이터는 제거하세요.- 또는 통합도를 올리려면
graphInit.getGraph()/getMarkerMap()을 스텁하고waybleDijkstraService목을 사용하지 않거나 부분 목으로 전환하여 경로 및 가중치 영향까지 검증하세요.
65-77: 폴리라인 좌표 순서 계약 확인 필요테스트의
polyline은[lat, lon]순서로 보입니다. 반면 프로덕션 코드(WaybleDijkstraService)는[lon, lat]로 좌표를 생성합니다. 시스템 전반의 계약(lat, lon vs lon, lat)을 명확히 한 뒤 테스트/프로덕션을 일치시켜 주세요.계약이 확정되면 양쪽을 일괄 정리하는 패치를 만들어 드릴 수 있습니다.
src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java (2)
64-69: 폴리라인 좌표 순서 확인 필요: 현재 [lon, lat] 생성다른 계층(예: 테스트, API 응답 소비자)에서
[lat, lon]을 기대할 수 있습니다. 계약을 확인하고 필요 시 아래처럼 교체하세요.계약이 [lat, lon]일 경우:
- double[] fromCoord = new double[]{fromNode.lon(), fromNode.lat()}; - double[] toCoord = new double[]{toNode.lon(), toNode.lat()}; + double[] fromCoord = new double[]{fromNode.lat(), fromNode.lon()}; + double[] toCoord = new double[]{toNode.lat(), toNode.lon()};
74-88: 중복/백트래킹 제거 로직은 명확하고 안전함(LGTM), 단 장거리 성능 고려 여지현재 O(n) 역방향 탐색으로, 전체적으로 O(n^2) 최악 복잡도를 가질 수 있습니다. 매우 긴 폴리라인에서 성능 이슈 가능성은 낮지만, 필요 시 좌표를 격자화(라운딩)하여 키로 쓰는 해시 인덱스를 도입해 평균 O(1) 조회로 최적화 여지 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/wayble/server/direction/init/GraphInit.java(3 hunks)src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java(3 hunks)src/main/resources/wayble_markers.json(3 hunks)src/test/java/com/wayble/server/direction/service/WalkingServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/test/java/com/wayble/server/direction/service/WalkingServiceTest.java (1)
src/main/java/com/wayble/server/direction/service/WalkingService.java (2)
Slf4j(20-61)findWayblePath(41-51)
src/main/java/com/wayble/server/direction/init/GraphInit.java (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
buildGraph(312-334)
🔇 Additional comments (2)
src/main/resources/wayble_markers.json (1)
123-127: 마커 타입 변경 확인 및 그래프와의 정합성 검증 필요다음 ID에 대한 타입 변경이 일괄 RAMP로 변경되었습니다. 운영 데이터 기준 의도된 변경인지, 그리고 그래프 노드/엣지와 일관되게 매핑되는지 확인해 주세요.
- 1740825183: → RAMP
- 1012654209: → RAMP
- 6875654572: → RAMP
- 2747865798: → RAMP
추가 확인 권고:
- 변경된 마커들이 실제로 그래프의 노드에 근접 매핑되는지(오차/최근접 노드) 확인
- GraphInit의 경사로 가중치 적용 기준(램프 노드 집합)과 본 마커 변경이 일치하는지 검증
원하시면 저장소 스크립트를 통해 위 ID들이 그래프 노드에 매핑되는지 자동 점검하는 도구를 작성해드릴 수 있습니다.
Also applies to: 129-133, 159-163, 279-283
src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java (1)
161-163: 좌표 허용 오차 판정 isClose 적절함(LGTM)0.000001도(약 11cm) 허용은 실데이터 노이즈를 흡수하기에 합리적입니다.
src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java
Outdated
Show resolved
Hide resolved
src/test/java/com/wayble/server/direction/service/WalkingServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/wayble/server/direction/init/GraphInit.java (2)
57-59: init 단계에서 rampMarkers 유도 로직을 추가해 주세요markerMap 계산 직후 Type.RAMP 노드만 추려 rampMarkers를 채워야 isRamp 가중치가 작동합니다. 아래처럼 init()에 코드 한 블록을 추가하는 것을 제안합니다.
markerMap = findWaybleMarkers(); + // RAMP 타입 노드 집합 유도 (경사로 가중치에 사용) + this.rampMarkers = markerMap.entrySet().stream() + .filter(e -> e.getValue() == Type.RAMP) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); adjacencyList = buildAdjacencyList();
32-32: 램프 가중치가 영구 비활성화됩니다: rampMarkers가 비어 있어 isRamp가 항상 false입니다현재 rampMarkers는 빈 Set으로 초기화만 되고, 어디에서도 채워지지 않습니다. 그 결과 Line 70의 isRamp는 항상 false가 되어, “경사로 가중치 수정” 목표가 실질적으로 적용되지 않습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/wayble/server/direction/init/GraphInit.java(2 hunks)src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java
|
수고하셨습니다!! |
seung-in-Yoo
left a comment
seung-in-Yoo
left a comment
There was a problem hiding this comment.
테스트 코드 되게 깔끔하네요 수고하셨습니다!!!
✔️ 연관 이슈
📝 작업 내용
Summary by CodeRabbit