[2단계 - 경로 조회 기능] 채채(신채원) 미션 제출합니다. #203

chaewon121 · 2023-05-22T08:44:18Z

No description provided.

Gomding

안녕하세요 채채!
2단계 열심히 구현해주셨군요
다음 요청엔 1단계 merge 하며 남긴 피드백도 함께 반영해주세요~

Gomding · 2023-05-22T14:29:21Z

src/main/java/subway/application/ShortestPathService.java

+    private int calculate(Distance distance) {
+        if (distance.isShorterSame(10)) {
+            return 1250;
+        }
+
+        if (distance.isShorterSame(50)) {
+            return 1250 + over10under50(distance.minusDistance(10).getDistance());
+        }
+        return 1250 + over10under50(40) + over50(distance.minusDistance(50).getDistance());
+    }
+
+    private int over10under50(int distance) {
+        return (int) ((Math.ceil((distance - 1) / 5) + 1) * 100);
+    }
+
+    private int over50(int distance) {
+        return (int) ((Math.ceil((distance - 1) / 8) + 1) * 100);
+    }


해당 부분은 도메인 요구사항으로 보이네요!

도메인 레이어에 구현해보면 어떨까요?

도메인 레이어로 수정하였습니다!

Gomding · 2023-05-22T14:41:54Z

src/main/java/subway/application/ShortestPathService.java

+        WeightedMultigraph<String, DefaultWeightedEdge> graph
+                = new WeightedMultigraph(DefaultWeightedEdge.class);


WeightedMultigraph 의 vertex 는 역 이름을 사용하신 이유가 있을까요?

다시 고민해본 결과 아이디로 하는 것이 맞다고 판단하여 수정하였습니다!

아이디로 하는것이 맞다고 판단한 배경도 궁금해요~ 😄

Station 객체를 vertex 로 사용해볼 수 있지 않을까요?

Gomding · 2023-05-22T14:42:15Z

src/main/java/subway/application/ShortestPathService.java

+        this.stationDao = stationDao;
+    }
+
+    public ShortestPathResponse getDijkstraShortestPath(final ShortestPathRequest shortestPathRequest) {


읽기 전용 트랜잭션을 적용해보는건 어떨까요?

읽기전용 트렌젝셔널과, 일반트렌젝셔널 모두 적용 하였습니다. 감사합니다!

Gomding · 2023-05-22T14:43:19Z

src/main/java/subway/application/ShortestPathService.java

+        WeightedMultigraph<String, DefaultWeightedEdge> graph
+                = new WeightedMultigraph(DefaultWeightedEdge.class);


라이브러리는 언제든 변경될 수 있다고 생각해요 :)
추상화해보는건 어떨까요?

인터페이스로 분리하였습니다!

Gomding · 2023-05-22T14:44:00Z

src/main/java/subway/ui/ShortestPathController.java

