[3단계 - 요금 정책 추가] 우르(김현우) 미션 제출합니다 #181

java-saeng · 2023-05-22T04:02:01Z

안녕하세요 영이 !!

이번에 3단계에서는 요금 부과 정책과 요금 할인 정책 인터페이스를 따로 나누고,

요금 정책에 관한 xxxComposite 클래스 하나를 만들어서 모든 요금 정책에 관한 빈들을 주입받고, 요금을 계산해주었습니다.
(ArgumentResolver 방법을 차용했습니다,,)

다만 여기서 요금 부과 정책과 요금 할인 정책을 하나의 인터페이스로 두고 싶었는데, 요청 값들이 서로 달라 하나의 인터페이스로 묶기가 조금 애매했습니다.

다른 방법이 있는지 궁금합니다!!

테스트는 Service는 Integration, Controller는 WebMvcTest를 이용한 단위 테스트를 했습니다.

이렇게 한 이유는 저는 Controller에서 검증하고 싶은 것은 Service에 관한 메서드가 제대로 호출되었는지, 상태코드가 제대로 오는지 확인하기 때문에 굳이 통합 테스트를 할 필요가 없다고 생각했어요.

그리고 비즈니스 로직이 포함된 Service에서는 DB와 연결이 잘 되는지, 비즈니스 로직이 잘 수행되는지 확인하기 위해 통합 테스트를 진행했습니다.

docker-compose.yml 해두었습니다 !

상황 A -> B -> C 에서 A -> K 를 추가할 경우 exception 발생

SubwayPricePolicy - 파라미터 Route 로 수정 Route - 파라미터로 조회할 경로의 출발, 도착역 추가

- 삭제 시 No Content 로 수정

java-saeng · 2023-05-22T10:34:11Z

src/main/java/subway/domain/route/Route.java

+            final Station from,
+            final Station to
+    ) {
+        this.routeFinder = new JgraphtRouteFinder(lines);


Route 도메인에서 JgraphtRouteFinder 라는 구체적인 외부 라이브러리에 의존하는게 좋지 않은 설계라고 생각합니다.

그래서 이를 외부 에서 주입받는 형식으로 사용하고 싶은데, 그렇게 하기 위해서는 누군가가 JgraphtRouteFinder 에 lines를 주입해줘야합니다.

하지만 lines는 빈이 아니어서 스프링에서 주입을 해주지 않기 때문에 수동으로 설정해줘야합니다.

이를 어딘가에서는 넣어줘야하는데,, lines를 파라미터로 받아서 쉽지가 않습니다,,

애초에 Route라는 도메인에서 RouteFinder를 가지고 있는게 문제인가 라는 생각이 듭니다.

혹시 어떤 해결 방법이 있는지 여쭤볼 수 있을까요?

도메인이 외부 라이브러리에 의존하지 않으면서 유연한 설계를 하고 싶습니다.

route 객체에 RouteFinder를 필드로 가지고 있을 필요는 없을것 같네요
util 성 클래스일것 같은데 RouteFinder는 생성자로 생성하지 않고
static 메서드로 처리될수있도록 하는게 좀 더 의도를 명확하게 보여줄 수 있지 않을까 라는 생각입니다!

맞습니다,, DM에서 말씀주신 것처럼 자동차 미션을 예로 들면 랜덤 숫자를 만들어주는 인터페이스가 필드를 가진 것과 비슷한 느낌이 들긴합니다,, ㅋㅋㅋㅋ 바로 삭제하고 static 메서드로 한번 만들어보았습니다 !!

choijy1705

안녕하세요 우르!
3단계 까지 빠르게 진행해주셨네요!👍
3단계는 많이 복잡해져서 파악하는데 시간이 좀 걸리네요
route api를 한번 호출하는데 똑같은 route를 세번이나 만들어주고 있는것 같은데
한번만 생성하여 처리할 순 없을지 고민해보면 좋을것 같아요!

choijy1705 · 2023-05-22T15:44:46Z

src/test/java/subway/yml/ProfileActiveFirstTest.java

+@SpringBootTest
+@ActiveProfiles({"test", "first"})
+public class ProfileActiveFirstTest {
+
+    @Autowired
+    private DataSource dataSource;
+
+    @Test
+    void test_first() throws Exception {
+        String expectedDatabaseUrl = "jdbc:h2:mem:first";
+        String actualDatabaseUrl = dataSource.getConnection().getMetaData().getURL();
+
+        assertEquals(expectedDatabaseUrl, actualDatabaseUrl);
+    }
+}


profile 학습테스트 인가요?👍

네 그렇습니다 !!! 보여드리는게 나을 것 같아서요 :)

