[Spring 경로 조회 - 1단계] 써머(최혜원) 미션 제출합니다. #242
Merged
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
* refactor: remove frontend code * refactor: flatten project structure * docs: remove frontend guide * refactor: remove unusable resources * chore: Update versions
* feat: add cors configuration * refactor: add generic type to ResponseEntity
* feat: 존재하는 이름 입력 시 예외 발생 * feat: 예외 처리를 위한 ExceptionHandler 추가 * feat: 노선을 나타내는 Line 및 데이터 관리를 위한 LineDao 추가 * feat: 노선 생성 기능 추가 * feat: 노선 중복에 대한 검증 추가 * feat: 노선 조회를 위한 기능 추가 * feat: 단일 노선 조회 기능 추가 * feat: 노선 수정 기능 추가 * feat: 노선 삭제 기능 추가 * feat: 2단계 준비를 위한 세팅 추가 * feat: JDBC를 활용한 지하철역 저장 기능 추가 * test: 중복 지하철역 이름에 대한 예외 테스트 추가 * feat: JDBC를 이용한 모든 지하철역 조회 기능 추가 * refactor: StationController 기존 Dao 의존성 변경 * feat: JDBC를 활용하여 지하철역 삭제 기능 추가 * feat: JDBC를 활용하여 노선 저장 기능 추가 * test: 노선 중복 예외 검증 테스트 추가 * test: 존재하지 않는 노선 조회시 예외 검증을 위한 테스트 추가 * feat: JDBC를 활용하여 노선 수정 기능 추가 * feat: JDBC를 활용하여 노선 전체 조회 및 단일 삭제 기능 추가 * refactor: LineController 의존성 변경 * feat: 예외 처리 로직 분리 * refactor: 인수 테스트를 위한 schema 수정 * refactor: 불 필요한 메서드 제거 * style: 쓰이지 않는 import 문 제거 * refactor: 컨트롤러 @RequestMapping을 이용한 공통 매핑 value 단축 * feat: 500 상태 코드애 대한 에러 메세지 추가 * refactor: 테스트 코드에서만 사용하는 메서드를 운영 코드에서 제거 * feat: LineService 추가 * feat: StationService 추가 * refactor : 테스트 코드 변수 명 변경 및 코드 단축 Co-authored-by: hyeonic <dev.hyeonic@gmail.com>
toneyparky
pushed a commit
that referenced
this pull request
May 19, 2022
* docs(README): 요구사항 작성 * feat(StationController): 중복된 지하철 이름 사용시 BAD REQUEST 반환하는 기능 구현 * feat: 지하철역 이름에 빈 문자열 사용시 Bad Request 반환 * feat: 존재하지 않는 지하철 역 삭제 기능 구현 * style(StationService): 마지막줄 공백 추가 (`EOF`) * refactor(StationService): deleteById 메서드 구조 개선 * refactor(StationController): `NotExistStationException` 반환 코드 변경 (`400` -> `404`) * refactor: Request, Response 클래스 정리 * test(StationAcceptanceTest): `requestCreateStation` 텍스트 픽스쳐 추출 * refactor(StationAcceptanceTest): 지하철 역 생성 시 requestCreateStation 사용 * style: 자바 컨벤션 적용 * feat: 빈 이름의 노선 생성이 Bad Request를 반환하는 컨트롤러 생성 * test: 지하철 노선 생성을 위한 테스트 작성 * feat: 지하철 노선 저장 기능 구현 * feat: 지하철 노선 저장 요청 시 201 및 body 반환 * feat: 지하철 노선 중복 등록 방지하는 기능 구현 * feat: 지하철 노선 조회 기능 구현 * feat: 존재하지 않는 지하철 노선 조회시 404 반환 * feat: 지하철 노선 목록 조회 * feat: 이름이 공백인 지하철 노선을 수정할 수 없는 기능을 구현 * feat: 지하철 노선 정보 수정 기능 구현 * feat: 지하철 중복 노선 수정을 허용하지 않는 기능 구현 * fix: 기존과 동일한 지하철 노선 이름으로 수정은 가능 * test: 존재하지 않는 지하철 노선 수정 시 테스트 추가 * feat: 지하철 노선 제거 및 존재하지 않는 노선 제거 구현 * chore: h2 db 세팅 * refactor: 서비스 계층 스프링 DI 적용 * refactor: LineDao JdbcTemplate 적용 * refactor: StationDao JdbcTemplate 적용 * test: `@DirtiesContext` 제거 및 `@Transactional` 적용 * style: 자바 컨벤션 적용 Co-authored-by: progress0407 <progress0407@gmail.com>
TheDevLuffy
approved these changes
May 20, 2022
| import org.jgrapht.alg.shortestpath.DijkstraShortestPath; | ||
| import org.jgrapht.graph.DefaultWeightedEdge; | ||
| import org.jgrapht.graph.WeightedMultigraph; | ||
| import org.jgrapht.graph.WeightedPseudograph; | ||
|
|
||
| public class Sections { |
There was a problem hiding this comment.
기존 코드를 변경하신 부분이 좀 있는데요. 이 변경을 없앨 수 있지 않을까요?
OCP 개방 폐쇄 원칙에 대해 학습해보시면 도움이 될 것 같아요
There was a problem hiding this comment.
처음 코멘트를 읽었을 때 그냥 메서드를 추가하는 건 확장에 속한다고 생각해서 좀 고민해 봤는데요,
현재 변경이 OCP를 지키지 못한 이유가 만약 최적 경로 찾기 로직에 변경이 생겼을 때 메서드 자체를 수정해야 해서 그런 걸까요?
sections가 경로를 찾는 어떤 클래스를 주입받는다면 OCP에서 말하는 적절한 확장을 할 수 있을까요?
There was a problem hiding this comment.
제가 해당 코멘트를 남긴 이유는 추가해주신 메서드가 Sections 를 확장하기 위함이 아닌 Sections 에 단순히 메서드를 분류해둔 것으로 이해했습니다. findShortestPath 에서 프로퍼티인 sections 을 사용하는 곳은 addAllSectionsAsEdge 여기인데요. 이 부분에 메시지를 보내서 최단경로를 구하는 역할을 위임하는 방향으로 확장해도 되지 않을까 생각을 했습니다 ㅎㅎ
제가 지하철 노선도 도메인에 대해 고민한 시간보다 써머가 고민한 시간이 많으니 저보다 써머가 더 잘 구현해주셨을수도 있습니다 ! ㅎㅎ 피드백은 정답이 아니니 참고만 해주셔도 될 것 같아요~
| import org.jgrapht.GraphPath; | ||
| import org.jgrapht.graph.DefaultWeightedEdge; | ||
|
|
||
| public class Path { |
|
|
||
| public class Path { | ||
|
|
||
| public static final int BASIC_FARE = 1250; |
| .then().log().all() | ||
| .extract(); | ||
|
|
||
| assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); |
There was a problem hiding this comment.
RestAssured 는 BDD 스타일 기반으로 많은 검증 메서드를 지원하고 있어요.
extract 하지 않아도 괜찮지 않을까요 ㅎㅎ hamcrest matchers 를 참고해봐도 좋을 것 같아요.
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.
안녕하세요, 루피! 이번 미션 리뷰 매칭 된 써머라고 합니다 🙇
지난 미션에서 개발한 도메인이 이번 미션에 필요한 걸 거의 갖추고 있어서 추가된 게 생각보다 적네요
이번 미션 기간 동안 잘부탁드려요!!