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

loop-study · 2022-02-23T03:21:31Z

안녕하세요 리뷰어님!
박현철입니다.
이번 단계를 진행하면서 여러가지 질문이 있습니다.

아직은 생소한 JWT 로그인에 NullLoginMember 객체를 추가해보려고 시도해봤는데, 해당 방식이 맞는지 궁금합니다.
PathController에 LoginMember 의존성이 추가되어서, PathController에서 나이만 뽑아서 운임 비용에 사용하고 있습니다.
나이가 아니라 LoginMember 의존성 자체를 서비스계층과 도메인계층까지 넘기는 방식과 비교하면 무엇이 더 좋다고 생각 되시는지 궁금합니다.
단위테스트에서 LineFixture로 Line, Station 생성을 몰아 넣었습니다.
규모가 커질 수록 픽스쳐의 로직도 많아지고, 이를 사용하는 테스트도 많아질텐데 추후 픽스쳐데이터 복잡성은 어떤 방식으로 관리하는지 궁금합니다.

확인 부탁드립니다. 🙇🏻‍♀️
감사합니다

2022. 02. 22. Path - 조회된 경로만 담는 걸로 복구 PathResponse - 요청한 경로와 최단 경로 받는 형태로 수정 PathService - PathResponse.of() 호출 형식에 맞게 로직 수 Resolves : loop-study See also : junwoochoi

2022. 02. 22. README.md - 요금 정책 요구사항 정리 Resolves : loop-study See also : junwoochoi

2022. 02. 22. LineAcceptanceTest, LineSteps - 노선 생성 인자에 노선 요금 추가 Resolves : loop-study See also : junwoochoi

2022. 02. 22. Line, LineRequest - 추가 요금 필드 추가 LineService - 노선 생성 시, 추가 요금 넣는 로직 추가 LineFixture - 노선 픽스처 생성 LineServiceMockTest, LineServiceTest, LineTest, SubwayMapTest - 추가된 필드에 관련된 테스트 수정 Resolves : loop-study See also : junwoochoi

2022. 02. 22. SubwayMapTest - 지하철역과 노선 관계 표현 Resolves : loop-study See also : junwoochoi

2022. 02. 22. 요금 필드 BigDecimal로 변경 Resolves : loop-study See also : junwoochoi

2022. 02. 22. AgeFareTest - 나이에 따른 요금할인 테스트 추가 Resolves : loop-study See also : junwoochoi

2022. 02. 22. PathFare -> DistanceFare로 변경 Resolves : loop-study See also : junwoochoi

2022. 02. 22. AgeFare - 나이별 요금제 분리 Resolves : loop-study See also : junwoochoi

2022. 02. 22. AgeFare - 나이별 요금액 기능 구현 AgeFareTest - 나이별 할인금액 테스트 추가 Resolves : loop-study See also : junwoochoi

2022. 02. 22. PathAcceptanceTest - 토큰 로그인 테스트 추가 Resolves : loop-study See also : junwoochoi

2022. 02. 22. AuthenticationPrincipal - 비로그인 여부 필드 추가 AuthenticationPrincipalArgumentResolver - 비로그인용 널객체 주입 로직 추가 NullLoginMember - 비로그인용 널객체 생성 PathController - LoginMember 인자 추가 PathService - 경로찾는 메서드에 나이 인자 추가 PathDocumentation - 변경된 로직에 맞게 스텁빙 수정 Resolves : loop-study See also : junwoochoi

2022. 02. 22. PathFare - 정책별 요금 계산하는 객체 생성 PathService - 운임 관련 로직 변경 PathResponse - 인자 Path -> PathFare 변경 Sections, Section, Path - 최대 추가 운임 구하는 로직 추가 Path - 이전에 운임 비용 구하는 메서드 제거 PathDocumentation - 변경된 로직에 맞게 스텁빙 수정 SubwayMapTest - 최대 운임 구하는 테스트 추가 PathAcceptanceTest - 나이별 요금제 및 추가운임 인수테스트 추가 Resolves : loop-study See also : junwoochoi

2022. 02. 23. README.md - 체크리스트 수정 Resolves : loop-study See also : junwoochoi

2022. 02. 23. LineFixture - Station 생성 로직 추가 LineServiceMockTest, LineTest, SubwayMapTest - new Station 코드 -> createStation 호출로 변경 Resolves : loop-study See also : junwoochoi

junwoochoi

안녕하세요! 현철님!
미션 정말 깔끔히 잘 수행해주셨네요 😄
테스트도 꼼꼼히 해주시고, 로직도 도메인에 응집되게 잘 구현해주신 것 같습니다! 💯

몇가지 간단한 코멘트 달아보았는데 확인해주시면 감사하겠습니다!

저는 작성하신 방법 좋다고 생각합니다. null로 리턴할 수도 있지만, 그렇게 되면 null체크가 매번 필요하기도 하고 뭔가 명확하지 않은 것 같아서요 :) 더 궁금하시면, Spring Security에서 AnonymousAuthenticationToken이라는 클래스는 어떻게 되어있는지 구경해보는 것도 좋은 방법으로 보입니다!
저는 나이만 넣는 것보다는 LoginMember 객체를 넘기는게 더 좋다고 생각합니다. 현재는 나이만 기준이지만, LoginMember에 예를 들어 성별이 들어가거나, 직업이 들어가거나, 기타 등등의 유저 정보가 들어가게 되고, 그에 따른 할인정책이 추가적으로 나올 수 있지 않을까......? 하는 생각입니다.
지금처럼 도메인별로 묶어서 팩토리 메소드를 만들어서 사용하시는 것 좋다고 생각합니다. 각 테스트 케이스나 상황에 따라 팩토리 메소드의 네이밍으로 해당 케이스에 대한 의미를 넣어주면 가독성이 올라가면서 관리도 조금 더 용이해지지 않을까 싶기도 하네요 :)

src/main/java/nextstep/auth/authorization/AuthenticationPrincipal.java

src/main/java/nextstep/auth/authorization/AuthenticationPrincipalArgumentResolver.java

src/main/java/nextstep/subway/domain/AgeFare.java

src/main/java/nextstep/subway/domain/DistanceFare.java

src/main/java/nextstep/member/domain/NullLoginMember.java

src/main/java/nextstep/subway/domain/PathFare.java

src/test/java/nextstep/subway/unit/AgeFareTest.java

2022. 02. 25. final 선언 Resolves : loop-study See also : junwoochoi

2022. 02. 25. PathFare - 지하철운임 계산로직 생성시점으로 변경 Resolves : loop-study See also : junwoochoi

2022. 02. 25. PathController - PathService에 LoginMember 객체로 넘김 - 관련된 로직 변경 Resolves : loop-study See also : junwoochoi

2022. 02. 25. NullLoginMember - 싱글톤으로 변경 Resolves : loop-study See also : junwoochoi

loop-study · 2022-02-25T06:42:41Z

안녕하세요 리뷰어님!
박현철입니다.
피드백 받자마자 곧바로 진행해야했지만
개인사정으로 이제야 반영했습니다.
확인 부탁드립니다. 🙇🏻‍♀️
감사합니다!

junwoochoi

안녕하세요! 마지막 미션까지 열심히 달려오시느라 정말 고생많으셨습니다!!

앞으로도 업무 및 학습하시면서 ATDD 많이 활용해주시길 응원하겠습니다! 🥇

src/main/java/nextstep/auth/AuthConfig.java

src/test/java/nextstep/subway/documentation/PathDocumentation.java

junwoochoi · 2022-02-26T05:05:05Z

src/main/java/nextstep/member/domain/NullLoginMember.java

+    public static NullLoginMember getInstance() {
+        return instance;
+    }


싱글톤 객체 구현 👍
https://readystory.tistory.com/116
싱글톤을 구현하는 여러가지 방법이 있으니 한번 함께 참고해보시면 좋겠네요 :)

링크 감사합니다!

loop-study added 15 commits February 22, 2022 17:16