choijy1705 · 2023-05-22T15:46:01Z

src/test/java/subway/yml/ProfileActiveSecondTest.java

+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@SpringBootTest
+@ActiveProfiles({"second", "test"})


build.gradle 에 spring.profiles.active 를 설정하여
test에 매번 ActiveProfiles를 지정하지 않아도 test.yml 파일을 보도록 할 수 있을것 같아요
한번 찾아보시면 좋을것 같네요!

test { useJUnitPlatform() systemProperty 'spring.profiles.active', 'test' }

이런 식으로 설정해주었는데도 잘 안 되네요,,!!

제가 알고 있기로는 profiles.active 는 이제 application-{profile}.yml 파일을 읽는 것이고,
yml 파일 안에서 ---를 사용하게 되면 yml 파일에 spring.config.activate.on-profile이라는 다른 옵션을 주어서 아마 gradle에서는 spring.profiles.active 실행하면 안 먹히는 것 같습니다 !!

제가 잘못 알고 있는걸까요!?

choijy1705 · 2023-05-22T15:48:04Z

src/main/java/subway/controller/LineController.java

                    .collect(Collectors.toList());
    }

+    @GetMapping("/lines/{line-id}")


Suggested change

@GetMapping("/lines/{line-id}")

@GetMapping("/lines/{lineId}")

보통은 카멜케이스를 쓰는것 같아요

https://restfulapi.net/resource-naming/

저는 해당 문서가 restful 공식문서인지 아닌지는 모르겠지만, best-practice를 보면 {id} 또는 {resource-id} 로 사용하고 있어서 이게 표준인 줄 알았습니다,,

물론 표준을 무조건 지켜야하는건 아니지만, 공식문서에서는 - 를 사용하던데, 이 부분은 같은 팀간의 컨벤션으로 결정하는 걸까요?

아하 이런것도 공식문서가 있네요
몰랐어요!
공식문서를 따르는게 좋을것 같아요

choijy1705 · 2023-05-22T15:59:42Z

src/main/java/subway/service/RouteQueryService.java

