Step3 3단계 요금 정책 추가 리뷰 부탁드립니다. #235
Conversation
There was a problem hiding this comment.
안녕하세요~ 리뷰가 조금 늦었네요ㅠ
양해 부탁드립니다 🥲
먼저 테스트를 비롯하여 전반적을 코드를 아주 깔끔하게 잘 구현해주셨어요!
특별히 코멘트 드릴 부분이 많지는 않았지만
경로 계산하는 로직 관련해서 코멘트를 남겼습니다.
- Path가 age를 가지고 있는 부분
- fare 추출하는 로직이 조금 더 유연했으면 하는 부분
이렇게 크게 두 부분에 대해서 의견을 남겼어요.
현재 Path는 어떤 요금 계산 정책을 가졌는지에 따라 코드 수정이 불가피한 구조라고 볼 수 있는데요~
Path는 경로와 관련된 책임만 가지게 하고 요금 계산과 관련된 부분은 외부로 위임하여
요금 정책 변경에 영향을 받지 않도록 리팩터링 해보시는 것을 추천드립니다!
| String 청소년; | ||
| String 어린이; | ||
| String 성인; |
There was a problem hiding this comment.
역할에 따라 먼저 지정해두면 토큰을 미리 준비해두는 방법 좋은 것 같아요 👍
참고로 한 테스트에서만 사용한다면 지역 변수로 지정하고 해도 됩니다.
| * 신사역 | ||
| * | (46km, 5분) | ||
| * 고속터미널역 | ||
| * | | ||
| * 교대역 --- *2호선*(10km, 10분) --- 강남역 | ||
| * | | | ||
| * *3호선*(2km,5분) (추가요금 900원) *신분당선* (10km, 10분) (추가요금 500원) | ||
| * | | | ||
| * 남부터미널역 --- *3호선*(3km, 5분) (추가요금 900원) --- 양재 | ||
| */ |
There was a problem hiding this comment.
가독성 측면에서 주석을 이용한 테스트 설명 아주 좋아요 👍
|
|
||
| @DisplayName("어린이(6세 이상~ 13세 미만) 운임에서 350원을 공제한 금액의 50%할인") | ||
| @ParameterizedTest | ||
| @ValueSource(ints = {6, 12}) |
| Line line = new Line("2호선", "green"); | ||
| Line line = new Line("2호선", "green", 900); |
There was a problem hiding this comment.
반복되는 수정이 불편해지시면
기본 Line을 생성하는 로직을 분리해서 재사용해도 좋았을 것 같습니다~
There was a problem hiding this comment.
다 수정후 고친거긴하지만 createFare는 메서드로 분리해봤습니다
| DistanceFarePolicy distanceFarePolicy = DistanceFarePolicy.create(extractDistance()); | ||
| int fare = distanceFarePolicy.calculateFare() + getSections().lineExtraFare(); | ||
| DiscountFarePolicy discountFarePolicy = DiscountFarePolicy.create(fare, getAge()); |
There was a problem hiding this comment.
요금 정책이 거리정책, 추가요금, 할인정책으로 나뉘어져있는데요
이 부분을 하나의 추상적인 요금 체계를 갖출 수 없을까요?
예를 들면 Path에서 요금 정책을 주입 받고
주입 받은 요금 정책에 현재 정보를 넘겨주면 요금이 출력되는 것 처럼요
There was a problem hiding this comment.
Fare Class를 생성해서 요금과 관련된 로직들을 수행하도록 해봤습니다.
혹시 거리 요금 정책과 할인 정책으로 분리된 정책을 하나의 정책으로 통합시키라는 의미일까요?
| @Getter | ||
| public class Path { | ||
| private Sections sections; | ||
| private int age; |
There was a problem hiding this comment.
경로에 나이라는 상태는 조금 어색한 것 같아요~
There was a problem hiding this comment.
요금 계산 관련 클래스로 나눔으로써 age는 제외했습니다
늦어서 죄송합니다. ㅠㅠ