Step4: 가장 빠른 도착 시간 조회 구현 #470
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seovalue
requested changes
Mar 7, 2024
|
|
||
| @SpringBootTest | ||
| @Transactional | ||
| public class ArrivalTimeCalculatorTest { |
There was a problem hiding this comment.
TDD로 구현해주신 것이 맞을까요? commit만 봤을 때는 유추하기가 어렵네요.
| assertEquals(arrivalTime, LocalTime.parse("10:09", DateTimeFormatter.ISO_LOCAL_TIME)); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
displayName 으로 어떤 테스트인지 설명이 붙으면 좋을 것 같아요.
There was a problem hiding this comment.
그리고 테스트코드도 가독성있게 주석을 다는 등 전반적으로 가독성을 챙겨보면 좋을 것 같아요~
| if (sourceId.equals(targetId)) { | ||
| throw new InvalidInputException("출발역과 도착역이 동일합니다."); | ||
| } | ||
| Station source = stationRepository.findById(sourceId).orElseThrow(EntityNotFoundException::new); | ||
| Station target = stationRepository.findById(targetId).orElseThrow(EntityNotFoundException::new); | ||
|
|
||
| SearchType searchType = SearchType.from(type); | ||
| Path path = searchType.findPath(pathFinder, source, target); | ||
| LocalTime departureTime = parseDepartureTime(searchType, time); |
There was a problem hiding this comment.
parseDepartureTime과 같은 로직을 메서드 상단에서 수행한다면 parse error가 났을 때 불필요하게 역을 찾는 쿼리를 발생시키지 않을 수 있을 것 같아요.
| if (sourceId.equals(targetId)) { | ||
| throw new InvalidInputException("출발역과 도착역이 동일합니다."); | ||
| } | ||
| Station source = stationRepository.findById(sourceId).orElseThrow(EntityNotFoundException::new); | ||
| Station target = stationRepository.findById(targetId).orElseThrow(EntityNotFoundException::new); | ||
|
|
||
| SearchType searchType = SearchType.from(type); | ||
| Path path = searchType.findPath(pathFinder, source, target); | ||
| LocalTime departureTime = parseDepartureTime(searchType, time); | ||
| Path path = searchType.findPath(pathFinder, source, target, departureTime); |
There was a problem hiding this comment.
searchType에서 path를 찾는 것에 대해서 어떻게 생각하시나요?
|
|
||
| ARRIVAL_TIME { | ||
| @Override | ||
| public Path findPath(PathFinder pathFinder, Station source, Station target, LocalTime departureTime) { |
There was a problem hiding this comment.
findPath 내부 체이닝된 stream에 대해서 가독성이 많이 떨어지는 것 같아요. 실제로도 정확히 어떤 로직을 수행하는지 단번에 이해하기가 어렵다고 느껴져요. 이 부분을 어떻게 개선할 수 있을까요?
| @@ -28,6 +28,28 @@ public class LineSteps { | |||
| .extract(); | |||
| } | |||
|
|
|||
| public static ExtractableResponse<Response> 노선_생성_요청2(String name, Long upstationId, Long downstationId, int distance, int duration, int extraFare, | |||
| @@ -28,6 +28,28 @@ public class LineSteps { | |||
| .extract(); | |||
| } | |||
|
|
|||
| public static ExtractableResponse<Response> 노선_생성_요청2(String name, Long upstationId, Long downstationId, int distance, int duration, int extraFare, | |||
| String firstDepartureTime, String lastDepartureTime, int intervalTime) { | |||
| Map<String, String> params = new HashMap<>(); | |||
There was a problem hiding this comment.
실제 request dto 객체가 아닌 map을 넘기시는 이유가 궁금합니다!
| public class ArrivalTimeCalculator { | ||
| private List<Section> path; | ||
|
|
||
| private LocalTime departureTime; |
| this.departureTime = departureTime; | ||
| } | ||
|
|
||
| public LocalTime calculate() { |
|
|
||
| Long lineShinbundangId; | ||
|
|
||
| private void setUp() { |
There was a problem hiding this comment.
beforeEach가 아닌 setup을 매번 호출해주신 이유가 궁금해요~
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요! 리팩토링을 어떻게 할까 고민을 많이 하다가 제 맘대로 요구사항을 약간 수정했습니다. 도착 시간을 조회하지 않는 경우(최단거리, 최소시간)에도 도착시간을 함께 리턴하도록 구현했습니다. 최단거리, 최소시간 조회할 때는 그냥 요청한 시간을 기준으로 한 도착시간을 명시하도록 했습니다.
새로운 요구사항을 구현하면서 기존 코드와 어떻게 통합할까 고민하다가 이런 식으로 구현했습니다. 더 나은 방법이 있을지 모르겠네요... 사소한 것까지 지적해주시면 감사하겠습니다!