-    public int searchLeastCost(final ShortestRouteRequest shortestRouteRequest) {
-        return subwayPricePolicy.calculate(searchShortestDistance(shortestRouteRequest));
+    public double searchLeastCost(final ShortestRouteRequest shortestRouteRequest) {
+        final List<Line> lines = lineQueryService.searchAllLine();


하나의 api 요청에서 lines를 세번 조회할 필요는 없을것 같아요!😭
searchShortestRoute, searchLeastCost, searchShortestDistance 에서 세번 조회하고 route도 똑같을것 같은데 세번 생성하고 있네요

최단거리와 최소요금이 같은 route로 계산되어지는데
현실에서는 최단거리가 최소요금이 아닌 경우도 있을수 있을것 같아요!

제가 요구사항 분석을 미흡하게 한 것 같습니다.

해당 메서드는 최단 거리에 따른 요금인데 왜 searchLeastCost라는 메서드를 붙였을까요,,,

2단계 : 경로 조회 시 요금 같이 반환
3단계 : 요금 정책 변경

어디에도 최소 요금이라는 요구 사항이 없네요,, 메서드명 바로 수정하도록 하겠습니다 !

하나의 api 요청에서 lines를 세번 조회할 필요는 없을것 같아요!😭
searchShortestRoute, searchLeastCost, searchShortestDistance 에서 세번 조회하고 route도 똑같을것 같은데 세번 생성하고 있네요

이 부분은 처음에 최대한 컨트롤러 응답 DTO의 변환을 controller에서 해주고 싶었습니다.
그래서 service에서 모든 요청을 받을 때마다 Route를 생성했었어요,,

아니면 차선책으로 처음에 Service로부터 Route를 만들고, route를 파라미터로 전달하면서 해도 되겠지만 조금 어색한 느낌이 들었어요.

그래서 변환하는 위치는 상황마다 다르기 때문에 해당 API 의 변환은 service에서 하는게 적절하다고 생각이 들어서 Service에서 나머지 요금, 거리 조회는 private 메서드로 수정했습니다.

src/main/java/subway/domain/policy/ChargePolicyComposite.java

choijy1705 · 2023-05-23T08:09:01Z

src/main/java/subway/domain/route/JgraphtRouteFinder.java

+    @Override
+    public List<String> findShortestRoute(final String startStation, final String endStation) {
+        return dijkstraGraph.getPath(startStation, endStation).getVertexList();
+    }


station이 db에 따로 저장되고 있지 않아서 따로 체크할 방법은 없을것 같고
없는 역이름을 입력한 경우 dijkstraGraph 에서 raw한 에러를 내려줄것 같은데
예외 처리가 필요하지 않을까요?

choijy1705 · 2023-05-23T08:15:54Z

src/main/java/subway/domain/route/JgraphtRouteFinder.java

+    private final DijkstraShortestPath<String, EdgeSection> dijkstraGraph;
+
+    public JgraphtRouteFinder(final List<Line> lines) {
+        Graph<String, EdgeSection> graph


Station에 equalsAndHashCode 구현하고
vertex 타입을 Station 사용해도 되지 않나요?

그 방법을 생각 못했네요 !! 2호선 잠실이나 8호선 잠실이나 같은 잠실이어서 충분히 equals hashCode 구현해도 될 것 같습니다 !

choijy1705 · 2023-05-23T08:25:52Z

src/main/java/subway/domain/policy/discount/DiscountCondition.java

@@ -0,0 +1,14 @@
+package subway.domain.policy.discount;
+
+public class DiscountCondition {


딱히 기능도 없고
복잡성만 늘리는 원시값 포장이지 않을까요?😭

나중에 할인 조건이 생기지 않을까 하는 생각에 조건을 가지는 객체를 생성해두었습니다,,

현재는 나이 밖에 없어서 파라미터에 age가 들어가지만, 나중에 다른 조건들이 필요할 때 age 를 파라미터로 사용한 관련 코드를 다 바꿔야 할 것 같아서 미리 만들어두었습니다.

조금 과한 경향이 있는걸까요?

개인적으로 필요한 시점에 추가하는게 가장 좋은것 같아요!

choijy1705 · 2023-05-23T08:27:35Z

src/main/java/subway/domain/policy/discount/AgeGroup.java

+
+    public static AgeGroup findAgeGroup(final Integer target) {
+        return Arrays.stream(values())
+                     .filter(age -> Objects.nonNull(target))


Integer 대신 int를 사용하면 굳이 null 체크 안해도 되겠네요!

이 부분은 일부러 null을 사용했습니다 !!

요금 계산할 때 나이에 따른 요금 할인 정책이 존재하는데, 나이를 입력하지 않는 경우에는 아무 할인도 적용되지 않게 처리하기 위해서 null 을 사용했습니다 !

만약 nullable하지 않게 하려면 ShortestRouteRequest 여기에서 age를 int로 받아와서 0으로 해야하는데 이렇게 처리하는게 나을까요???

의도한거라면 상관없을것 같네요!

choijy1705 · 2023-05-23T08:29:40Z

src/main/java/subway/domain/policy/fare/LineFarePolicy.java

+    private static final Map<String, Integer> priceMap
+            = Map.of("1호선", 500,
+                     "2호선", 1000);


line의 요금은 db에 저장되는게 좋지 않을까요?
line을 생성할때 지정해주고
요금 계산할때 db의 정보를 이용하여 계산하는 흐름이 자연스러울것 같아서요!

저도 이 부분을 구현할 때 많이 생각해봤었습니다,,,

DB에 값을 저장 vs 현재처럼 애플리케이션에 상수처럼 관리하기

많은 고민 끝에 애플리케이션에 상수처럼 관리하게 된 이유는 호선에 관한 요금 정책은 무조건 해당 클래스에서만 이뤄지기 때문에, 즉, 호선에 따른 요금 부과 책임을 가지는 곳이 해당 클래스밖에 없기 때문에 여기에서 상수로 관리해도 좋겠다 라는 생각이 들었습니다.

영이가 말씀해주신 부분이 자연스럽긴하지만, 상수로도 충분히 관리가 될 것 같아서 구현했습니다 !!
남이 보았을 때도 클래스 이름으로 충분히 파악이 가능하다고 생각이 들어서 해당 클래스만 보면 어떻게 계산이 되는지 파악하는데 어려움이 없을 것이라 생각이 들었습니다 !!

line을 조회했을때도 추가 요금에 관한 부분이 나오면 좋지않을까요?

- 최소 요금이 아닌 최단거리에 대한 요금을 구하는 메서드임

- 굳이 세 개의 메서드에서 매번 Route를 만들면서 할 필요가 없어서 하나로 합침

- JgraphtRouteFinder를 빈으로 등록

…e 테스트에 JgraphtRouteFinder 를 Mock으로 등록

- 경로, 거리, 가격을 계산하는 유틸리티 클래스 생성 - RouteFinder 인터페이스 삭제 - 요금 관련 정책 파라미터 변경

java-saeng · 2023-05-25T16:09:29Z

안녕하세요 영이 !!

리뷰를 다 반영했는데도 아직 헷갈리는 부분들 + 어려움이 있는 부분이 있어서요,,

#181 (comment)

해당 리뷰를 반영하다가 어려움이 있었어요,,

DM에서 얘기를 나눴다시피 어떤 라이브러리를 쓰는지 예상도 되기 때문에 핸들링해야한다고 생각합니다,,!!
그런데 아래의 사진을 보시면 jgraph자체에서 exception 을 던져주는데 이게 같은 Type의 Exception이라 sink 와 source를 구분하는게 제어할 수 없더라구요,,

그래서 생각을 해봤는데도 도저히 떠오르지 않아서,, 도움 요청 드립니다,,ㅠㅠㅠ 그대로 메시지 내려주기에는 조금 그렇고,, 그렇다고 핸들링하기도 어렵고,, 어떻게 처리하는게 좋을까요?

두번째로, 현재 외부 라이브러리를 사용하고 있기 때문에 이 외부 라이브러리와의 의존을 최대한 약하게 가져가고 싶었어요. 또한 외부 라이브러리가 바뀔 경우에는 그 변경 바운더리가 최대한 좁았으면 좋겠구요.
+성능이라고 말하기에는 매우 작은 부분이지만 최대한 객체를 생성하는 중복 코드를 줄여보고 싶었어요.

그래서 제가 내린 결론은 DijkstraGraphFactory 에서 jgraph 와 강결합을 통해 graph 를 만들고, JgraphtRouteFinder 유틸리티 클래스에서 Factory 를 통해 그래프를 생성하여 거리, 계산, 최단 경로를 찾습니다.

이 부분은 어느 위치인지 제가 따로 코멘트 달아놓겠습니다,, !! 답변해주시면 감사하겠습니다 !!

src/main/java/subway/domain/policy/ChargePolicyComposite.java

java-saeng · 2023-05-23T14:41:58Z

src/test/java/subway/yml/ProfileActiveFirstTest.java

+@SpringBootTest
+@ActiveProfiles({"test", "first"})
+public class ProfileActiveFirstTest {
+
+    @Autowired
+    private DataSource dataSource;
+
+    @Test
+    void test_first() throws Exception {
+        String expectedDatabaseUrl = "jdbc:h2:mem:first";
+        String actualDatabaseUrl = dataSource.getConnection().getMetaData().getURL();
+
+        assertEquals(expectedDatabaseUrl, actualDatabaseUrl);
+    }
+}


네 그렇습니다 !!! 보여드리는게 나을 것 같아서요 :)

java-saeng · 2023-05-23T14:47:35Z

src/main/java/subway/controller/LineController.java

                    .collect(Collectors.toList());
    }

+    @GetMapping("/lines/{line-id}")


https://restfulapi.net/resource-naming/

저는 해당 문서가 restful 공식문서인지 아닌지는 모르겠지만, best-practice를 보면 {id} 또는 {resource-id} 로 사용하고 있어서 이게 표준인 줄 알았습니다,,

물론 표준을 무조건 지켜야하는건 아니지만, 공식문서에서는 - 를 사용하던데, 이 부분은 같은 팀간의 컨벤션으로 결정하는 걸까요?

java-saeng · 2023-05-23T14:51:05Z

src/main/java/subway/domain/policy/ChargePolicyComposite.java

+
+@Primary
+@Component
+public class ChargePolicyComposite implements SubwayFarePolicy, SubwayDiscountPolicy {


제가 원하던 설계 방식은 요금 방식이라는 객체에서 모든 요금들(요금 할인, 요금 부과)을 계산하고 나서의 값을 반환해주고 싶었습니다.

그런데 요금 부과와 요금 할인은 서로 필요한 조건들이 다르기 때문에 같이 추상화시키는데 어려움이 있었습니다.

그래서 제가 내린 결론은 요금 방식(ChargePolicyComposite)이 요금 할인(SubwayDiscountPolicy), 요금 부과(SubwayFarePolicy)를 구현해서 요금 방식에서 calculate와 discount를 통해 모든 요금을 계산하고나서의 결과값을 반환해주도록 했습니다.

많이 어색한걸까요,,?

java-saeng · 2023-05-23T14:57:03Z

src/main/java/subway/domain/policy/discount/AgeGroup.java

+
+    public static AgeGroup findAgeGroup(final Integer target) {
+        return Arrays.stream(values())
+                     .filter(age -> Objects.nonNull(target))


이 부분은 일부러 null을 사용했습니다 !!

요금 계산할 때 나이에 따른 요금 할인 정책이 존재하는데, 나이를 입력하지 않는 경우에는 아무 할인도 적용되지 않게 처리하기 위해서 null 을 사용했습니다 !

만약 nullable하지 않게 하려면 ShortestRouteRequest 여기에서 age를 int로 받아와서 0으로 해야하는데 이렇게 처리하는게 나을까요???

java-saeng · 2023-05-25T15:10:21Z

src/main/java/subway/domain/route/RouteFinder.java

+public interface RouteFinder {
+
+    List<String> findShortestRoute(final String startStation, final String endStation);
+
+    Distance findShortestRouteDistance(final String startStation, final String endStation);
+
+    List<EdgeSection> findShortestRouteSections(final String startStation, final String endStation);
+}


DM 에서 말씀해주신대로 생각이 들어서 인터페이스 삭제 했습니다 !!

java-saeng · 2023-05-25T15:11:00Z

src/main/java/subway/domain/route/JgraphtRouteFinder.java

+
+import java.util.List;
+
+public class JgraphtRouteFinder implements RouteFinder {


수정했습니다 !!

java-saeng · 2023-05-25T15:11:42Z

src/main/java/subway/domain/route/JgraphtRouteFinder.java

+    private final DijkstraShortestPath<String, EdgeSection> dijkstraGraph;
+
+    public JgraphtRouteFinder(final List<Line> lines) {
+        Graph<String, EdgeSection> graph


그 방법을 생각 못했네요 !! 2호선 잠실이나 8호선 잠실이나 같은 잠실이어서 충분히 equals hashCode 구현해도 될 것 같습니다 !

java-saeng · 2023-05-25T15:16:12Z

src/main/java/subway/domain/policy/fare/LineFarePolicy.java

+    private static final Map<String, Integer> priceMap
+            = Map.of("1호선", 500,
+                     "2호선", 1000);


저도 이 부분을 구현할 때 많이 생각해봤었습니다,,,

DB에 값을 저장 vs 현재처럼 애플리케이션에 상수처럼 관리하기

많은 고민 끝에 애플리케이션에 상수처럼 관리하게 된 이유는 호선에 관한 요금 정책은 무조건 해당 클래스에서만 이뤄지기 때문에, 즉, 호선에 따른 요금 부과 책임을 가지는 곳이 해당 클래스밖에 없기 때문에 여기에서 상수로 관리해도 좋겠다 라는 생각이 들었습니다.

영이가 말씀해주신 부분이 자연스럽긴하지만, 상수로도 충분히 관리가 될 것 같아서 구현했습니다 !!
남이 보았을 때도 클래스 이름으로 충분히 파악이 가능하다고 생각이 들어서 해당 클래스만 보면 어떻게 계산이 되는지 파악하는데 어려움이 없을 것이라 생각이 들었습니다 !!

java-saeng · 2023-05-25T16:12:18Z

src/main/java/subway/route/application/JgraphtRouteFinder.java

+      final Station departure,
+      final Station arrival
+  ) {
+    return DijkstraGraphFactory.makeDijkstraGraph(lines).getPath(departure, arrival)


이 부분에서 3개의 static method를 각각 만들 때마다 그래프를 만들어주고 있습니다.

이 부분이 조금 마음에 들지 않아서 #findShortestRoute에서 최단 경로, 가격, 섹션까지 조회하는 객체를 반환해줄까 라는 생각을 했는데 DTO 느낌이 들어서 차라리 이렇게 따로 만들자 라는 생각을 했습니다.

혹시 영이라면 어떻게 하셨을지 고견을 좀 들어볼 수 있을까요?

여기서 이제 "파라미터로 graph 를 받지 않는게 좋겠다"라는 생각을 했었는데, 그 이유는 해당 클래스는 RouteQueryService에서 호출하는데, graph 를 받게 된다면 RouteQueryService또한 jgraph와 강결합이 되기 때문에 받지 않는게 낫겠다 라는 생각을 했습니다.

searchShortestRoute 메서드에서 DijkstraGraphFactory.makeDijkstraGraph 를 호출하는게 강결합인가요?
우르가 생각하는 강결합의 정의가 무엇이고 거기에서 발생하는 문제점이 어떤건지 궁금합니다!

choijy1705

안녕하세요 우르!
3단계까지 진행하신다고 고생하셨습니다!
이번 미션은 머지하겠습니다
다음 미션도 화이팅입니다!

choijy1705 · 2023-05-26T15:47:13Z

src/main/java/subway/policy/application/ChargePolicyComposite.java

+public class ChargePolicyComposite implements SubwayFarePolicy, SubwayDiscountPolicy {
+
+  private final List<SubwayFarePolicy> farePolicies;
+  private final List<SubwayDiscountPolicy> discountPolicies;
+
+  public ChargePolicyComposite(
+      final List<SubwayFarePolicy> farePolicies,
+      final List<SubwayDiscountPolicy> discountPolicies
+  ) {
+    this.farePolicies = farePolicies;
+    this.discountPolicies = discountPolicies;
+  }


이 패턴은 따로 참고하신게 있나요??😭
너무 처음보는 패턴이고 처음보는 사람이 이해하기에는 어려운것 같아요
이펙티브 자바 item 18 상속보다 조합을써라는 말이 있는데
지금 구조는 상속도 사용하고 조합도 사용한 구조인것 같아요
DistanceFarePolicy, LineFarePolicy, AgeDiscountPolicy를 필드로 가지고 있고
이를 조합하여 계산하는 흐름이 더 자연스러워 보이네요

choijy1705 · 2023-05-26T15:50:33Z

src/main/java/subway/policy/infrastructure/AgeDiscountPolicy.java

+  private static class DiscountValue {
+
+    private final int discountPrice;
+    private final int percent;
+
+    public DiscountValue(final int discountPrice, final int percent) {
+      this.discountPrice = discountPrice;
+      this.percent = percent;
+    }
+
+    public int getDiscountPrice() {
+      return discountPrice;
+    }
+
+    public int getPercent() {
+      return percent;
+    }
+  }


ageGroup enum 에서 가지고 있어도 되지 않을까요?

choijy1705 · 2023-05-26T15:54:18Z

src/main/java/subway/route/application/JgraphtRouteFinder.java

+      final Station arrival
+  ) {
+    return new Distance(
+        (int) DijkstraGraphFactory.makeDijkstraGraph(lines).getPathWeight(departure, arrival));


반환값을 Int로 다시 바꾸기보다 distance의 value 타입을 double로 바꾸는게 좋지 않을까요?

choijy1705 · 2023-05-26T16:01:55Z

src/main/java/subway/route/application/JgraphtRouteFinder.java

+      final Station departure,
+      final Station arrival
+  ) {
+    return DijkstraGraphFactory.makeDijkstraGraph(lines).getPath(departure, arrival)


searchShortestRoute 메서드에서 DijkstraGraphFactory.makeDijkstraGraph 를 호출하는게 강결합인가요?
우르가 생각하는 강결합의 정의가 무엇이고 거기에서 발생하는 문제점이 어떤건지 궁금합니다!

choijy1705 · 2023-05-26T16:07:31Z

src/main/java/subway/route/application/RouteQueryService.java

+    return new ShortestRouteResponse(
+        mapToStationNameFrom(JgraphtRouteFinder.findShortestRoute(lines, departure, arrival)),
+        searchCost(lines, departure, arrival, shortestRouteRequest.getAge()).getValue(),
+        JgraphtRouteFinder.findShortestRouteDistance(lines, departure, arrival).getValue()
+    );


한번에 처리하기보다는
끊어서 처리하면 더 가독성이 좋을것 같네요
findShortestRoute, searchCost, findShortestRouteDistance가 핵심 로직인데
dto로 바꾸는 로직이 더 중요해보입니다!

java-saeng added 27 commits May 19, 2023 13:36

fix : 제일 앞의 Section에 역을 추가할 경우 예외 수정

62a03e6

상황 A -> B -> C 에서 A -> K 를 추가할 경우 exception 발생

refactor : 도메인이 외부 라이브러리에 바로 의존하지 않고 인터페이스에 의존하면서 최단 경로 조회

752ed20

feat : 하나의 yml 안에 여러 profile 설정해보기

6e9cf73

feat : 최소 경로 구할 때, Section 반환하는 기능 추가

f9eebfc

feat : 호선에 따라서 요금 정책 기능 추가

8eb7638

SubwayPricePolicy - 파라미터 Route 로 수정 Route - 파라미터로 조회할 경로의 출발, 도착역 추가

refactor : SubwayPricePolicy -> SubwayChargePolicy 로 변경

8856369

feat : 연령별 요금 할인 정책 기능 구현

e8ec926

refactor : 돈 관련 메서드 모두 Money로 변경

dac9a3b

refactor : 요금 할인, 요금 부과 계산을 ChargePolicyComposite 에서 모두 하기

402a425

refactor : 패키지 수정

c2f7753

refactor : 예외 메시지 전달

6eddf3d

refactor : 불필요한 메서드 삭제

a42f94a

feat : line id 로 조회하는 API 생성

aa0e619

refactor : line 조회 시 id 반환

781c569

refactor : 도메인 패키지 변경에 따른 테스트 코드 패키지 변경

71c6785

refactor : 도메인 패키지 변경에 따른 테스트 코드 패키지 변경

4bcca05

test : LineDao 테스트 작성

13d1d2d

test : SectionDao 테스트 작성

19f4307

test : LineControllerTest 작성

074f896

test : RouteControllerTest 작성

35ea991

test : StationController 작성

ff4297e

- 삭제 시 No Content 로 수정

test : LineCommandService Integration Test 추가

9c12f91

test : LineQueryService Integration Test 추가

e90d594

test : RouteQueryService Integration Test 추가

028a82a

test : SectionCommandService Integration Test 추가

3927e12

test : SectionQueryService Integration Test 추가

7c02029

test : StationService Integration Test 추가

868b698

java-saeng changed the title ~~[3단계 - 요금 정책 추가] 우르(김현우) 미션 제출합니다!~~ [3단계 - 요금 정책 추가] 우르(김현우) 미션 제출합니다 May 22, 2023

refactor : exception 패키지 이동

f385149

java-saeng commented May 22, 2023

View reviewed changes

choijy1705 requested changes May 23, 2023

View reviewed changes

java-saeng added 13 commits May 23, 2023 23:40

refactor : java style google 로 변경

5f0a505

refactor : 메서드 명 변경

89a3d4a

- 최소 요금이 아닌 최단거리에 대한 요금을 구하는 메서드임

refactor : 경로 조회 시 하나의 service 에서 조회하도록 변경

7a95acc

- 굳이 세 개의 메서드에서 매번 Route를 만들면서 할 필요가 없어서 하나로 합침

refactor : DefaultFarePolicy -> DistanceFarePolicy 로 이름 변경

beded7a

refactor : Route 도메인 삭제

d320f85

- JgraphtRouteFinder를 빈으로 등록

feat : 처음 애플리케이션이 실행될 때와 역에 변경이 일어날 경우 다익스트라 그래프 수정하는 이벤트 추가

c9ee774

test : yml 테스트 시 애플리케이션 컨테이네가 띄워지면 자동으로 이벤트가 수행되기 때문에 이를 방지하고자 Profil…

62caf14

…e 테스트에 JgraphtRouteFinder 를 Mock으로 등록

refactor : Policy 관련 빈 해제하고, Composite 만 빈 등록

d3d66cb

refactor : Line 생성 시 이벤트 발행

10b95bb

refactor : 최소 경로 구할 때, key 값을 역 이름(String)이 아닌 Station으로 사용

966df84

refactor : Event 삭제

c6984bb

refactor : 그래프 생성하는 Factory 클래스 생성

1390aaf

- 경로, 거리, 가격을 계산하는 유틸리티 클래스 생성 - RouteFinder 인터페이스 삭제 - 요금 관련 정책 파라미터 변경

refactor : 패키지 구조 변경

727c10b

java-saeng commented May 25, 2023

View reviewed changes

java-saeng requested a review from choijy1705 May 25, 2023 16:17

choijy1705 approved these changes May 26, 2023

View reviewed changes

choijy1705 merged commit a6a307f into woowacourse:java-saeng May 26, 2023

	@GetMapping("/lines/{line-id}")
	@GetMapping("/lines/{lineId}")

		@@ -0,0 +1,14 @@
		package subway.domain.policy.discount;

		public class DiscountCondition {


		import java.util.List;

		public class JgraphtRouteFinder implements RouteFinder {

[3단계 - 요금 정책 추가] 우르(김현우) 미션 제출합니다 #181

[3단계 - 요금 정책 추가] 우르(김현우) 미션 제출합니다 #181

Conversation

java-saeng commented May 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choijy1705 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

java-saeng commented May 25, 2023

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

choijy1705 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

java-saeng commented May 22, 2023 •

edited

Loading