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

[스프링 - 협업 미션] 케빈(박진홍) 미션 제출합니다. #7

Merged
merged 62 commits into from
Jun 5, 2021

Conversation

xlffm3
Copy link

@xlffm3 xlffm3 commented May 28, 2021

안녕하세요 코니. 자동차 미션 이후로 또 만나게 되서 반갑습니다! :)

배포 주소는 https://jipark.p-e.kr/ 입니다.
프론트엔드 크루들과 협업하면서 논의한 것은 예외 상황에 어떤 에러 코드와 메시지를 반환할 것인가에 대한 내용이라, 대부분의 api 스펙은 https://woowacourse.github.io/atdd-subway-fare/ 와 동일합니다. 해당 부분에 대해서도 문서를 작성해보겠습니다..!


질문 1

경로 조회(GET /path) 요청에 대해

  • 로그인과 비로그인 멤버를 전부 허용하게 하라.
  • 단, 로그인 멤버의 경우 나이에 따라 요금 조회 할인을 적용하라.

는 요구사항이 있었는데요. 이 때문에 Interceptor와 ArgumentResolver 쪽에서 Http 요청의 url이 /path 일 때는 로그인이 아니더라도 허용하게끔 처리를 해줬습니다. 이후 서비스 레이어 로직에서 LoginMember가 null인지 아닌지 여부에 따라 계산 로직을 다르게 진행합니다.

그런데 로그인과 비로그인 멤버 모두를 허용하는 url이 많아질수록 Interceptor 혹은 ArgumentResolver 쪽에 if 분기가 많아질 것으로 생각되는데요. 어떤 방식으로 이를 보완할 수 있을까요? 이번 미션 내내 머리를 굴려봐도 별다른 방법이 생각나지 않네요...

xlffm3 and others added 30 commits May 21, 2021 15:17
- dto 정적 팩토리 메서드
- service id 유효성 검사 메서드
Copy link

@choihz choihz left a comment

Choose a reason for hiding this comment

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

안녕하세요, 케빈! 정말 오랜만에 다시 뵙네요 👋

협업 미션을 아주 잘 구현해 주셨네요 👍 테스트 코드도 꼼꼼하게 잘 작성해 주셨고요. 피드백 보시고 궁금한 점이나 의견이 있으시다면 언제든 코멘트 또는 DM 주세요~

Comment on lines 38 to 40
private boolean isAnonymousPermissible(HttpServletRequest httpRequest, String credentials) {
return Objects.isNull(credentials) && "/paths".equals(httpRequest.getRequestURI());
}
Copy link

Choose a reason for hiding this comment

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

그런데 로그인과 비로그인 멤버 모두를 허용하는 url이 많아질수록 Interceptor 혹은 ArgumentResolver 쪽에 if 분기가 많아질 것으로 생각되는데요. 어떤 방식으로 이를 보완할 수 있을까요? 이번 미션 내내 머리를 굴려봐도 별다른 방법이 생각나지 않네요...

로그인과 비로그인 멤버 모두를 허용할 때 사용하는 ArgumentResolver 및 어노테이션을 별도로 만드는 방법이 최선이지 않을까 해요. 저도 이런 상황을 다뤄본 적은 없어서... 🙄

@@ -5,43 +5,40 @@
import wooteco.subway.auth.dto.TokenRequest;
Copy link

Choose a reason for hiding this comment

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

org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'authService' defined in file [/Users/user/Documents/choihz/woowa/atdd-subway-fare/build/classes/java/main/wooteco/subway/auth/application/AuthService.class]: Unsatisfied dependency expressed through constructor parameter 1; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'jwtTokenProvider': Injection of autowired dependencies failed; nested exception is java.lang.IllegalArgumentException: Could not resolve placeholder 'security.jwt.token.secret-key' in value "${security.jwt.token.secret-key}"
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:800) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:229) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1354) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1204) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:564) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:524) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:335) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:333) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:944) ~[spring-beans-5.3.4.jar:5.3.4]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:917) ~[spring-context-5.3.4.jar:5.3.4]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:582) ~[spring-context-5.3.4.jar:5.3.4]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:144) ~[spring-boot-2.4.3.jar:2.4.3]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:767) ~[spring-boot-2.4.3.jar:2.4.3]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:759) ~[spring-boot-2.4.3.jar:2.4.3]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:426) ~[spring-boot-2.4.3.jar:2.4.3]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:326) ~[spring-boot-2.4.3.jar:2.4.3]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1311) ~[spring-boot-2.4.3.jar:2.4.3]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1300) ~[spring-boot-2.4.3.jar:2.4.3]
	at wooteco.subway.SubwayApplication.main(SubwayApplication.java:10) ~[main/:na]

