Step2 경로 조회 기능 리뷰 요청드립니다. #739
Conversation
chae-yh
left a comment
chae-yh
left a comment
There was a problem hiding this comment.
정현님 안녕하세요! step1의 피드백 잘 반영해주셨고, step2의 코드도 전체적으로 잘 작성해주셨습니다 👍
다만 조금 더 고민해주시면 좋을만한 내용이 있어 코멘트 드리니 확인 부탁드립니다!
감사합니다 : )
| import org.springframework.web.bind.annotation.ExceptionHandler; | ||
| import org.springframework.web.bind.annotation.RestControllerAdvice; | ||
|
|
||
| @RestControllerAdvice |
There was a problem hiding this comment.
@RestControllerAdvice 를 사용하여 모든 에러를 한군데서 처리해줄 수 있게해준 부분 좋습니다 👍
| @RestControllerAdvice | ||
| public class ControllerExceptionHandler { | ||
|
|
||
| @ExceptionHandler(DataIntegrityViolationException.class) |
There was a problem hiding this comment.
이 부분은 조금 궁금하여 여쭤봅니다! DataIntegrityViolationException 는 현재 어플리케이션 내에서 언제, 어떤 케이스에서 발생하게 되나요...?
|
|
||
| public class Lines { | ||
|
|
||
| List<Line> lines = new ArrayList<>(); |
| this.lines = lines; | ||
| } | ||
|
|
||
| public GraphPath minPath(Station sourceStation, Station targetStation) { |
There was a problem hiding this comment.
동사구로 메서드를 표현해보는 것은 어떨까요...? 그리고 혹시 제너릭을 안쓰시는 이유가 있으실까요...?
| private boolean stationExistsAtEnd(Section section) { | ||
| return sections.stream().anyMatch( | ||
| sectionIterate -> sectionIterate.getDownStation().equals(section.getUpStation()) | ||
| || sectionIterate.getUpStation().equals(section.getDownStation())); |
There was a problem hiding this comment.
이부분도 Section에 메시지를 던져서 확인해볼 수 있을 것 같습니다
|
|
||
| private void connectFrontBackSection(Line line, Optional<Section> upLineStation, Optional<Section> downLineStation) { | ||
| private void connectFrontBackSection(Line line, Optional<Section> upLineStation, | ||
| Optional<Section> downLineStation) { |
| } | ||
|
|
||
| public PathResponse minPath(Long sourceStationId, Long targetStationId) { | ||
| System.out.println("PathService.minPath"); |
There was a problem hiding this comment.
여기에 혹시 System 함수를 쓰신 이유가 있을까요...? Sysytem 함수는 리소스를 많이 먹어서 최대한 사용하지 않는 것이 좋습니다.
| public PathResponse minPath(Long sourceStationId, Long targetStationId) { | ||
| System.out.println("PathService.minPath"); | ||
|
|
||
| Station sourceStation = stationRepository.findById(sourceStationId) |
There was a problem hiding this comment.
Repository 조회하는 부분이 있는데 readOnly 옵션을 주는 것은 어떨까요....?
|
|
||
|
|
||
| @DisplayName("지하철 경로 조회") | ||
| public class PathAcceptanceTest extends AcceptanceTest { |
There was a problem hiding this comment.
인수테스트 좋습니다 👍 다만 이번에 새로만드신 Lines에 대한 단위 테스트, PathService에 대한 단위테스트도 추가되면 좋을 것 같습니다!
Lines의 경우 실제 외부 라이브러리를 사용하므로 해당 실제 객체를 이용하여 테스트를 작성하고, PathService의 경우는 외부 의존성을 Mocking 한 단위 테스트를 작성해보는 것도 좋은 공부가 될 것 같습니다!
|
|
||
| @Test | ||
| @DisplayName("경로 조회시 미존재 역 에러 확인") | ||
| public void findMinPathNotExsistStation() { |
There was a problem hiding this comment.
연결되어 있지 않은 역에 대한 최단거리 조회의 경우에도 테스트를 추가해보는 것은 어떨까요...?
2 participants
안녕하세요 step2 경로 조회 기능 리뷰 요청드립니다~ 감사합니다.