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

Step3 경로조회 기능 #629

Open
wants to merge 3 commits into
base: owen-q
Choose a base branch
from
Open

Step3 경로조회 기능 #629

wants to merge 3 commits into from

Conversation

owen-q
Copy link

@owen-q owen-q commented Jul 22, 2023

step3 진행하였습니다~!

@hiblue02
Copy link

  1. 지난 pr에서 요청드린 내용 반영 부탁드려요! 미션 2: 지하철 구간 제거 기능 개선  #595 (review)
  2. Sections의 possibleToAddSection, possibleToDeleteSection if문이 많은데요. 요구사항이 추가되면 계속 해서 if문이 추가될 것 같아요. 다른 구조로 개선해볼 수 있을 것 같아요! 😄

Copy link

@hiblue02 hiblue02 left a comment

Choose a reason for hiding this comment

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

3단계 미션도 빠르게 진행해주셨네요! 👍
코멘트 확인해주시고, 다시 리뷰 요청 주세요!

Comment on lines +85 to +89
Assertions.assertThat(shortestPath.getDistance()).isEqualTo(16);
Assertions.assertThat(shortestPath.getStations().get(0)).isEqualTo(stationD);
Assertions.assertThat(shortestPath.getStations().get(1)).isEqualTo(stationA);
Assertions.assertThat(shortestPath.getStations().get(2)).isEqualTo(stationB);
Assertions.assertThat(shortestPath.getStations().get(3)).isEqualTo(stationC);

Choose a reason for hiding this comment

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

  1. assertThat 앞에 Assertions가 붙는데요, static import를 사용해보면 어떨까요?
  2. assertThat에서 리스트 내부 객체의 순서를 검증할 땐 containsExactly를 사용해면 어떨까요?

Comment on lines +121 to +127
void 오류가발생한다() {
ExtractableResponse<Response> response = RestAssured.given().log().all()
.when().get(getPathUrl(stationC2.getId(), stationC2.getId()))
.then().log().all().extract();

Assertions.assertThat(response.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value());
}

Choose a reason for hiding this comment

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

오류 케이스까지 꼼꼼히 작성해주셨네요! 💯

Comment on lines +6 to +13
@Configuration
public class ShortedPathFinderConfiguration {

@Bean
public ShortestPathFinder shortestPathFinder() {
return new DijkstraShortestPathFinder();
}
}

Choose a reason for hiding this comment

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

DI로 조립해주셨군요! 👍

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