refactor : 피드백 반영

7f91ab5

2022. 02. 22. Path - 조회된 경로만 담는 걸로 복구 PathResponse - 요청한 경로와 최단 경로 받는 형태로 수정 PathService - PathResponse.of() 호출 형식에 맞게 로직 수 Resolves : loop-study See also : junwoochoi

docs : step3 요구사항 정

0add536

2022. 02. 22. README.md - 요금 정책 요구사항 정리 Resolves : loop-study See also : junwoochoi

test : 인수테스트 변경

0bbd642

2022. 02. 22. LineAcceptanceTest, LineSteps - 노선 생성 인자에 노선 요금 추가 Resolves : loop-study See also : junwoochoi

refactor : 가독성용 주석 추가

7563fd8

2022. 02. 22. SubwayMapTest - 지하철역과 노선 관계 표현 Resolves : loop-study See also : junwoochoi

refactor : int -> BigDecimal로 변경

1d6efb3

2022. 02. 22. 요금 필드 BigDecimal로 변경 Resolves : loop-study See also : junwoochoi

test : 나이에 따른 요금할인 테스트 추가

8ead5cb

2022. 02. 22. AgeFareTest - 나이에 따른 요금할인 테스트 추가 Resolves : loop-study See also : junwoochoi

refacotr : 거리별 운임제 클래스명 변경

6910c1a

2022. 02. 22. PathFare -> DistanceFare로 변경 Resolves : loop-study See also : junwoochoi

feat : 나이별 요금 분류 구현

54a44c1

2022. 02. 22. AgeFare - 나이별 요금제 분리 Resolves : loop-study See also : junwoochoi

feat : 나이별 요금 할인금액 구현

b9d53da

2022. 02. 22. AgeFare - 나이별 요금액 기능 구현 AgeFareTest - 나이별 할인금액 테스트 추가 Resolves : loop-study See also : junwoochoi

test : 경로 인수테스트 추가

e2de04c

2022. 02. 22. PathAcceptanceTest - 토큰 로그인 테스트 추가 Resolves : loop-study See also : junwoochoi

docs : 문서 수정

77244e9

2022. 02. 23. README.md - 체크리스트 수정 Resolves : loop-study See also : junwoochoi

refactor : 중복코드 제거

2ca675f

2022. 02. 23. LineFixture - Station 생성 로직 추가 LineServiceMockTest, LineTest, SubwayMapTest - new Station 코드 -> createStation 호출로 변경 Resolves : loop-study See also : junwoochoi

junwoochoi requested changes Feb 23, 2022

View reviewed changes

loop-study added 4 commits February 25, 2022 15:04

refactor : 피드백 반영

fcafc99

2022. 02. 25. final 선언 Resolves : loop-study See also : junwoochoi

refactor : 피드백 반영

32469d3

2022. 02. 25. PathFare - 지하철운임 계산로직 생성시점으로 변경 Resolves : loop-study See also : junwoochoi

refactor : 피드백 반영

ca2b1de

2022. 02. 25. PathController - PathService에 LoginMember 객체로 넘김 - 관련된 로직 변경 Resolves : loop-study See also : junwoochoi

refactor : 피드백 반영

45b0c67

2022. 02. 25. NullLoginMember - 싱글톤으로 변경 Resolves : loop-study See also : junwoochoi

junwoochoi approved these changes Feb 26, 2022

View reviewed changes

junwoochoi merged commit d457839 into next-step:loop-study Feb 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

loop-study commented Feb 23, 2022

junwoochoi left a comment

loop-study commented Feb 25, 2022

junwoochoi left a comment

junwoochoi Feb 26, 2022

loop-study Feb 28, 2022

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

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

Conversation

loop-study commented Feb 23, 2022

junwoochoi left a comment

Choose a reason for hiding this comment

loop-study commented Feb 25, 2022

junwoochoi left a comment

Choose a reason for hiding this comment

junwoochoi Feb 26, 2022

Choose a reason for hiding this comment

loop-study Feb 28, 2022

Choose a reason for hiding this comment