Open
Conversation
Walkthrough서브웨이 경로 탐색을 A*에서 Floyd–Warshall 기반의 행렬 사전계산으로 전환하고, 중심 좌표 계산을 Weiszfeld 유사 반복법으로 변경했으며, 후보 선정에 공정성·효율성 가중치 기반 스코어와 검색 범위·최소 후보수 기준을 도입했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Loader as SubwayCsvLoader
participant Processor as SubwayPathProcessor
participant Matrix as PrecomputedMatrices
Client->>Loader: 애플리케이션 시작 시 데이터 로드 요청
Loader->>Processor: 서브웨이 목록 전달 (정규화·중복 제거)
Processor->>Matrix: subwayId→index 매핑 및 shortestTime/nextNode 행렬 생성
Processor->>Matrix: Floyd–Warshall로 모든 쌍 최단 경로 사전계산
Client->>Processor: findShortestPath(startId, endId) 요청
Processor->>Matrix: 인덱스 조회 및 nextNode 기반 경로 재구성
Processor-->>Client: SubwayPathResult 반환 (또는 null)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 제안 라벨
제안 검토자
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java (1)
83-87:⚠️ Potential issue | 🟠 Major
ClassPathResource#getFile()은 JAR 배포 환경에서 실패합니다.라인 86과 138에서
getFile().toPath()패턴은 fat-jar 환경에서FileNotFoundException을 유발합니다. 클래스패스 리소스는 JAR 내부에 직접 파일 경로가 없기 때문입니다.getInputStream()기반 접근이 안전합니다.📦 제안 수정안 (스트림 기반 리소스 읽기)
+import java.io.InputStreamReader; import java.io.IOException; import java.io.Reader; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; ... - try (Reader reader = Files.newBufferedReader(resource.getFile().toPath(), StandardCharsets.UTF_8)) { + try (Reader reader = new InputStreamReader(resource.getInputStream(), StandardCharsets.UTF_8)) { ... - try (Reader reader = Files.newBufferedReader(resource.getFile().toPath(), StandardCharsets.UTF_8)) { + try (Reader reader = new InputStreamReader(resource.getInputStream(), StandardCharsets.UTF_8)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java` around lines 83 - 87, Replace the File-based classpath access in loadAndSaveSubways (and any other places using ClassPathResource.getFile().toPath()) with stream-based reading: open the CSV via ClassPathResource.getInputStream(), wrap it in an InputStreamReader (with StandardCharsets.UTF_8) and pass that Reader into the CSV mapping logic (e.g., HeaderColumnNameMappingStrategy<SubwayCsvMapping> and the CSV parser you use) instead of Files.newBufferedReader(resource.getFile().toPath(), ...); remove getFile()/toPath() usages to ensure it works inside a jar.
🧹 Nitpick comments (4)
src/main/java/com/meetup/server/subway/implement/reader/SubwayReader.java (1)
41-57: 최소 후보 수 미달 시 처리 확인 필요
MAX_SEARCH_RADIUS_M에 도달해도MIN_CANDIDATE_COUNT를 충족하지 못할 경우, 현재 로직은 찾은 결과를 그대로 반환합니다. 이 동작 자체는 적절한 fallback이지만, 디버깅/모니터링을 위해 경고 로그 추가를 고려해 주세요.♻️ 로깅 추가 제안
public List<Subway> readNearbySubways(Point centerPoint) { double radius = NEAREST_SUBWAY_RADIUS_M; List<Subway> nearbySubways = new ArrayList<>(); while (radius <= MAX_SEARCH_RADIUS_M) { nearbySubways = readAllWithinRadius(centerPoint, radius); if (nearbySubways.size() >= MIN_CANDIDATE_COUNT) { break; } radius += RADIUS_INCREMENT_M; } + if (nearbySubways.size() < MIN_CANDIDATE_COUNT) { + log.warn("최소 후보 수({}) 미달: {}개 발견, 중심점: {}", + MIN_CANDIDATE_COUNT, nearbySubways.size(), centerPoint); + } + return nearbySubways; }로깅을 추가하려면 클래스 레벨에
@Slf4j어노테이션도 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/implement/reader/SubwayReader.java` around lines 41 - 57, The readNearbySubways method currently returns whatever results were found even if MIN_CANDIDATE_COUNT is not met after expanding to MAX_SEARCH_RADIUS_M; add a warning log when this fallback occurs: annotate the class with `@Slf4j`, and at the end of readNearbySubways check if nearbySubways.size() < MIN_CANDIDATE_COUNT and log a warning including the final radius (use MAX_SEARCH_RADIUS_M or the local radius variable) and the found count to aid debugging; keep existing return behavior unchanged.src/main/java/com/meetup/server/global/util/CoordinateUtil.java (1)
34-35: 상수를 클래스 레벨로 추출하는 것을 고려해 주세요.
MAX_ITERATIONS와CONVERGENCE_THRESHOLD는 UPPER_CASE 네이밍을 사용하고 있어 클래스 레벨 상수로 추출하면 관례에 더 부합하고, 추후 테스트나 유지보수 시 값 조정이 용이해집니다.♻️ 클래스 레벨 상수로 추출
`@NoArgsConstructor`(access = AccessLevel.PRIVATE) public class CoordinateUtil { + private static final int MAX_ITERATIONS = 100; + private static final double CONVERGENCE_THRESHOLD = 1e-7; + public static Point createPoint(double longitude, double latitude) {그리고 메서드 내 로컬 선언을 제거:
// Weiszfeld 알고리즘을 사용 double medianLongitude = points.stream().mapToDouble(Point::getX).average().orElse(0); double medianLatitude = points.stream().mapToDouble(Point::getY).average().orElse(0); - final int MAX_ITERATIONS = 100; - final double CONVERGENCE_THRESHOLD = 1e-7; - for (int iter = 0; iter < MAX_ITERATIONS; iter++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/global/util/CoordinateUtil.java` around lines 34 - 35, Move the method-local constants MAX_ITERATIONS and CONVERGENCE_THRESHOLD into the CoordinateUtil class as private static final fields (e.g., private static final int MAX_ITERATIONS and private static final double CONVERGENCE_THRESHOLD) and remove their local declarations inside the method; update any references in the method to use the class-level constants so the naming convention, testability, and maintainability are preserved.src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.java (1)
144-151: 경로 복원 루프에 hop 제한을 두는 것이 안전합니다.Line 144~151은
nextNode데이터 이상(사이클/자가참조) 시 탈출하지 못할 수 있습니다. 방어용 제한을 두면 장애 전파를 줄일 수 있습니다.🛡️ 제안 수정안 (루프 가드 추가)
List<Integer> path = new ArrayList<>(); int current = startIdx; + int hopCount = 0; + int maxHop = indexToSubwayId.length; while (current != endIdx) { + if (hopCount++ > maxHop) { + log.warn("[경로 복원 중단] 비정상 순환 감지. 출발역ID={} → 도착역ID={}", startSubwayId, endSubwayId); + return null; + } path.add(indexToSubwayId[current]); current = nextNode[current][endIdx]; if (current == -1) { log.warn("[경로 복원 실패] 출발역ID={} → 도착역ID={}", startSubwayId, endSubwayId); return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.java` around lines 144 - 151, The path reconstruction loop in SubwayPathProcessor (the while loop using current, endIdx, nextNode, and indexToSubwayId) needs a hop-count guard to prevent infinite loops on bad nextNode data; add a maxHop value (e.g., nextNode.length or indexToSubwayId.length), increment a counter each iteration, and if the counter exceeds maxHop log a warning including startSubwayId/endSubwayId and return null, mirroring the existing failure path so the method safely exits on cycles or self-references.src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java (1)
39-53: 정렬 comparator에서 공정성 점수를 반복 계산하고 있습니다.Line 41, Line 50에서 비교 시마다
calculateFairnessScore가 재실행됩니다. 후보역이 많아지면 정렬 비용이 불필요하게 커집니다. 목적지별 점수를 1회 계산해서 재사용하는 편이 낫습니다.⚡ 제안 수정안 (점수 사전 계산)
+ Map<Integer, Double> fairnessScoreByDestination = destinationSubwayTimeMap.entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + entry -> calculateFairnessScore(entry.getValue()) + )); + List<Integer> result = destinationSubwayTimeMap.entrySet().stream() .filter(entry -> entry.getValue().size() >= MINIMUM_PEOPLE_REQUIRED) - .sorted(Comparator.comparingDouble(entry -> calculateFairnessScore(entry.getValue()))) + .sorted(Comparator.comparingDouble(entry -> fairnessScoreByDestination.get(entry.getKey()))) .limit(MAX_SUBWAY_COUNT) .map(Map.Entry::getKey) .collect(Collectors.toList()); ... result = destinationSubwayTimeMap.entrySet().stream() - .sorted((e1, e2) -> Double.compare(calculateFairnessScore(e1.getValue()), calculateFairnessScore(e2.getValue()))) + .sorted(Comparator.comparingDouble(entry -> fairnessScoreByDestination.get(entry.getKey()))) .limit(MAX_SUBWAY_COUNT) .map(Map.Entry::getKey) .collect(Collectors.toList());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java` around lines 39 - 53, The comparator repeatedly calls calculateFairnessScore during sorting causing O(n log n) * score-cost overhead; precompute a Map<Integer, Double> of fairness scores for each entry in destinationSubwayTimeMap (using calculateFairnessScore on entry.getValue()), then use that map in both stream pipelines (the initial filter+sorted and the fallback sorted) to sort by the cached score and map keys to result, keeping MINIMUM_PEOPLE_REQUIRED and MAX_SUBWAY_COUNT logic unchanged and updating variable result accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.java`:
- Around line 45-46: The init timing is wrong: `@PostConstruct` runs before
ApplicationRunner (SubwayCsvLoader.run()) so the cache builds when DB is empty
causing findShortestPath() to stay null; replace the `@PostConstruct` usage on the
init() method in SubwayPathProcessor with an ApplicationReadyEvent-based
listener (e.g., annotate or handle ApplicationReadyEvent) so init() runs after
ApplicationRunner completes and the DB is populated, then rebuild the cache
inside that init() method to ensure correct shortest-path results.
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`:
- Around line 46-67: The current conditional allows entry when either connCount
or transferCount is zero but always calls parseSectionTimeCsv(...) and
generateTransferInfo(...), then subwayConnectionRepository.saveAll(...) and
transferInfoRepository.saveAll(...), risking duplicate inserts; modify the logic
so you only parse/generate and call saveAll for the missing dataset: when
connCount == 0 run parseSectionTimeCsv(subwayCodeMap) and
subwayConnectionRepository.saveAll(connections), and when transferCount == 0 run
generateTransferInfo(subwayCodeMap.values()) and
transferInfoRepository.saveAll(transferInfos); keep the existing subway-loading
logic (loadAndSaveSubways() / subwayRepository.findAll()) unchanged and reuse
subwayCodeMap for whichever saves are executed.
---
Outside diff comments:
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`:
- Around line 83-87: Replace the File-based classpath access in
loadAndSaveSubways (and any other places using
ClassPathResource.getFile().toPath()) with stream-based reading: open the CSV
via ClassPathResource.getInputStream(), wrap it in an InputStreamReader (with
StandardCharsets.UTF_8) and pass that Reader into the CSV mapping logic (e.g.,
HeaderColumnNameMappingStrategy<SubwayCsvMapping> and the CSV parser you use)
instead of Files.newBufferedReader(resource.getFile().toPath(), ...); remove
getFile()/toPath() usages to ensure it works inside a jar.
---
Nitpick comments:
In `@src/main/java/com/meetup/server/global/util/CoordinateUtil.java`:
- Around line 34-35: Move the method-local constants MAX_ITERATIONS and
CONVERGENCE_THRESHOLD into the CoordinateUtil class as private static final
fields (e.g., private static final int MAX_ITERATIONS and private static final
double CONVERGENCE_THRESHOLD) and remove their local declarations inside the
method; update any references in the method to use the class-level constants so
the naming convention, testability, and maintainability are preserved.
In
`@src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.java`:
- Around line 144-151: The path reconstruction loop in SubwayPathProcessor (the
while loop using current, endIdx, nextNode, and indexToSubwayId) needs a
hop-count guard to prevent infinite loops on bad nextNode data; add a maxHop
value (e.g., nextNode.length or indexToSubwayId.length), increment a counter
each iteration, and if the counter exceeds maxHop log a warning including
startSubwayId/endSubwayId and return null, mirroring the existing failure path
so the method safely exits on cycles or self-references.
In
`@src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java`:
- Around line 39-53: The comparator repeatedly calls calculateFairnessScore
during sorting causing O(n log n) * score-cost overhead; precompute a
Map<Integer, Double> of fairness scores for each entry in
destinationSubwayTimeMap (using calculateFairnessScore on entry.getValue()),
then use that map in both stream pipelines (the initial filter+sorted and the
fallback sorted) to sort by the cached score and map keys to result, keeping
MINIMUM_PEOPLE_REQUIRED and MAX_SUBWAY_COUNT logic unchanged and updating
variable result accordingly.
In `@src/main/java/com/meetup/server/subway/implement/reader/SubwayReader.java`:
- Around line 41-57: The readNearbySubways method currently returns whatever
results were found even if MIN_CANDIDATE_COUNT is not met after expanding to
MAX_SEARCH_RADIUS_M; add a warning log when this fallback occurs: annotate the
class with `@Slf4j`, and at the end of readNearbySubways check if
nearbySubways.size() < MIN_CANDIDATE_COUNT and log a warning including the final
radius (use MAX_SEARCH_RADIUS_M or the local radius variable) and the found
count to aid debugging; keep existing return behavior unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/resources/csv/SPOT_지하철역_정보.csvis excluded by!**/*.csv
📒 Files selected for processing (6)
src/main/java/com/meetup/server/global/util/CoordinateUtil.javasrc/main/java/com/meetup/server/subway/implement/processor/SubwayPathNode.javasrc/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.javasrc/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.javasrc/main/java/com/meetup/server/subway/implement/reader/SubwayReader.javasrc/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java
💤 Files with no reviewable changes (1)
- src/main/java/com/meetup/server/subway/implement/processor/SubwayPathNode.java
Comment on lines
45
to
46
| @PostConstruct | ||
| public void init() { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify lifecycle-related annotations and current init flow.
rg -n "class SubwayPathProcessor|@PostConstruct|void init\(" src/main/java
rg -n "class SubwayCsvLoader|implements ApplicationRunner|void run\(" src/main/java
rg -n "SubwayPathProcessor\.|findShortestPath\(|rebuild|refresh|init\(" src/main/java/com/meetup/server/subwayRepository: Team-MOISAM/moisam-server
Length of output: 2075
🏁 Script executed:
#!/bin/bash
# Read SubwayPathProcessor to understand init() implementation
echo "=== SubwayPathProcessor.java ==="
wc -l src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.java
sed -n '45,100p' src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.javaRepository: Team-MOISAM/moisam-server
Length of output: 2725
🏁 Script executed:
#!/bin/bash
# Read SubwayCsvLoader to understand run() implementation
echo "=== SubwayCsvLoader.java ==="
wc -l src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java
cat -n src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.javaRepository: Team-MOISAM/moisam-server
Length of output: 11174
🏁 Script executed:
#!/bin/bash
# Check for cache rebuild/refresh methods in SubwayPathProcessor
echo "=== Full SubwayPathProcessor.java ==="
wc -l src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.javaRepository: Team-MOISAM/moisam-server
Length of output: 194
초기화 시점이 잘못되어 첫 기동 시 경로 캐시가 비어있을 수 있습니다.
Line 45의 @PostConstruct는 ApplicationRunner(SubwayCsvLoader.run())보다 먼저 실행되므로, 초기 DB가 비어있는 환경에서 init() 메서드가 실행될 때 캐시가 비운 상태로 계산됩니다. 이후 SubwayCsvLoader가 CSV 데이터를 로드하고 DB에 저장하지만, SubwayPathProcessor의 캐시를 재계산하지 않으므로 findShortestPath()는 지속적으로 null을 반환합니다.
🔧 제안 수정안
+import org.springframework.boot.context.event.ApplicationReadyEvent;
+import org.springframework.context.event.EventListener;
...
- `@PostConstruct`
- public void init() {
+ `@EventListener`(ApplicationReadyEvent.class)
+ public void init() {ApplicationReadyEvent는 모든 ApplicationRunner 실행 완료 후에 발생하므로, 이 시점에 init()을 실행하면 DB가 CSV 데이터로 채워진 상태에서 캐시를 올바르게 구축할 수 있습니다.
📝 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.
Suggested change
| @PostConstruct | |
| public void init() { | |
| import org.springframework.boot.context.event.ApplicationReadyEvent; | |
| import org.springframework.context.event.EventListener; | |
| `@EventListener`(ApplicationReadyEvent.class) | |
| public void init() { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/meetup/server/subway/implement/processor/SubwayPathProcessor.java`
around lines 45 - 46, The init timing is wrong: `@PostConstruct` runs before
ApplicationRunner (SubwayCsvLoader.run()) so the cache builds when DB is empty
causing findShortestPath() to stay null; replace the `@PostConstruct` usage on the
init() method in SubwayPathProcessor with an ApplicationReadyEvent-based
listener (e.g., annotate or handle ApplicationReadyEvent) so init() runs after
ApplicationRunner completes and the DB is populated, then rebuild the cache
inside that init() method to ensure correct shortest-path results.
Comment on lines
46
to
67
| if (connCount == 0 || transferCount == 0) { | ||
| log.info("지하철 구간 및 환승 데이터 초기화/업데이트를 시작합니다. (기존 역 데이터 보존)"); | ||
|
|
||
| // subwayConnectionRepository.deleteAllInBatch(); | ||
| // transferInfoRepository.deleteAllInBatch(); | ||
|
|
||
| Map<String, Subway> subwayCodeMap = new HashMap<>(); | ||
|
|
||
| if (subwayCount == 0) { | ||
| subwayCodeMap = loadAndSaveSubways(); | ||
| } else { | ||
| List<Subway> existingSubways = subwayRepository.findAll(); | ||
| for (Subway subway : existingSubways) { | ||
| subwayCodeMap.put(normalizeCode(subway.getCode()), subway); | ||
| } | ||
| } | ||
|
|
||
| List<SubwayConnection> connections = parseSectionTimeCsv(subwayCodeMap); | ||
| subwayConnectionRepository.saveAll(connections); | ||
|
|
||
| List<TransferInfo> transferInfos = generateTransferInfo(subwayCodeMap.values()); | ||
| transferInfoRepository.saveAll(transferInfos); |
There was a problem hiding this comment.
부분 복구 분기에서 기존 데이터 중복 저장 위험이 있습니다.
Line 46 조건은 한쪽 테이블만 비어도 진입하지만, Line 63~67에서 connection/transfer를 모두 saveAll합니다.
예: connCount == 0 && transferCount > 0인 경우 transfer를 다시 저장해 중복 데이터 또는 유니크 제약 충돌이 발생할 수 있습니다.
🧩 제안 수정안 (필요한 테이블만 채우기)
- if (connCount == 0 || transferCount == 0) {
+ boolean needsConnections = connCount == 0;
+ boolean needsTransfers = transferCount == 0;
+ if (needsConnections || needsTransfers) {
log.info("지하철 구간 및 환승 데이터 초기화/업데이트를 시작합니다. (기존 역 데이터 보존)");
...
- List<SubwayConnection> connections = parseSectionTimeCsv(subwayCodeMap);
- subwayConnectionRepository.saveAll(connections);
+ List<SubwayConnection> connections = Collections.emptyList();
+ if (needsConnections) {
+ connections = parseSectionTimeCsv(subwayCodeMap);
+ subwayConnectionRepository.saveAll(connections);
+ }
- List<TransferInfo> transferInfos = generateTransferInfo(subwayCodeMap.values());
- transferInfoRepository.saveAll(transferInfos);
+ List<TransferInfo> transferInfos = Collections.emptyList();
+ if (needsTransfers) {
+ transferInfos = generateTransferInfo(subwayCodeMap.values());
+ transferInfoRepository.saveAll(transferInfos);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`
around lines 46 - 67, The current conditional allows entry when either connCount
or transferCount is zero but always calls parseSectionTimeCsv(...) and
generateTransferInfo(...), then subwayConnectionRepository.saveAll(...) and
transferInfoRepository.saveAll(...), risking duplicate inserts; modify the logic
so you only parse/generate and call saveAll for the missing dataset: when
connCount == 0 run parseSectionTimeCsv(subwayCodeMap) and
subwayConnectionRepository.saveAll(connections), and when transferCount == 0 run
generateTransferInfo(subwayCodeMap.values()) and
transferInfoRepository.saveAll(transferInfos); keep the existing subway-loading
logic (loadAndSaveSubways() / subwayRepository.findAll()) unchanged and reuse
subwayCodeMap for whichever saves are executed.
anxi01
force-pushed
the
feat/MOISAM-254
branch
from
ff8c8fd to
8124fca
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java (3)
56-61: 로그 출력 내용 검토Line 57-58에서
destinationSubwayTimeMap.keySet()전체를 로깅하면 후보역이 많을 경우 로그가 과도하게 길어질 수 있습니다. 운영 환경에서 로그 가독성을 위해 개수만 출력하거나 일부만 출력하는 것을 고려해 보세요.♻️ 로그 간소화 제안
if (result.isEmpty()) { - log.warn("[후보역 없음] 지하철로 도달 가능한 중간지점을 찾을 수 없습니다. | 전체 후보역ID={}", - destinationSubwayTimeMap.keySet()); + log.warn("[후보역 없음] 지하철로 도달 가능한 중간지점을 찾을 수 없습니다. | 전체 후보역 수={}", + destinationSubwayTimeMap.size()); } else { log.info("[중간지점 확정] 최종 후보역 목록 생성 완료 | 후보군: {}", result); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java` around lines 56 - 61, The warning log currently prints the entire destinationSubwayTimeMap.keySet() which can be huge; change the log to print a concise summary instead (e.g., destinationSubwayTimeMap.size() and at most the first N keys or a sampled subset) and update the log message generation used in the if branch that references destinationSubwayTimeMap.keySet(); similarly ensure the else branch logging of result is bounded (e.g., print result.size() and a limited sample) to avoid excessively long log lines in SubwayProcessor.java where log.warn and log.info are called.
39-54: 정렬 로직 중복 개선 제안Lines 41과 50에서 동일한 공정성 점수 기반 정렬 로직이 중복됩니다. 또한 두 곳에서 서로 다른 Comparator 스타일을 사용하고 있어 일관성이 부족합니다. 공통 Comparator를 추출하여 중복을 제거하고 가독성을 높이는 것을 권장합니다.
♻️ 중복 제거 리팩토링 제안
public List<Integer> findTopFairSubways(Map<StartPoint, List<SubwayPathResult>> startPointToSubwayPaths) { Map<Integer, List<Integer>> destinationSubwayTimeMap = new HashMap<>(); startPointToSubwayPaths.forEach((startPoint, subwayPaths) -> subwayPaths.forEach(subwayPath -> { int destinationId = subwayPath.path().getLast(); destinationSubwayTimeMap .computeIfAbsent(destinationId, k -> new ArrayList<>()) .add(subwayPath.totalTime()); }) ); + Comparator<Map.Entry<Integer, List<Integer>>> byFairnessScore = + Comparator.comparingDouble(entry -> calculateFairnessScore(entry.getValue())); + List<Integer> result = destinationSubwayTimeMap.entrySet().stream() .filter(entry -> entry.getValue().size() >= MINIMUM_PEOPLE_REQUIRED) - .sorted(Comparator.comparingDouble(entry -> calculateFairnessScore(entry.getValue()))) + .sorted(byFairnessScore) .limit(MAX_SUBWAY_COUNT) .map(Map.Entry::getKey) .collect(Collectors.toList()); if (result.isEmpty() && !destinationSubwayTimeMap.isEmpty()) { log.info("[검색 조건 완화] 최소 참여 인원({}) 미달 -> 전체 {}개 후보역 대상으로 중간지점 산출", MINIMUM_PEOPLE_REQUIRED, destinationSubwayTimeMap.size()); result = destinationSubwayTimeMap.entrySet().stream() - .sorted((e1, e2) -> Double.compare(calculateFairnessScore(e1.getValue()), calculateFairnessScore(e2.getValue()))) + .sorted(byFairnessScore) .limit(MAX_SUBWAY_COUNT) .map(Map.Entry::getKey) .collect(Collectors.toList()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java` around lines 39 - 54, Extract a shared Comparator for fairness and reuse it in both stream pipelines to remove duplication and ensure consistent comparator style: create a variable like fairnessComparator = Comparator.comparingDouble(entry -> calculateFairnessScore(entry.getValue())) and use it in the first stream (where you currently call .sorted(Comparator.comparingDouble(...))) and in the fallback stream (replacing the manual Double.compare lambda), keeping the rest of the logic with destinationSubwayTimeMap, MINIMUM_PEOPLE_REQUIRED, MAX_SUBWAY_COUNT, calculateFairnessScore and result unchanged.
21-22: 가중치 상수 설정 방식 고려현재 가중치가 하드코딩되어 있습니다. 향후 운영 환경에서 공정성과 효율성 간의 균형을 조정해야 할 경우,
@Value어노테이션을 통해 application.yml에서 설정 가능하도록 외부화하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java` around lines 21 - 22, Replace the hard-coded static constants FAIRNESS_WEIGHT and EFFICIENCY_WEIGHT in SubwayProcessor with configurable instance fields injected from application.yml using Spring's `@Value` (e.g. `@Value`("${subway.weights.fairness:0.6}") private double fairnessWeight; and similarly for efficiencyWeight), remove static/final modifiers if present, and update any usages to refer to the instance fields; ensure sensible defaults in the annotation so behavior remains unchanged if the properties are missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java`:
- Around line 56-61: The warning log currently prints the entire
destinationSubwayTimeMap.keySet() which can be huge; change the log to print a
concise summary instead (e.g., destinationSubwayTimeMap.size() and at most the
first N keys or a sampled subset) and update the log message generation used in
the if branch that references destinationSubwayTimeMap.keySet(); similarly
ensure the else branch logging of result is bounded (e.g., print result.size()
and a limited sample) to avoid excessively long log lines in
SubwayProcessor.java where log.warn and log.info are called.
- Around line 39-54: Extract a shared Comparator for fairness and reuse it in
both stream pipelines to remove duplication and ensure consistent comparator
style: create a variable like fairnessComparator =
Comparator.comparingDouble(entry -> calculateFairnessScore(entry.getValue()))
and use it in the first stream (where you currently call
.sorted(Comparator.comparingDouble(...))) and in the fallback stream (replacing
the manual Double.compare lambda), keeping the rest of the logic with
destinationSubwayTimeMap, MINIMUM_PEOPLE_REQUIRED, MAX_SUBWAY_COUNT,
calculateFairnessScore and result unchanged.
- Around line 21-22: Replace the hard-coded static constants FAIRNESS_WEIGHT and
EFFICIENCY_WEIGHT in SubwayProcessor with configurable instance fields injected
from application.yml using Spring's `@Value` (e.g.
`@Value`("${subway.weights.fairness:0.6}") private double fairnessWeight; and
similarly for efficiencyWeight), remove static/final modifiers if present, and
update any usages to refer to the instance fields; ensure sensible defaults in
the annotation so behavior remains unchanged if the properties are missing.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/meetup/server/subway/implement/processor/SubwayProcessor.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java (1)
44-61:⚠️ Potential issue | 🟠 Major부분 초기화 상태를 복구하지 못합니다.
Line 48의 조건(
subwayCount == 0 && connCount == 0 && transferCount == 0) 때문에, 예를 들어 연결/환승만 비어있는 경우에도 로더가 아무 작업도 하지 않습니다. 이 경우 로컬 데이터가 반쯤 깨진 상태로 고정됩니다.🛠️ 제안 수정안
`@Override` public void run(ApplicationArguments args) throws Exception { long subwayCount = subwayRepository.count(); long connCount = subwayConnectionRepository.count(); long transferCount = transferInfoRepository.count(); - if (subwayCount == 0 && connCount == 0 && transferCount == 0) { - log.info("지하철 데이터가 모두 비어있어 초기화를 시작합니다."); - - Map<String, Subway> subwayCodeMap = loadAndSaveSubways(); - - List<SubwayConnection> connections = parseSectionTimeCsv(subwayCodeMap); - subwayConnectionRepository.saveAll(connections); - - List<TransferInfo> transferInfos = generateTransferInfo(subwayCodeMap.values()); - transferInfoRepository.saveAll(transferInfos); + boolean needsSubways = subwayCount == 0; + boolean needsConnections = connCount == 0; + boolean needsTransfers = transferCount == 0; + if (needsSubways || needsConnections || needsTransfers) { + log.info("지하철 데이터 초기화/보강 시작 - 역:{}, 구간:{}, 환승:{}", + needsSubways, needsConnections, needsTransfers); + + Map<String, Subway> subwayCodeMap = needsSubways + ? loadAndSaveSubways() + : subwayRepository.findAll().stream() + .collect(Collectors.toMap(Subway::getCode, s -> s, (a, b) -> a)); + + List<SubwayConnection> connections = Collections.emptyList(); + if (needsConnections) { + connections = parseSectionTimeCsv(subwayCodeMap); + subwayConnectionRepository.saveAll(connections); + } + + List<TransferInfo> transferInfos = Collections.emptyList(); + if (needsTransfers) { + transferInfos = generateTransferInfo(subwayCodeMap.values()); + transferInfoRepository.saveAll(transferInfos); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java` around lines 44 - 61, Current all-or-nothing check (subwayCount == 0 && connCount == 0 && transferCount == 0) prevents recovering partial datasets; change logic to handle each missing piece independently: check each repository count separately, and for each zero count do the minimal work to populate it — if subwayCount == 0 call loadAndSaveSubways() to create and persist subways and get subwayCodeMap; otherwise build subwayCodeMap from existing Subway rows (e.g., subwayRepository.findAll() -> map by code) so you can always produce connections/transfers; if connCount == 0 call parseSectionTimeCsv(subwayCodeMap) and save the connections; if transferCount == 0 call generateTransferInfo(subwayCodeMap.values()) and save transferInfos; keep using subwayConnectionRepository.saveAll(...) and transferInfoRepository.saveAll(...) for persistence.
🧹 Nitpick comments (1)
src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java (1)
76-76:ClassPathResource#getFile()는 JAR 패키징 환경에서 실패합니다.
resource.getFile().toPath()호출은 JAR 내부의 리소스에서는 파일 시스템 경로가 없어FileNotFoundException을 던집니다. Spring Framework의 권장 방식인getInputStream()으로 변경하세요.수정 예시:
try (InputStream inputStream = resource.getInputStream(); Reader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8)) { // CSV 처리 로직 }필요시
import java.io.InputStreamReader;를 추가하세요. 같은 패턴이ParkingLotCsvLoader.java(45번 줄)에도 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java` at line 76, Replace the use of resource.getFile().toPath() in SubwayCsvLoader (and likewise in ParkingLotCsvLoader) with the Spring-safe resource.getInputStream() pattern: open the resource InputStream, wrap it in an InputStreamReader with StandardCharsets.UTF_8, and use that Reader in the try-with-resources block (add import for java.io.InputStreamReader if missing) so the CSV loading works from inside JARs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`:
- Around line 64-71: The normalizeCode(String code) method must not return null
because its result is used directly as map keys and persisted as the record
code; change normalizeCode to never return null (e.g., return an empty string or
trimmed value) and then update the callers that use its result when building
keys and persisting records to explicitly skip/ignore records with blank/empty
codes (check String.isBlank() on the normalized value) so invalid/blank CSV
codes are filtered out before creating keys or saving to storage.
---
Duplicate comments:
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`:
- Around line 44-61: Current all-or-nothing check (subwayCount == 0 && connCount
== 0 && transferCount == 0) prevents recovering partial datasets; change logic
to handle each missing piece independently: check each repository count
separately, and for each zero count do the minimal work to populate it — if
subwayCount == 0 call loadAndSaveSubways() to create and persist subways and get
subwayCodeMap; otherwise build subwayCodeMap from existing Subway rows (e.g.,
subwayRepository.findAll() -> map by code) so you can always produce
connections/transfers; if connCount == 0 call parseSectionTimeCsv(subwayCodeMap)
and save the connections; if transferCount == 0 call
generateTransferInfo(subwayCodeMap.values()) and save transferInfos; keep using
subwayConnectionRepository.saveAll(...) and transferInfoRepository.saveAll(...)
for persistence.
---
Nitpick comments:
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`:
- Line 76: Replace the use of resource.getFile().toPath() in SubwayCsvLoader
(and likewise in ParkingLotCsvLoader) with the Spring-safe
resource.getInputStream() pattern: open the resource InputStream, wrap it in an
InputStreamReader with StandardCharsets.UTF_8, and use that Reader in the
try-with-resources block (add import for java.io.InputStreamReader if missing)
so the CSV loading works from inside JARs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java
Comment on lines
+64
to
+71
| private String normalizeCode(String code) { | ||
| if (code == null || code.isBlank()) return null; | ||
| try { | ||
| return String.valueOf(Integer.parseInt(code.trim())); | ||
| } catch (NumberFormatException e) { | ||
| return code.trim(); | ||
| } | ||
| } |
There was a problem hiding this comment.
정규화 결과가 null일 때 무효 역 데이터가 저장될 수 있습니다.
Line 65에서 null을 반환하는데, Line 91/103에서 그대로 키로 사용하고 Line 94/106에 code로 저장합니다. CSV에 공백 코드가 있으면 무효 레코드 저장 또는 제약조건 충돌로 초기화 실패가 발생할 수 있습니다.
🛡️ 제안 수정안
String fromCode = normalizeCode(csvLine.getFromCode());
-if (!subwayCodeMap.containsKey(fromCode)) {
+if (fromCode == null) {
+ log.warn("역 코드 누락으로 from 역 스킵 - name: {}", csvLine.getFromName());
+} else if (!subwayCodeMap.containsKey(fromCode)) {
Subway fromSubway = Subway.builder()
.name(csvLine.getFromName())
.code(fromCode)
@@
String toCode = normalizeCode(csvLine.getToCode());
-if (!subwayCodeMap.containsKey(toCode)) {
+if (toCode == null) {
+ log.warn("역 코드 누락으로 to 역 스킵 - name: {}", csvLine.getToName());
+} else if (!subwayCodeMap.containsKey(toCode)) {
Subway toSubway = Subway.builder()
.name(csvLine.getToName())
.code(toCode)Also applies to: 90-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/meetup/server/subway/infrastructure/csv/SubwayCsvLoader.java`
around lines 64 - 71, The normalizeCode(String code) method must not return null
because its result is used directly as map keys and persisted as the record
code; change normalizeCode to never return null (e.g., return an empty string or
trimmed value) and then update the callers that use its result when building
keys and persisting records to explicitly skip/ignore records with blank/empty
codes (check String.isBlank() on the normalized value) so invalid/blank CSV
codes are filtered out before creating keys or saving to storage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Why - 해결하려는 문제가 무엇인가요?
20분, 40분vs50분, 50분일 경우 후자가 반환됨)✅ What - 무엇이 변경됐나요?
🛠️ How - 어떻게 해결했나요?
SubwayPathProcessor: Floyd-Warshall 알고리즘으로 계산된 거리 행렬을 통해 경로 조회 시 O(1) 탐색SubwayProcessor:score = α × 표준편차 + β × 평균소요시간으로 역 선정CoordinateUtil: Weiszfeld 알고리즘으로 참여자 좌표들의 기하학적 중심 반복 수렴 계산🖼️ Attachment
💬 기타 코멘트
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항