+
+@RestController
+@RequestMapping("/shortestPath")
+public class ShortestPathController {


테스트 코드가 필요해보여요!

테스트코드 추가하겠습니다!

Gomding · 2023-05-22T14:44:21Z

src/main/java/subway/ui/ShortestPathController.java

+
+
+@RestController
+@RequestMapping("/shortestPath")


path가 카멜케이스인 이유가 궁금해요!

찾아보니 pathVariable은 카멜케이스로 쓰고 url 은 카멜케이스를 쓰지 않는군요..! 수정하겠습니다!

Gomding · 2023-05-22T14:45:32Z

src/main/java/subway/ui/ShortestPathController.java

+
+    @GetMapping
+    public ResponseEntity<ShortestPathResponse> createStation(@Valid @RequestBody ShortestPathRequest shortestPathRequest) {
+        ShortestPathResponse path = shortestPathService.getDijkstraShortestPath(shortestPathRequest);


Controller 는 service 에게 다익스트라 알고리즘을 사용해서 최소 경로를 요청하는 것보다는
단순하게 최소경로 요청이 맞다고 생각하는데
채채는 어떻게 생각하시나요?

같은 의견입니다! 인터페이스를 분리함과 동시에 getDijkstraShortestPath 라는 함수이름도 getPath로 수정하였습니다! 꼼꼼학 봐주셔서 감사합니다!

Gomding

안녕하세요 채채!
리뷰 요청기간이 지나서 이번 미션은 이만 Merge 하겠습니다~
추가로 남긴 코멘트가 있으니 확인 부탁드려요 🙏
2단계 남은 미션도 화이팅입니다 🔥

궁금하신 점 있으시면 언제든 편하게 DM 남겨주세요 😄

Gomding · 2023-06-07T14:10:54Z

src/main/java/subway/domain/path/ShortestDistancePathStrategy.java

+        setGraph(allSections, graph);
+        GraphPath<Long, DefaultWeightedEdge> path = getPath(startStationId, targetStationId, graph);
+
+        return new AbstractMap.SimpleEntry<>(path.getVertexList(), new Distance((int) path.getWeight()));


Map 자료구조로 반환해야할 이유가 있었을까요?! 🤔

경로(List) 와 거리(Distance)를 가지는 객체를 반환해보면 어떨까요~
1단계 때 쭉 해오신것 처럼요~

Gomding · 2023-06-07T14:11:48Z

src/main/java/subway/domain/path/ShortestDistancePathStrategy.java

+        return new AbstractMap.SimpleEntry<>(path.getVertexList(), new Distance((int) path.getWeight()));
+    }
+
+    private void setGraph(List<Section> allSections, WeightedMultigraph<Long, DefaultWeightedEdge> graph) {


지하철 이라는 도메인에서는 Graph 를 세팅하는 것일까요 노선도 경로를 세팅하는 것일까요? 😄

Gomding · 2023-06-07T14:14:13Z

src/main/java/subway/domain/path/PathStrategy.java

+import java.util.List;
+import java.util.Map;
+
+public interface PathStrategy {


PathStrategy 로 해주신것은 나중에 최단경로 조회 외에도 최소환승경로 조회 같은 전략을 갈아끼우기 위해서 였을까요~?

Gomding · 2023-06-07T14:15:07Z

src/main/java/subway/domain/subway/Distance.java

+    public Distance plusDistance(final int distance) {
+        return new Distance(this.distance += distance);
+    }


값 객체끼리 더해주기 위한 메서드 활용 👍

+)
아래 두 코드는 어떤 차이가 있을까요?

new Distance(this.distance += distance); new Distance(this.distance + distance);

Gomding · 2023-06-07T14:16:42Z

src/main/java/subway/domain/subway/Distance.java

+    public boolean isShorterSame(final int distance) {
+        return this.distance < distance;
+    }


메서드 이름과 내부 로직이 일치하고 있을까요 🤔

Gomding · 2023-06-07T14:28:50Z

src/test/java/subway/ui/ShortestPathControllerTest.java

+    @Test
+    void 최소경로를_가져온다() {
+        PathRequest pathRequest = new PathRequest(station1Id, station3Id);


테스트 잘 추가해주셨네요!!

Gomding · 2023-06-07T14:30:49Z

src/test/java/subway/domain/fee/NormalFeeStrategyTest.java

+    @Test
+    void 요금_계산_10_50() {
+        NormalFeeStrategy feeStrategy = new NormalFeeStrategy();
+        Distance distance = new Distance(16); // 예시 거리: 30


예시 거리는 무엇을 의미하고 있는지 궁금하군요! 😄

Gomding · 2023-06-07T14:38:18Z

src/test/java/subway/dao/SectionDaoTest.java

+    private Section findById(Long id) {
+        final String sql = "SELECT se.id AS section_id, se.distance, se.line_id, " +


테스트를 위해 테스트에만 정의한 메서드로 보이네요 💯
쉽지 않은 구현이었을텐데 고생 많으셨습니다~!!

개인적으로는 findById(식별자로 조회해오는 메서드) 같이 기본적이고 사용될 가능성이 있는 메서드라면
꼭 테스트에 정의하지 않아도 된다고 생각해요~

하지만.. 개인적인 취향인 관계로
엄격하게 나눈다면 채채가 구현해준 방법도 좋습니다~!!

Gomding · 2023-06-07T14:52:44Z

src/main/java/subway/dao/dto/SectionStationResultMap.java

+//package subway.dao.dto;
+//
+//public class SectionStationResultMap {
+//
+//    private final Long sectionId;


안쓰는 파일이라면 삭제해주셔도 되겠어요~

Gomding · 2023-06-07T14:54:24Z

src/main/java/subway/config/Config.java

+import subway.domain.path.ShortestDistancePathStrategy;
+
+@Configuration
+public class Config {


Config 라는 이름의 클래스는 설정 이라는 단순한 정보만 제공할 수 있겠어요 😢

feat: 최단경로와 요금 계산 추가

b08a4b4

chaewon121 changed the title ~~feat: 최단경로와 요금 계산 추가~~ [2단계 - 경로 조회 기능] 채채(신채원) 미션 제출합니다. May 22, 2023

test: sectionDaoTest에 구간 모두 조회 테스트 추가

e6b6fa2

Gomding requested changes May 22, 2023

View reviewed changes

chaewon121 added 6 commits June 1, 2023 22:45

refactor: 최단거리 기능 구현 인터페이스로 분리

a551026

refactor: SectionStationResultMap 제거

d08a511

feat: mysql 연동 구현

19c8235

fix: mysql 연동 구현 수정

5e42332

refactor: 트렌젝셔널 추가

04470c0

refactor: url 수정

00c712e

Gomding approved these changes Jun 7, 2023

View reviewed changes

Gomding merged commit c4c399d into woowacourse:chaewon121 Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 경로 조회 기능] 채채(신채원) 미션 제출합니다. #203

[2단계 - 경로 조회 기능] 채채(신채원) 미션 제출합니다. #203

chaewon121 commented May 22, 2023

Gomding left a comment

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding Jun 7, 2023 •

edited

Loading

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding May 22, 2023

chaewon121 Jun 6, 2023

Gomding left a comment

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

Gomding Jun 7, 2023

		WeightedMultigraph<String, DefaultWeightedEdge> graph
		= new WeightedMultigraph(DefaultWeightedEdge.class);

		private Section findById(Long id) {
		final String sql = "SELECT se.id AS section_id, se.distance, se.line_id, " +



		@RestController
		@RequestMapping("/shortestPath")

[2단계 - 경로 조회 기능] 채채(신채원) 미션 제출합니다. #203

[2단계 - 경로 조회 기능] 채채(신채원) 미션 제출합니다. #203

Conversation

chaewon121 commented May 22, 2023

Gomding left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gomding Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gomding left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gomding Jun 7, 2023 •

edited

Loading