-
Notifications
You must be signed in to change notification settings - Fork 280
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
[Step3] 경로 조회 리뷰 요청드립니다. #277
base: misudev
Are you sure you want to change the base?
Changes from all commits
abe2a64
3f066cf
9d1c006
4571823
246bbc2
5c5866c
4e6fa93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package nextstep.subway.applicaion; | ||
|
||
import java.util.List; | ||
import nextstep.subway.applicaion.dto.PathResponse; | ||
import nextstep.subway.domain.Line; | ||
import nextstep.subway.domain.LineRepository; | ||
import nextstep.subway.domain.Path; | ||
import nextstep.subway.domain.PathFinder; | ||
import nextstep.subway.domain.Station; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Service | ||
@Transactional | ||
public class PathService { | ||
private final LineRepository lineRepository; | ||
private final StationService stationService; | ||
|
||
public PathService(LineRepository lineRepository, | ||
StationService stationService) { | ||
this.lineRepository = lineRepository; | ||
this.stationService = stationService; | ||
} | ||
|
||
public PathResponse searchPath(Long sourceId, Long targetId) { | ||
Station source = stationService.findById(sourceId); | ||
Station target = stationService.findById(targetId); | ||
List<Line> lines = lineRepository.findAll(); | ||
|
||
PathFinder pathFinder = PathFinder.from(lines); | ||
Path shortestPath = pathFinder.searchPath(source, target); | ||
return PathResponse.from(shortestPath); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package nextstep.subway.applicaion.dto; | ||
|
||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import nextstep.subway.domain.Path; | ||
|
||
public class PathResponse { | ||
private List<StationResponse> stations; | ||
private int distance; | ||
|
||
private PathResponse(List<StationResponse> stations, int distance) { | ||
this.stations = stations; | ||
this.distance = distance; | ||
} | ||
|
||
public static PathResponse from(Path path) { | ||
List<StationResponse> stationResponses = path.getStations().stream() | ||
.map(StationResponse::from) | ||
.collect(Collectors.toList()); | ||
|
||
return new PathResponse(stationResponses, path.getDistance()); | ||
} | ||
|
||
public List<StationResponse> getStations() { | ||
return stations; | ||
} | ||
|
||
public int getDistance() { | ||
return distance; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package nextstep.subway.domain; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
|
||
public class Path { | ||
private List<Station> stations; | ||
private int distance; | ||
|
||
public Path(List<Station> stations, int distance) { | ||
this.stations = stations; | ||
this.distance = distance; | ||
} | ||
|
||
public List<Station> getStations() { | ||
return Collections.unmodifiableList(stations); | ||
} | ||
|
||
public int getDistance() { | ||
return distance; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package nextstep.subway.domain; | ||
|
||
import java.util.Collection; | ||
import java.util.List; | ||
import java.util.Optional; | ||
import org.jgrapht.GraphPath; | ||
import org.jgrapht.alg.shortestpath.DijkstraShortestPath; | ||
import org.jgrapht.graph.DefaultWeightedEdge; | ||
import org.jgrapht.graph.WeightedMultigraph; | ||
|
||
public class PathFinder { | ||
private WeightedMultigraph<Station, DefaultWeightedEdge> graph = new WeightedMultigraph(DefaultWeightedEdge.class); | ||
|
||
private PathFinder() { | ||
} | ||
|
||
public static PathFinder from(List<Line> lines) { | ||
PathFinder pathFinder = new PathFinder(); | ||
pathFinder.init(lines); | ||
return pathFinder; | ||
} | ||
|
||
public void init(List<Line> lines) { | ||
lines.stream() | ||
.map(Line::getSections) | ||
.map(Sections::getSectionList) | ||
.flatMap(Collection::stream) | ||
.forEach( | ||
section -> { | ||
Station upStation = section.getUpStation(); | ||
Station downStation = section.getDownStation(); | ||
graph.addVertex(upStation); | ||
graph.addVertex(downStation); | ||
graph.setEdgeWeight(graph.addEdge(upStation, downStation), section.getDistance()); | ||
} | ||
); | ||
} | ||
|
||
public Path searchPath(Station source, Station target) { | ||
validationSearchPathParams(source, target); | ||
GraphPath<Station, DefaultWeightedEdge> shortestPath = searchShortestPath(source, target); | ||
|
||
List<Station> stations = shortestPath.getVertexList(); | ||
int distance = (int) shortestPath.getWeight(); | ||
return new Path(stations, distance); | ||
} | ||
|
||
public GraphPath<Station, DefaultWeightedEdge> searchShortestPath(Station source, Station target) { | ||
DijkstraShortestPath<Station, DefaultWeightedEdge> dijkstraShortestPath = new DijkstraShortestPath<>(graph); | ||
GraphPath<Station, DefaultWeightedEdge> graphPath = dijkstraShortestPath.getPath(source, target); | ||
|
||
return Optional.ofNullable(graphPath) | ||
.orElseThrow(() -> { | ||
throw new IllegalArgumentException("출발역과 도착역이 연결되어 있지 않습니다."); | ||
}); | ||
} | ||
|
||
private void validationSearchPathParams(Station source, Station target) { | ||
if (source.equals(target)) { | ||
throw new IllegalArgumentException("출발역과 도착역이 동일합니다."); | ||
} | ||
|
||
if (!graph.containsVertex(source) || !graph.containsVertex(target)) { | ||
throw new IllegalArgumentException("노선에 포함된 역의 경로만 조회 가능합니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,19 +63,19 @@ public void update(Station newUpStation, int minusDistance) { | |
} | ||
|
||
public Section merge(Section section) { | ||
if (!isDownStation(section.upStation)) { | ||
if (!hasDownStationAs(section.upStation)) { | ||
throw new IllegalArgumentException("합치려는 구간의 상행역이 하행역과 같아야 합니다."); | ||
} | ||
|
||
return Section.of(line, upStation, section.downStation, distance + section.distance); | ||
} | ||
|
||
|
||
public boolean isUpStation(Station station) { | ||
public boolean hasUpStationAs(Station station) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 변경된 네이밍이 의미를 조금 더 잘 전달하는 것 같아요 👍 |
||
return upStation.equals(station); | ||
} | ||
|
||
public boolean isDownStation(Station station) { | ||
public boolean hasDownStationAs(Station station) { | ||
return downStation.equals(station); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package nextstep.subway.domain; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
|
@@ -34,23 +35,30 @@ public void add(Section section) { | |
public void remove(Station station) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 적절한 사이즈로 메서드를 나누어 주셨네요 👍 |
||
validationRemoveStation(station); | ||
|
||
Optional<Section> firstSection = sections.stream() | ||
.filter(s -> s.isDownStation(station)) | ||
.findAny(); | ||
|
||
Optional<Section> secondSection = sections.stream() | ||
.filter(s -> s.isUpStation(station)) | ||
.findAny(); | ||
Optional<Section> sectionHasUpStation = findSectionHasUpStationAs(station); | ||
Optional<Section> sectionHasDownStation = findSectionHasDownStationAs(station); | ||
|
||
firstSection.ifPresent(section -> sections.remove(section)); | ||
secondSection.ifPresent(section -> sections.remove(section)); | ||
sectionHasUpStation.ifPresent(section -> sections.remove(section)); | ||
sectionHasDownStation.ifPresent(section -> sections.remove(section)); | ||
|
||
if (firstSection.isPresent() && secondSection.isPresent()) { | ||
mergeExistingSections(firstSection.get(), secondSection.get()); | ||
if (sectionHasUpStation.isPresent() && sectionHasDownStation.isPresent()) { | ||
mergeExistingSections(sectionHasUpStation.get(), sectionHasDownStation.get()); | ||
} | ||
|
||
} | ||
|
||
private Optional<Section> findSectionHasDownStationAs(Station station) { | ||
return sections.stream() | ||
.filter(s -> s.hasUpStationAs(station)) | ||
.findAny(); | ||
} | ||
|
||
private Optional<Section> findSectionHasUpStationAs(Station station) { | ||
return sections.stream() | ||
.filter(s -> s.hasDownStationAs(station)) | ||
.findAny(); | ||
} | ||
|
||
private List<Station> getStations() { | ||
List<Station> stations = new ArrayList<>(); | ||
stations.add(sections.get(0).getUpStation()); | ||
|
@@ -152,4 +160,7 @@ private void mergeExistingSections(Section firstSection, Section secondSection) | |
sections.add(firstSection.merge(secondSection)); | ||
} | ||
|
||
public List<Section> getSectionList() { | ||
return Collections.unmodifiableList(sections); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package nextstep.subway.ui; | ||
|
||
import nextstep.subway.applicaion.PathService; | ||
import nextstep.subway.applicaion.dto.PathResponse; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
@RequestMapping("/path") | ||
public class PathController { | ||
private final PathService pathService; | ||
|
||
public PathController(PathService pathService) { | ||
this.pathService = pathService; | ||
} | ||
|
||
@GetMapping | ||
public ResponseEntity<PathResponse> getPaths(@RequestParam Long source, @RequestParam Long target) { | ||
PathResponse pathResponse = pathService.searchPath(source, target); | ||
return ResponseEntity.ok().body(pathResponse); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
package nextstep.subway.acceptance; | ||
|
||
import static nextstep.subway.acceptance.LineSteps.지하철_노선_생성_요청; | ||
import static nextstep.subway.acceptance.LineSteps.지하철_노선에_지하철_구간_생성_요청; | ||
import static nextstep.subway.acceptance.PathSteps.경로_조회_요청; | ||
import static nextstep.subway.acceptance.StationSteps.지하철역_생성_요청; | ||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import io.restassured.response.ExtractableResponse; | ||
import io.restassured.response.Response; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.http.HttpStatus; | ||
|
||
@DisplayName("지하철 경로 검색") | ||
public class PathAcceptanceTest extends AcceptanceTest { | ||
private Long 교대역; | ||
private Long 강남역; | ||
private Long 양재역; | ||
private Long 남부터미널역; | ||
private Long 청량리역; | ||
private Long 회기역; | ||
|
||
private Long 일호선; | ||
private Long 이호선; | ||
private Long 신분당선; | ||
private Long 삼호선; | ||
|
||
private int 교대역_강남역_거리 = 10; | ||
private int 강남역_양재역_거리 = 5; | ||
private int 교대역_남부터미널역_거리 = 2; | ||
private int 남부터미널역_양재역_거리 = 3; | ||
private int 청량리역_회기역_거리 = 7; | ||
|
||
/** (10) | ||
* 교대역 --- *2호선* --- 강남역 | ||
* | | | ||
* *3호선* (2) *신분당선* (5) | ||
* | | | ||
* 남부터미널역 --- *3호선* --- 양재 | ||
(3) **/ | ||
|
||
@BeforeEach | ||
public void setUp() { | ||
super.setUp(); | ||
|
||
교대역 = 지하철역_생성_요청("교대역").jsonPath().getLong("id"); | ||
강남역 = 지하철역_생성_요청("강남역").jsonPath().getLong("id"); | ||
양재역 = 지하철역_생성_요청("양재역").jsonPath().getLong("id"); | ||
남부터미널역 = 지하철역_생성_요청("남부터미널역").jsonPath().getLong("id"); | ||
청량리역 = 지하철역_생성_요청("청량리역").jsonPath().getLong("id"); | ||
회기역 = 지하철역_생성_요청("회기역").jsonPath().getLong("id"); | ||
|
||
일호선 = 지하철_노선_생성_요청("1호선", "blue", 청량리역, 회기역, 청량리역_회기역_거리).jsonPath().getLong("id"); | ||
이호선 = 지하철_노선_생성_요청("2호선", "green", 교대역, 강남역, 교대역_강남역_거리).jsonPath().getLong("id"); | ||
신분당선 = 지하철_노선_생성_요청("신분당선", "red", 강남역, 양재역, 강남역_양재역_거리).jsonPath().getLong("id"); | ||
삼호선 = 지하철_노선_생성_요청("3호선", "orange", 교대역, 남부터미널역, 교대역_남부터미널역_거리).jsonPath().getLong("id"); | ||
|
||
지하철_노선에_지하철_구간_생성_요청(삼호선, createSectionCreateParams(남부터미널역, 양재역, 남부터미널역_양재역_거리)); | ||
Comment on lines
+50
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 단위 테스트의 픽스쳐 관리 방법처럼 해당 객체들의 재사용이 필요한 경우 인수 테스트용 픽스쳐를 만들어서 관리할 수 도 있겠네요! |
||
} | ||
|
||
/** | ||
* Given 지하철 노선 (2호선, 3호선, 신분당선) 을 생성하고 역, 구간을 생성한다. | ||
* When 서로 다른 두 역의 최단 거리를 조회하면 | ||
* Then 최단 거리 조회에 성공한다. | ||
*/ | ||
@DisplayName("최단 거리 조회하기") | ||
@Test | ||
void searchShortestPath() { | ||
// when | ||
ExtractableResponse<Response> response = 경로_조회_요청(남부터미널역, 강남역); | ||
|
||
// then | ||
assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); | ||
assertThat(response.jsonPath().getList("stations.id", Long.class)).containsExactly(남부터미널역, 양재역, 강남역); | ||
assertThat(response.jsonPath().getInt("distance")).isEqualTo(남부터미널역_양재역_거리 + 강남역_양재역_거리); | ||
} | ||
|
||
/** | ||
* Given 지하철 노선 (2호선, 3호선, 신분당선) 을 생성하고 역, 구간을 생성한다. | ||
* 기존 노선과 연결되지 않는 새로운 노선 (1호선)을 생성한다. | ||
* When 연결 되지 않은 두 역의 최단 거리 조회를 요청 하면 | ||
* Then 최단 거리 조회에 실패한다. | ||
*/ | ||
@DisplayName("최단 거리 조회하기 - 연결되지 않은 역을 조회 할 경우") | ||
@Test | ||
void searchShortestPathDoesNotExistPath() { | ||
// when | ||
ExtractableResponse<Response> response = 경로_조회_요청(강남역, 청량리역); | ||
|
||
// then | ||
assertThat(response.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value());; | ||
} | ||
|
||
/** | ||
* Given 지하철 노선 (2호선, 3호선, 신분당선) 을 생성하고 역, 구간을 생성한다. | ||
* When 출발역과 도착역이 동일한데 최단 거리 조회를 요청하면 | ||
* Then 최단 거리 조회에 실패한다. | ||
*/ | ||
@DisplayName("최단 거리 조회하기 - 출발역과 도착역이 동일한 경우") | ||
@Test | ||
void searchShortestPathSourceEqualsTarget() { | ||
// when | ||
ExtractableResponse<Response> response = 경로_조회_요청(강남역, 강남역); | ||
|
||
// then | ||
assertThat(response.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value());; | ||
} | ||
|
||
private Map<String, String> createSectionCreateParams(Long upStationId, Long downStationId, int distance) { | ||
Map<String, String> params = new HashMap<>(); | ||
params.put("upStationId", upStationId + ""); | ||
params.put("downStationId", downStationId + ""); | ||
params.put("distance", distance + ""); | ||
return params; | ||
} | ||
} |
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.
경로를 찾기위해 lines라는 정보가 있고, 이 정보를 이용해 경로를 찾는데 이 경로를 찾는 책임을 PathFinder가 가지고 있고 그 결과 정보는 Path가 가지고 있네요!
객체지향에서 객체는 상태와 행위로 이루어져있다고 하는데요
경로를 찾기 위한 상태인 lines와 행위인 PathFinder를 하나의 객체로 도출해볼 수는 없을까요?
만약에 이렇게 리팩터링 할 경우 기존에 만들어 두었던 테스트를 활용해서 테스트의 검증이라는 보호 속에 리팩터링을 해보시는 것을 권해드립니다!