로컬에서 어플리케이션 실행이 안 되네요 😢

스크린샷 2021-05-30 오전 4 36 23

제공해 주신 주소로 접근 시에는 위와 같은 화면만 보이고 있어요.

Copy link
Author

Choose a reason for hiding this comment

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

앗 ec2에서는 실행할때 -Dspring.active.profiles=prod 옵션을 줬는데 로컬에서는 별도로 옵션 지정을 안 해놨네요. 해당 부분 수정하겠습니다 :)

그리고 배포한 서버는 프론트 엔드 코드는 없고 서버만 돌고 있어서 postman으로 테스트할수밖에 없네요... ㅠㅠ;;;

Comment on lines 38 to 43
private boolean checkIfAnonymousPermissible(HttpServletRequest request) {
if ("/paths".equals(request.getRequestURI())) {
return true;
}
throw new LoginRequiredException();
}
Copy link

Choose a reason for hiding this comment

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

AuthenticationPrincipalConfig에서 처리하지 않은 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

코니의 로그인과 비로그인 멤버 모두를 허용할 때 사용하는 ArgumentResolver 및 어노테이션을 별도로 만드는 방법이 최선이지 않을까 해요. 저도 이런 상황을 다뤄본 적은 없어서... 피드백을 보고 나니

굳이 이렇게 처리할 필요가 없겠네요. WebMvcConfigurer의 설정에서 로그인 비로그인 확인이 필요없는 경우 인터셉터에서 제외하고, 새로운 어노테이션과 아규먼트 리졸버를 사용하는 방법도 괜찮을것같습니다.

Comment on lines 3 to 8
public class WrongEmailException extends AuthorizationException {
private static final String MESSAGE = "잘못된 이메일입니다.";

public WrongEmailException() {
super(MESSAGE);
}
Copy link

Choose a reason for hiding this comment

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

예외가 굉장히 많은데요, 프로젝트가 커지면 커질수록 관리하기가 굉장히 힘들 것 같아요. 모든 상황마다 그 상황에서만 사용할 수 있는 예외를 생성하는 게 적절할까요?

Copy link
Author

@xlffm3 xlffm3 May 31, 2021

Choose a reason for hiding this comment

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

원래는 표준 예외를 선호하는데 에러에 따른 상태코드 / 메시지 등을 관리하기 위해 커스텀 예외를 생성하게 되었는데요.
말씀하신 것처럼 프로젝트가 커지면서 예외 클래스가 너무 많아지는것같아요.

다양한 방식을 고민해보다가

7ef1330

Enum을 사용해보았어요. 사실 예외 핸들링에 대해 여러 글을 읽어봐도, 각각의 방식마다 trade-off가 존재하는것같아서 피드백 부탁드려요! (지금 생각나는 현 방식의 가장 큰 단점은 도메인이나 서비스에서 예외를 발생시킬 때 특정 예외 상수 구현에 강하게 의존하는 문제입니다. 다만 예외를 호출할 때만 사용된다는 특수성을 생각해볼 때 괜찮을 것 같다는 생각도 들구요...!)

사실 현재 구조에서는 각 enum에 포함된 status int 값이 별 의미는 없고, InvalidValueException은 400, EntityNotFound는 404, AuthException은 401로 일괄 처리해도 동일한데요. 그런데 비슷한 예외들이더라도, 에러 응답코드가 달라질 상황도 있으리라 생각해서 추가했습니다...

Copy link

Choose a reason for hiding this comment

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

지금은 학습하는 단계이기 때문에 여러 가지 방법을 시도해 보고 각각의 장단점을 느껴보시는 것도 좋다는 생각이 들어 남긴 코멘트였어요. 사실 저는 거의 모든 상황마다 예외를 만들고 있긴 합니다 🙄 다만, 예외를 한 곳에 모으기보다 사용하는 쪽에 배치하고 있어요. 이렇게 할 때의 장단점은 또 뭐가 있을지 고민해 보시면 좋을 것 같네요.

Comment on lines 10 to 13
INFANT(age -> age < 6, fare -> 0),
CHILD(age -> 6 <= age && age < 13, fare -> (fare - 350) / 2),
ADOLESCENT(age -> 13 <= age && age < 19, fare -> (int) ((fare - 350) * 0.8)),
ADULT(age -> age >= 19, UnaryOperator.identity());
Copy link

Choose a reason for hiding this comment

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

매직넘버! 나이 말고 350, 0.8 같은 수는 의미를 드러낼 필요가 있어 보여요.

Copy link
Author

Choose a reason for hiding this comment

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

반영했습니다 :)

