Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3단계 - 요금 정책 추가 #218

Open
wants to merge 6 commits into
base: artium59
Choose a base branch
from
Open

Conversation

artium59
Copy link

@artium59 artium59 commented Aug 15, 2022

Summary

추가된 요금 정책

  • 노선별 추가 요금
    • 추가 요금이 있는 노선을 이용 할 경우 측정된 요금에 추가
    • 경로 중 추가요금이 있는 노선을 환승 하여 이용 할 경우 가장 높은 금액의 추가 요금만 적용
  • 로그인 사용자의 경우 연령별 요금으로 계산
    • 청소년: 운임에서 350원을 공제한 금액의 20%할인
    • 어린이: 운임에서 350원을 공제한 금액의 50%할인

Comment

  • 나이별 요금이 들어가니까 @ParameterizedTest가 있는 것이 더 깔끔해져서 쓰게 되었습니다.
  • 늦어서 죄송합니다.

Copy link
Member

@testrace testrace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 😃
3단계 미션 잘 수행해 주셨네요 👍

몇 가지 코멘트로 남겨두었으니 확인해 주시고 리뷰 요청 부탁드립니다.
궁금한 내용이 있으시면 편하게 DM 주세요!

Comment on lines +34 to +40
private int calculateByAge(int age, int price) {
if (age < 13 && age >= 6) {
return (price - 350) / 10 * 5;
} else if (age < 19 && age >= 13) {
return (price - 350) / 10 * 8;
}
return price;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석 없이는 쉽게 이해하기 힘든 코드라고 생각드네요 😢
(객체지향 생활 체조 규칙 2: else 예약어를 쓰지 않는다.)

할인 정책이 추가되거나 변경되면 조건문이 무한히 늘어날 수 을 것 같아요
정책을 효과적으로 관리할 수 있는 방법은 어떤게 있을까요?

this.lineService = lineService;
this.stationService = stationService;
this.pathResponseBuilder = pathResponseBuilder;
}

public PathResponse findPath(Long source, Long target, SearchType searchType) {
public PathResponse findPath(String email, Long source, Long target, SearchType searchType) {
int age = memberService.findMember(email).getAge();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

회원가입 되어 있는 회원이 로그인 상태가 아니라면 이 서비스는 이용을 못할 것 같네요
요구사항에 있듯이 로그인 사용자의 경우 연령별 요금 계산을 적용하고
비회원(로그인하지 않은 상태)에서는 연령별 할인 정책이 적용되지 않은 상태에서 경로를 조회할 수 있으면 좋을 것 같아요

import nextstep.subway.constant.SearchType;
import org.springframework.http.MediaType;

public class PathSteps extends AcceptanceTestSteps {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PathSteps를 분리해 주셨네요 👍

Comment on lines +17 to +19
@ParameterizedTest(name = "{index}: {3}: {0}km 이용 = {2}원")
@MethodSource("이용_거리_0km")
void 이용_거리가_0이면_요금도_0원이다(int distance, int age, int price, String message) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message 파라미터를 테스트의 이름으로 활용하시네요 👍

Comment on lines +41 to +47
private static int 연령별_요금(int age, int price) {
if (age < 13 && age >= 6) {
return (price - 350) / 10 * 5;
} else if (age < 19 && age >= 13) {
return (price - 350) / 10 * 8;
}
return price;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

프로덕션 코드와 동일한 코드 혹은 테스트 코드내에서 비즈니스 로직 코드가 있다면
관리포인트도 늘어나고 유지보수면에서 쉽지 않을 일이 될 것 같은데요
이 메서드가 테스트 코드 내에서 꼭 필요할까요?

Comment on lines +104 to +106
Arguments.of(51, 6, 연령별_요금(6, 이용거리_51km_요금), "어린이"),
Arguments.of(51, 16, 연령별_요금(16, 이용거리_51km_요금), "청소년"),
Arguments.of(51, 26, 연령별_요금(26, 이용거리_51km_요금), "성인"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반복되는 매직넘버, 매직 리터럴은 상수화 해도 좋을 것 같아요 😃

삼호선 = 지하철_노선_생성_요청("3호선", "orange", 교대역, 남부터미널역, 2, 10);
이호선 = 지하철_노선_생성_요청("2호선", "green", 교대역, 강남역, 10, 4, 200);
신분당선 = 지하철_노선_생성_요청("신분당선", "red", 강남역, 양재역, 10, 4, 1000);
삼호선 = 지하철_노선_생성_요청("3호선", "orange", 교대역, 남부터미널역, 2, 10, 300);

지하철_노선에_지하철_구간_생성_요청(관리자, 삼호선, createSectionCreateParams(남부터미널역, 양재역, 3, 1));
}

@DisplayName("두 역의 최단 거리 경로를 조회한다.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 내용이 변경됨에 따라 DisplayName도 변경하면 어떨까요?
경로와 시간, 요금까지 모두 조회할 수 있다는 표현이 포함되면 좋을 것 같아요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants