🚀 3단계 - 요금 정책 추가 #468

gutenLEE · 2024-03-06T16:04:39Z

내일이 연장해주신 실시간 리뷰 신청 마지막날이라 급하게 개발했네요 ㅠㅠ
마지막 리뷰 잘 부탁드립니다!

This reverts commit bac0da4.

- 노선별 추가 요금

jaranda-laura · 2024-03-06T16:06:41Z

src/main/java/nextstep/path/domain/Fare.java

+            fare += distanceFare.calculateFare(this);
+        }
+        addExtraFare();
+        this.fare = discount(discountCondition);


다시보니 값을 반환하여 재할당 보다는 discount() 내부에서 셋팅해주는게 좋았을듯합니다

jaranda-laura · 2024-03-06T16:07:23Z

src/main/java/nextstep/path/domain/Fare.java

+    private void addExtraFare() {
+        HashSet<LineFare> fares = new HashSet<>(lineFares);
+        for (LineFare lineFare : fares) {
+            if (lineFare.hasExtraFare()) {
+                addFare(lineFare.getExtraFare());
+            }
        }
    }


구간이 속한 라인의 추가요금을 가져와야 해서 이렇게 했습니다...
이 부분에서 시간이 많이 걸렸네요 !

jaranda-laura · 2024-03-06T16:09:50Z

src/main/java/nextstep/path/domain/SectionEdge.java

SectionEdge가 요금을 알아야 하는게 맞나 고민이 많이 되었습니다.

junwoochoi

안녕하세요! 몇가지 추가로 확인해보시면 좋을 부분들이 있어서 코멘트 드렸습니다!
확인 후 다시 요청 주세요 ~

junwoochoi · 2024-03-07T12:18:22Z

src/main/java/nextstep/auth/PathAuthenticationPrincipal.java

+@Target(ElementType.PARAMETER)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface PathAuthenticationPrincipal {
+}


기존 AuthenticationPrincipal을 활용하지 않은 이유가 있나요? 👀

junwoochoi · 2024-03-07T12:21:01Z

src/test/java/nextstep/path/DistanceFareTest.java