c3622ee

public Line(Long id, String name, String color) {
this.id = id;
this.name = name;
this.color = color;
}

public Line(Long id, String name, String color, int extraFare) {
Copy link

Choose a reason for hiding this comment

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

사용되지 않는 생성자는 지워도 될 것 같아요. 그리고 this()를 이용해 중복을 제거해 보는 건 어떨까요?

Copy link
Author

@xlffm3 xlffm3 May 31, 2021

Choose a reason for hiding this comment

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

매의눈,,, 반영했습니다! c9ec1de

Comment on lines +63 to +64
@DisplayName("회원 생성시 중복된 이메일로 생성할 수 없다.")
@Test
Copy link

Choose a reason for hiding this comment

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

테스트 메서드가 위에 배치되는 것이 좋지 않을까요?

Copy link
Author

@xlffm3 xlffm3 May 31, 2021

Choose a reason for hiding this comment

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

오토 포맷팅을 사용하다 보니 static한 메서드는 상위로 이동하고, private한 헬퍼 메서드들은 하단에 위치해서 통일성 있게 전부 상단으로 옮겨놨었는데요. 말씀하신대로 테스트 메서드들이 상단에 위치하는게 더 가독성이 좋은 것 같습니다 👍👍

fa27db8

return RestAssured
.given().log().all()
.auth().oauth2(tokenResponse.getAccessToken())
public static ExtractableResponse<Response> getMyMemberInfo(String token) {
Copy link

Choose a reason for hiding this comment

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

여러 곳에서 사용된다면, 특정 클래스에 놓아 새로운 의존 관계가 생기도록 하는 것보다는 AcceptanceTest로 올리는 게 어떨까요?

Copy link
Author

@xlffm3 xlffm3 May 31, 2021

Choose a reason for hiding this comment

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

네, 특히 Member 인수 테스트 뿐만 아니라 지하철 관련 인수테스트에서도
각 테스트 클래스들끼리 static 메서드를 공유해서 사용하고 있는데요.
이들 또한 상위 클래스로 옮기는게 적절해보일듯합니다. :)

08f362b

Comment on lines 45 to 51
@Nested
@DisplayName("calculateFare 메서드는")
class Describe_calculateFare {

@Nested
@DisplayName("10km 이하일 때는")
class Context_under10km {
Copy link

Choose a reason for hiding this comment

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

와아아아 💯👍🏆

docs/readme.md Outdated

## 로그인

- [ ] 경로조회 관련 로그인 인수테스트 작성
Copy link

Choose a reason for hiding this comment

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

완료되었으니 체크!

@xlffm3
Copy link
Author

xlffm3 commented May 31, 2021

안녕하세요 코니 :)
피드백 반영하면서 궁금한 점들이 많았는데~ DM 보내려다가 리뷰 요청으로 대신합니다!
부족한 부분 피드백 부탁드려요 ^_^~


public static void assertResponseMessage(ExtractableResponse<Response> response, String message) {
assertThat(response.body().asString()).isEqualTo(message);
}

@BeforeEach
Copy link

Choose a reason for hiding this comment

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

여기도 순서를 재배치하는 게 좋겠네요!

public class AuthAcceptanceTest extends AcceptanceTest {
private static final String EMAIL = "email@email.com";
private static final String PASSWORD = "password";
private static final Integer AGE = 20;

@DisplayName("Bearer Auth")
private ObjectMapper objectMapper = new ObjectMapper();
Copy link

Choose a reason for hiding this comment

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

ObjectMapper는 테스트하다 보면 꽤나 흔하게 쓰이는데, 보통 하나만 만들어 두고 테스트 전체가 공유하도록 해요. 여기서는 AcceptanceTest에 위치시킬 수 있겠네요.

Comment on lines 31 to 38
public int calculateLineFare() {
return sectionEdges.stream()
.map(SectionEdge::getLine)
.distinct()
.mapToInt(Line::getExtraFare)
.max()
.orElseThrow(() -> new InvalidValueException(InvalidValueExceptionStatus.INVALID_PATH));
}
Copy link

Choose a reason for hiding this comment

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

이 메서드가 SubwayPath에 위치하는 게 조금 어색한 느낌이 들어요. 책임이 잘 분리되어 있나요?

Copy link
Author

@xlffm3 xlffm3 Jun 3, 2021

Choose a reason for hiding this comment

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

최단거리 조회시 거치는 노선들의 추가요금들 중 최대값을 찾아 요금 정산에 사용하는데요.

따라서 경로에 포함된 노선 정보를 가진 SubwayPath가 해당 메서드를 가지는 것이 가장 적합하지 않을까 생각합니다.
다만 여러번 getter를 호출하는 것은 불필요하다 생각되어 변경했습니다. :) 조금 더 개선점이 있다면 피드백부탁드려요!!

Copy link

Choose a reason for hiding this comment

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

앗, 그러네요! 제가 이 부분은 잘못 보고 남긴 것 같아요. 그래도 개선해 주셔서 훨씬 좋아졌네요 👍

Comment on lines 47 to 56
private Fare generateFare(LoginMember loginMember, SubwayPath subwayPath) {
int distance = subwayPath.calculateDistance();
int lineExtraFare = subwayPath.calculateLineFare();
FareStrategy fareStrategy = DistanceFareStrategy.find(distance);
if (Objects.isNull(loginMember)) {
return new Fare(distance, lineExtraFare, fareStrategy);
}
DiscountStrategy discountStrategy = AgeDiscountStrategy.find(loginMember.getAge());
return new Fare(distance, lineExtraFare, fareStrategy, discountStrategy);
}
Copy link

@choihz choihz Jun 2, 2021

Choose a reason for hiding this comment

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

요금 계산의 흐름이 조금 파편화(?)되어있다는 느낌이 드네요. LoginMember의 존재 유무에 따라 분기가 생기는 것도 개선의 여지가 있어 보이고요.

Copy link
Author

Choose a reason for hiding this comment

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

ArgumentResolver에서 LoginMember or null을 반환하는 로직때문에 그런것같습니다.

User
   ㄴ Anonymous
   ㄴ Login

ArgumentResolver가 비회원인 경우 User 인터페이스를 구현한 AnonymousUser를 반환하게 함으로써 if 조건을 없앴어요.

public class AnonymousUser implements User {

    @Override
    public int getAge() {
        return AgeDiscountStrategy.ADULT_AGE;
    }
}

익명 회원은 가격 할인이 없기 때문에 할인이 없는 어른 나이를 참조하게 했습니다.


import static org.assertj.core.api.Assertions.assertThat;

class SubwayPathTest {
Copy link

Choose a reason for hiding this comment

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

테스트를 꼼꼼하게 잘 작성해 주셨지만, 요금 계산에 관한 테스트가 모두 SubwayPathTest에 위치한다는 게 조금 의문을 일으키기도 해요. 요금 계산의 책임을 온전하게 Fare가 가지게 해볼 수 있을까요? 이렇게 되면 테스트도 역할에 따라 잘 분리될 수 있을 것으로 보이네요.

Copy link
Author

Choose a reason for hiding this comment

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

네 :) 정신없이 작성하다보니 꼼꼼하지 못했네요. SubwayPath, Fare, PathService를 각각 분리했습니다.

Comment on lines 3 to 8
public class WrongEmailException extends AuthorizationException {
private static final String MESSAGE = "잘못된 이메일입니다.";

public WrongEmailException() {
super(MESSAGE);
}
Copy link

Choose a reason for hiding this comment

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

지금은 학습하는 단계이기 때문에 여러 가지 방법을 시도해 보고 각각의 장단점을 느껴보시는 것도 좋다는 생각이 들어 남긴 코멘트였어요. 사실 저는 거의 모든 상황마다 예외를 만들고 있긴 합니다 🙄 다만, 예외를 한 곳에 모으기보다 사용하는 쪽에 배치하고 있어요. 이렇게 할 때의 장단점은 또 뭐가 있을지 고민해 보시면 좋을 것 같네요.

Copy link

@choihz choihz left a comment

Choose a reason for hiding this comment

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

안녕하세요, 케빈!

피드백을 잘 반영해 주셨네요 👍 조금 더 개선해 볼 수 있을 것 같아 코멘트 몇 개 더 남겼어요. 궁금한 점 또는 의견이 있으시다면 언제든 코멘트 또는 DM 주세요~

@xlffm3
Copy link
Author

xlffm3 commented Jun 3, 2021

안녕하세요 코니!
남겨주신 피드백에 대해 많은 고민을 했는데 쉽사리 결론을 내리지 못하겠네요 :) 부족한 부분에 대해서 피드백 남겨주시면 감사하겠습니다~

아울러 다만, 예외를 한 곳에 모으기보다 사용하는 쪽에 배치하고 있어요. 이렇게 할 때의 장단점은 또 뭐가 있을지 고민해 보시면 좋을 것 같네요. 피드백에 대해서 학습해보았는데요

  • 패키지는 한 개의 기능 단위를 의미하고, 커스텀 예외 또한 한 개의 기능 단위의 일부분이다. 따라서 예의를 발생시키는 곳(도메인 등) 같은 패키지에 속해 있어야 한다.
  • 오히려 예외를 별도의 패키지로 그룹화하면 불필요한 내부 패키지 의존성을 유발한다.

사용하는 쪽에 예외를 배치시킨다면 기능 단위를 명확하게 이해할 수 있을것같습니다. 현재는 GlobalExceptionHandler를 사용하며 하나의 exception 패키지에 예외들을 위치시키는데, 해당 예외가 어플리케이션의 어느 위치 어느 문맥에서 발생하는지 알기 어렵다는 문제가 있을것같네요. :)

그리고 swagger를 적용해봤는데요.

https://jipark.p-e.kr/swagger-ui/

Postman처럼 실제 요청 응답이 오는지 테스트해보기 위해 Authorize버튼에 Bearer ${credential} 형태로 value를 입력하도록 설정을 수정했습니다.


@Override
public int getAge() {
return AgeDiscountStrategy.ADULT_AGE;
Copy link

Choose a reason for hiding this comment

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

ADULT_AGE 상수를 AgeDiscountStrategy에서 끌어 와야만 할까요? 이 상수가 사용되는 위치는 여기뿐인데 그럼 여기에 위치하는 게 맞지 않을까요? AnonymousUserAgeDiscountStrategy에 의존한다는 측면에서도 그렇고요.

@@ -0,0 +1,6 @@
package wooteco.subway.member.domain;

public interface User {
Copy link

Choose a reason for hiding this comment

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

👍

Comment on lines +41 to +45
int distance = subwayPath.calculateDistance();
int age = user.getAge();
FareStrategy fareStrategy = DistanceFareStrategy.find(distance);
DiscountStrategy discountStrategy = AgeDiscountStrategy.find(age);
Fare fare = new Fare(subwayPath, fareStrategy, discountStrategy);
Copy link

Choose a reason for hiding this comment

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

전략을 결정하고 Fare 객체를 만드는 일도 별도 객체로 분리할 만한 것 같아요. 이렇게 분리되면 지금의 FareTest에 있는, 매 테스트마다 전략을 찾아 준비하는 작업 없이도 모든 경우의 테스트를 더 간단하게 수행할 수 있지 않을까요?

Comment on lines 31 to 38
public int calculateLineFare() {
return sectionEdges.stream()
.map(SectionEdge::getLine)
.distinct()
.mapToInt(Line::getExtraFare)
.max()
.orElseThrow(() -> new InvalidValueException(InvalidValueExceptionStatus.INVALID_PATH));
}
Copy link

Choose a reason for hiding this comment

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

앗, 그러네요! 제가 이 부분은 잘못 보고 남긴 것 같아요. 그래도 개선해 주셔서 훨씬 좋아졌네요 👍

Copy link

@choihz choihz left a comment

Choose a reason for hiding this comment

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

안녕하세요, 케빈!

피드백을 아주 잘 반영해 주셨네요 💯 협업 미션은 여기서 마무리하도록 할게요. 레벨 2까지 그동안 정말 고생 많으셨어요. 언젠가 다시 만나요~ 👋

@choihz choihz merged commit 10b71c5 into woowacourse:xlffm3 Jun 5, 2021
@xlffm3
Copy link
Author

xlffm3 commented Jun 21, 2021

학습로그

빌드와 배포, CI, CD

태그

  • Devops

Servlet, Spring의 역사 및 관계

태그

  • Spring

Spring MVC와 Dispatcher Servlet

태그

  • Spring, MVC

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.

3 participants