[2단계 - 경로 조회 기능] 허브(방대의) 미션 제출합니다. #126

greeng00se · 2023-05-17T16:00:51Z

안녕하세요 서브웨이~ 잘 지내셨나요? 😄

1단계에서 좋은 리뷰를 많이 남겨주셨는데, 코멘트에 대한 추가 답변을 여기에다 이어서 남깁니다!

도메인 식별자(id)에 대한 equals & hashcode

Repository에서 변경된 부분만 제거, 추가하는 로직을 작성하기 위해 HashSet을 사용했습니다.
서브웨이의 코멘트를 반영하여 지우려 했지만 사용하지 않으면 코드가 가독성이 너무 떨어지는 것 같아서 equals & hashcode를 추가하여 사용하였습니다. 😢

Repository 추가, 삭제 => 수정, 추가에 대한 부분

Update에 대한 부분을 생각해보았는데, 기존에 있던 구간 사이에 새로운 역이 추가되는 경우 Update를 할 수 있을 것 같습니다.
ex) A - C가 있는 경우 A - B - C가 될 때 기존의 A - C 구간을 A - B로 업데이트하고 B - C를 추가한다.
이 경우 A - C 구간을 제거하고 A - B, B - C 구간을 새로 생성하는게 조금 더 명확하다고 느껴졌는데요!
서브웨이는 어떻게 생각하시나요?

해당 리뷰 요청에는 다음과 같은 부분이 추가로 반영되었습니다.

경로 조회

jgrapht를 사용할 때 Station의 동등성 비교를 하여 사용하고 싶었지만
현재 역이 2호선 교대역, 3호선 교대역이 서로 다른 역이라 역 이름을 정점으로 추가하여 사용했습니다.

요금 정책과 할인 정책

현재 요금 정책과 할인 정책의 구분 없이 같이 사용하였습니다.
그러다 보니 다음과 같이 정책에 대한 순서를 직접 지정해줬어야 했는데요.
현재 미션 요구사항에는 구분 없이 같이 사용하는게 더욱 명확하고 실용적이라고 생각되는데 서브웨이는 어떻게 생각하시나요?

new SubwayFarePolicy(List.of(
        new BaseFarePolicy(),
        new DistanceFarePolicy(),
        new AgeDiscountFarePolicy()
));

2단계도 잘 부탁드립니다!

joseph415

안녕하세요 허브!

2,3단계 미션 잘 구현해주셨네요ㅎㅎ💯
코멘트와 답변 함께 달아두었습니다! 확인 부탁드릴게요~! 1단계와 마찬가지로 확인이 완료된 리뷰는 resolve conversation 눌러주시면 감사하겠습니다!

README.md

joseph415 · 2023-05-20T15:03:36Z

README.md

@@ -19,6 +19,13 @@
    - [x] 노선에 포함된 역을 순서대로 보여줘야 한다.


3단계 미션의 주제는 다음과 같은데요.

이번 미션은 이전에 구현한 요금 계산 기능에 새로운 정책을 반영하는 미션입니다.
요구사항을 구현하면서 변경에 유연한 설계에 대해 다시 고민하는 시간을 가집니다.

허브는 이번에 2,3단계를 같이 하셔서 얼마나 큰 변경사항이 있어나 확인하기 쉽지 않은데, 커밋을 보니 2+3단계를 한번에 적용하신것 같네요ㅎㅎ

혹시 원래 기능에 새기능을 추가해주실 때 이런 걸 경험하셨는지 궁금합니다~

이 부분에 대해서 2, 3단계를 한 번에 진행했기 때문에 기존 요구사항 이외에 다른 요구사항이 추가로 들어온다면 어떻게 해야할까? 에 대해 크루들과 이야기를 많이 나눠보았는데요!
이 때 유연함과 명료함 사이에서 고민을 많이했습니다!
너무 유연함을 추구하다보면 추상적인 개념이 많이 추가되어 결국 코드의 복잡도를 증가시킬 것이라고 생각했고,
단순함을 추구하다보면 추후에 복잡한 요구사항이 추가될 때 대응하기 어려울 것이라고 생각했습니다.

현재의 애플리케이션의 경우 복잡한 요구사항이 많이 없었고, 이 경우 오히려 단순하게 만들어서 추가적인 요구사항을 대비하는 것이 좋아보였습니다.

그래도 추가적인 요구사항이 생겼을 때 아래의 클래스를 이용하여 반영할 수 있을 것 같습니다.

SectionEdge -> 지하철 도메인에 관련한 요구사항

Passenger -> 탑승객에 관련된 요구사항

저 두가지 클래스로 감당할 수 없는 요구사항이 있다면, 그 때 구조를 바꾸는 것이 오히려 효율적이라고 생각했습니다.

joseph415 · 2023-05-20T15:37:49Z

src/main/java/subway/config/FareConfiguration.java

+public class FareConfiguration {
+
+    @Bean
+    public FarePolicy farePolicy() {


도메인 패키지내에 spring Component를 넣지 않기 위해서 Bean으로 추가해주신걸까요~?

넵 맞습니다!
추가로 현재 추가운임 + 할인정책을 하나로 묶어서 컴포지트 패턴을 적용할 때 리프 부분을 컴포넌트로 등록해서 사용할까 고민을 했었는데
Configuration에서 직접 new를 이용해서 요구사항의 순서대로 추가하는게 조금 더 명료하다고 생각해서 위와 같이 설정했습니다!

src/main/java/subway/controller/PathController.java

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

joseph415 · 2023-05-20T17:20:15Z

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

+    }
+
+    public PathFindResult find(final String start, final String end) {
+        final DijkstraShortestPath<String, SectionEdge> shortestPath = initializeShortestPath();


이거는 한가지 의견을 드려보고 싶은데요.

도메인에 특정 기술의 의존성을 분리

원래 도메인은 특정 기술과 직접적인 의존을 최대한 피하는 것이 좋은데요. 지금 이렇게 되면, jgrapht라는 곳과 강결합이 되어버리기 때문에, jgrapht가 변하게 된다면, PathFinder 를 사용하고 있는 모든곳이 영향을 받을 수 있습니다. 그리고 만약, jgrapht가 아닌 다른 라이브러리 사용하게 됐을 때도, 변경에 영향을 받겠죠.

그렇기에 따라서, PathFinder라는 도메인 명세와 세부 구현을 나눠보는건 어떤가 의견드립니다. 구현부는 도메인패키지 외부로 빼고요.
그러면 SectionEdge도 같이 외부로 빠지겠네요ㅎㅎ 구현부에서만 사용할 객체가 되니까요!..

고민을 많이했던 부분인데 코멘트 남겨주셔서 너무 행복하네요 😄
처음에 작성한 로직처럼 jgrapht와 도메인 로직이 너무 강결합되는게 아닌가 생각을 했지만 일단 기능 구현을 해볼까? 라는 생각이 들면서 외부 라이브러리의 변경에 대한 고민이 부족했던 것 같습니다!

도메인에는 인터페이스를 두고 jgrapht 패키지를 따로 두어서 구현부를 분리했습니다. 👍 👍

src/main/java/subway/domain/fare/FarePolicy.java

src/main/java/subway/domain/fare/AgeDiscountFarePolicy.java

src/main/java/subway/dto/StationSaveRequest.java

src/test/resources/application.yml

joseph415 · 2023-05-20T17:55:36Z

Repository 추가, 삭제 => 수정, 추가에 대한 부분

넵ㅎㅎ 알겠습니다! 사실 현업에서는 백업디비를 같이 사용할거라, 괜찮을거에요! 근데 보통 리소스를 지워버리는것은 말씀드린것처럼 조심해야한다고 생각합니다! 그걸 파악하셨다면 지금 로직대로 해도 좋다고생각합니다!👍

…지고 있도록 변경

greeng00se · 2023-05-21T09:21:28Z

안녕하세요~ 서브웨이 꼼꼼하게 리뷰 남겨주셔서 감사합니다! 😄
너무나도 좋은 리뷰 남겨주셔서 리뷰 반영하면서 넘 재밌었습니다!

혹시나 제가 의도와 다르게 리뷰를 반영했다면 코멘트 남겨주시면 감사하겠습니다 👍 👍
좋은 주말 보내세요!

joseph415

안녕하세요 허브!

3단계 미션 리뷰 잘 적용해주셨네요💯ㅎㅎ
제 코멘트 의도를 잘 파악하시고 잘 적용해주신 것 같습니다! 요구사항도 충족한 것 같아서 마지막 3단계는 이만 merge해도 될 것 같아요ㅎㅎ 지하철 미션하느라 고생 많으셨습니다~

마지막 다음 미션도 화이팅하세요!👍

joseph415 · 2023-05-21T14:47:10Z

src/main/java/subway/domain/fare/Passenger.java

@@ -2,13 +2,13 @@

 public class Passenger {

-    private AgeGroup ageGroup;
+    private int age;


아 요거는 age와 ageGroup을 다 상태값으로 들고 있어야하지 않을까 해서 의견드렸었는데, 이렇게 해도 상관없겠네요ㅎㅎ👍

joseph415 · 2023-05-21T15:56:59Z

src/main/resources/application.yml

    username: ${DB_USERNAME:sa}
    password: ${DB_PASSWORD:}
    driver-class-name: ${DB_DRIVER:org.h2.Driver}
+
+spring:


greeng00se added 27 commits May 16, 2023 22:08

refactor: repository 저장 기능 메서드 분리

ba0e43c

chore: 테스트 환경 변수 설정

c8c9d70

docs: 요구사항 정리

f1af934

refactor: 패키지명 변경

ee43e6c

refactor: StationService 파라미터명 변경

deb51c2

refactor: 도메인 패키지 변경

d41ebd4

feat: 모든 구간, 역을 반환하는 메서드 추가

0f8522c

chore: jgrapht 설정 추가

dea88c1

feat: 최단 경로 탐색 기능 구현

d7aeff9

feat: 요금 정보 추가

d87d0c6

feat: 경로 탐색 결과를 라인 번호 기준으로 묶어서 반환하도록 변경

0faa68c

feat: 기본 운임 정책, 거리 별 운임 정책 추가

2946f62

feat: 노선 별 운임 계산 기능 추가

ffb0fba

feat: 연령대 Enum 추가

6577ddc

feat: 연령대 Enum 반환하는 정적 팩터리 메서드 추가

d32bb52

feat: 사용자의 나이에 따라 금액을 할인하는 정책 추가

b308581

feat: 최종 운임을 계산하는 정책 추가

cbfdbe2

refactor: 5이하의 경우 잘못 계산되던 문제 수정

2ba154f

docs: 조회 API 수정

7e7649b

remove: 경로 상의 구간을 라인 기준으로 반환하는 기능 제거

1d57a55

test: 잘못된 테스트 수정

8156ed2

remove: 사용하지 않는 메서드 제거

ba52a5c

feat: 경로 조회 서비스 추가

b4c0e53

feat: 경로 조회 컨트롤러 추가

decd8eb

test: 경로 조회 테스트 추가

8ac02f2

docs: README 변경

b1f1b08

feat: 서비스에 Transactional 추가

166ca4e

joseph415 requested changes May 20, 2023

View reviewed changes

docs: 3단계 요구사항 추가

46adac9

greeng00se added 9 commits May 21, 2023 15:25

refactor: 최단 경로 검색 GET으로 변경

7546b6a

refactor: 역 추가 전략명 변경

c485ecc

feat: 경로를 표현하는 Path 클래스 추가

ed30188

refactor: PathFindResult 제거하고 Path 사용하도록 수정

ec7fcf8

remove: 사용하지 않는 Distance 생성자 제거

ba84f58

refactor: Passenger가 나이 정보를 가지고 있도록, AgeDiscountFarePolicy가 계산의 책임을 가…

32bc91f

…지고 있도록 변경

refactor: 도메인에 인터페이스를 두고 Jgrapht 의존성 분리

d5bc4a7

refactor: Enum String으로 받도록 수정

d94cbd7

chore: DB 설정

4d42c46

joseph415 approved these changes May 21, 2023

View reviewed changes

joseph415 merged commit be5023b into woowacourse:greeng00se May 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 경로 조회 기능] 허브(방대의) 미션 제출합니다. #126

[2단계 - 경로 조회 기능] 허브(방대의) 미션 제출합니다. #126

greeng00se commented May 17, 2023

joseph415 left a comment

joseph415 May 20, 2023

greeng00se May 21, 2023

joseph415 May 20, 2023

greeng00se May 21, 2023

joseph415 May 20, 2023

greeng00se May 21, 2023

joseph415 commented May 20, 2023 •

edited

Loading

greeng00se commented May 21, 2023

joseph415 left a comment

joseph415 May 21, 2023

joseph415 May 21, 2023

		@@ -19,6 +19,13 @@
		- [x] 노선에 포함된 역을 순서대로 보여줘야 한다.

[2단계 - 경로 조회 기능] 허브(방대의) 미션 제출합니다. #126

[2단계 - 경로 조회 기능] 허브(방대의) 미션 제출합니다. #126

Conversation

greeng00se commented May 17, 2023

도메인 식별자(id)에 대한 equals & hashcode

Repository 추가, 삭제 => 수정, 추가에 대한 부분

경로 조회

요금 정책과 할인 정책

joseph415 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

joseph415 commented May 20, 2023 • edited Loading

Repository 추가, 삭제 => 수정, 추가에 대한 부분

greeng00se commented May 21, 2023

joseph415 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph415 commented May 20, 2023 •

edited

Loading