-        for (DistanceFare fare : distanceFare) {
-            sum += fare.calculateFare(16);
+        for (DistanceFare sut : distanceFare) {
+            sum += sut.calculateFare(FareSteps.buildFare(9));


DistanceFareTest중에 2개 테스트케이스가 깨지고 있습니다. 확인 후 수정 부탁드립니다~

@ParameterizedTest를 활용해보시는 것도 추천드려요!

junwoochoi · 2024-03-07T12:23:22Z

src/main/java/nextstep/path/ui/PathController.java

+            @RequestParam int source,
+            @RequestParam int target,
+            @RequestParam PathType type,
+            @PathAuthenticationPrincipal UserDetail userDetail) {
+        return ResponseEntity.ok(pathService.searchPath(source, target, type, userDetail));


로그인하지 않은 유저도 Path조회가 가능해야합니다!
@RequestParam의 required필드와 같은 방식을 도입하는것도 고려해보면 어떨까요?

junwoochoi · 2024-03-07T12:27:46Z

src/main/java/nextstep/path/domain/Fare.java

-    int fare = 0;
-    public void calculateFare(int distance, List<DistanceFare> distanceFares) {
+
+public class Fare implements Discountable {


상황마다 다르지만, 상속보다는 조합을 사용하는 것이 더 유연한 구조가 될 수도 있다는 점도 확인해보시면 좋겠습니다 :)
https://tecoble.techcourse.co.kr/post/2020-05-18-inheritance-vs-composition/

junwoochoi · 2024-03-07T12:29:28Z

src/main/java/nextstep/path/service/PathService.java

+    public PathsResponse searchPath(long source, long target, PathType pathType, UserDetail userDetail) {
        try {
            PathFinder pathFinder = createPathFinder(lineRepository.findAll(), pathType);
            PathsDto pathsDto = pathFinder.findPath(getStation(source), getStation(target));
-            Fare fare = new Fare();
-            fare.calculateFare(pathsDto.getDistance(), DistanceFareFactory.createDistanceFare());
+            List<SectionEdge> sectionEdges = pathsDto.getSectionEdges();
+            List<LineFare> lineFares = sectionEdges.stream().map(SectionEdge::getLineFare).collect(Collectors.toList());
+
+            Fare fare = new Fare(lineFares, pathsDto.getDistance());
+            fare.calculateFare(DistanceFareFactory.createDistanceFare(), DiscountConditionFactory.createDiscountCondition(userDetail.getAge()));


최소시간 기준으로 조회하거나, 최단거리 기준으로 조회하거나 요금은 항상 최단거리 기준으로 계산이 되어야합니다!
지하철 운임은 거리비례제로 책정됩니다. (실제 경로가 아닌 최단거리 기준)

junwoochoi · 2024-03-07T12:31:23Z

src/test/resources/features/path.feature

+    And 지하철 이용 요금 3250원도 함께 응답함
+
+
+  Scenario: 성인 유저가 두 역의 노선중 추가 할당 금액이 있는 경우 최소 시간 경로를 조회


시나리오는 성인유저인데, 하단의 when에서는 어린이로 되어있네요 😢

junwoochoi · 2024-03-07T12:35:49Z

src/main/java/nextstep/path/domain/Fare.java

+    }
+
+
+    public void calculateFare(List<DistanceFare> distanceFares, DiscountCondition discountCondition) {


calculateFare가 외부에서 여러번 호출되면 내부값이 계속 변경될거같은데 필드변수로 fare를 가지고 있는 것과 매번 계산하는 것은 어떤 차이가 있을까요~?

junwoochoi · 2024-03-07T12:36:59Z

src/main/java/nextstep/path/domain/Fare.java

+        HashSet<LineFare> fares = new HashSet<>(lineFares);
+        for (LineFare lineFare : fares) {
+            if (lineFare.hasExtraFare()) {
+                addFare(lineFare.getExtraFare());


경로 중에 노선 추가요금이 여러건 있는 경우 그중에 가장 높은 추가요금만 적용되어야합니다.

경로 중 추가요금이 있는 노선을 환승 하여 이용 할 경우 가장 높은 금액의 추가 요금만 적용 ex) 0원, 500원, 900원의 추가 요금이 있는 노선들을 경유하여 8km 이용 시 1,250원 -> 2,150원

junwoochoi · 2024-03-07T12:39:21Z

src/main/java/nextstep/path/domain/ChildDiscountCondition.java

+
+import java.math.BigDecimal;
+
+public class ChildDiscountCondition implements DiscountCondition {


TeenagerDiscountCondition과 다른 클래스로 나눠질 이유가 있을까요? 하나의 구현체로 관리해봐도 좋을 것 같습니다

junwoochoi · 2024-03-07T12:39:51Z

src/main/java/nextstep/path/domain/ChildDiscountCondition.java

+    public boolean support() {
+        return true;
+    }


support가 어떤 이유로 존재하는걸까요?

성인의 경우는 할인을 제공하지 않는다 의미였습니다

jaranda-laura added 9 commits March 5, 2024 23:16

refactor : path 인수테스트 코드 정리

bac0da4

Revert "refactor : path 인수테스트 코드 정리"

6f757cb

This reverts commit bac0da4.

refactor : path 인수테스트 코드 정리

c3b4060

feat : 인수테스트 작성

4907d9a

- 노선별 추가 요금

feat : 노선별 추가 요금완

9f6b306

fix : 사용하지 않는 클래스 제거

56e4b0c

feat : 어린이, 청소년 요금할인 인수테스트 작성

efda17f

chore : 오타 수정

f22f859

feat : 유저 나이대별 할인 기능 개발 & 단위테스트

25230d7

jaranda-laura reviewed Mar 6, 2024

View reviewed changes

junwoochoi requested changes Mar 7, 2024

View reviewed changes

		And 지하철 이용 요금 3250원도 함께 응답함


		Scenario: 성인 유저가 두 역의 노선중 추가 할당 금액이 있는 경우 최소 시간 경로를 조회

		}


		public void calculateFare(List<DistanceFare> distanceFares, DiscountCondition discountCondition) {


		import java.math.BigDecimal;

		public class ChildDiscountCondition implements DiscountCondition {

🚀 3단계 - 요금 정책 추가 #468

Are you sure you want to change the base?

🚀 3단계 - 요금 정책 추가 #468

Uh oh!

Conversation

gutenLEE commented Mar 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junwoochoi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants