[김대겸] Step2 PR #854
[김대겸] Step2 PR #854
Conversation
| ## 🚀 2단계 - 지하철 노선 기능 | ||
|
|
||
| ### 기능 요구사항 | ||
| - [X] 지하철 노선 생성 | ||
| - [X] 인수 테스트 작성 | ||
| - [X] 인수 테스트를 충족하는 기능 구현 | ||
| - [X] 지하철 노선 목록 조회 | ||
| - [X] 인수 테스트 작성 | ||
| - [X] 인수 테스트를 충족하는 기능 구현 | ||
| - [X] 지하철 노선 조회 | ||
| - [X] 인수 테스트 작성 | ||
| - [X] 인수 테스트를 충족하는 기능 구현 | ||
| - [X] 지하철 노선 수정 | ||
| - [X] 인수 테스트 작성 | ||
| - [X] 인수 테스트를 충족하는 기능 구현 | ||
| - [X] 지하철 노선 삭제 | ||
| - [X] 인수 테스트 작성 | ||
| - [X] 인수 테스트를 충족하는 기능 구현 |
| @Service | ||
| public class DatabaseCleanup implements InitializingBean { | ||
| @Autowired |
There was a problem hiding this comment.
데이터를 초기화하기 위해 database cleanup 을 직접 구현해주셨네요 👍👍👍
|
|
||
| public LineResponse findLine(final Long id) { | ||
| Line persistLine = lineRepository.findById(id) | ||
| .orElseThrow(() -> new IllegalArgumentException(id + "번 노선을 찾을 수 없습니다.")); |
| public void modify(final LineRequest lineRequest) { | ||
| this.name = lineRequest.getName(); | ||
| this.color = lineRequest.getColor(); | ||
| } |
There was a problem hiding this comment.
도메인이 controller 에서 사용하고 있는 dto 까지 의존성이 생긴 것 같아요 🤔
결국 도메인과 dto가 양방향 의존성을 가진 상태 같은데요.
개인적으로 코드 유지보수를 위해 의존성의 방향은 안쪽으로만 향해야 한다고 생각합니다.
혹시 이 의존성을 제거하는 방향에 대해서 어떻게 생각하시나요??🤔🤔
제가 고민해보고 참고한 자료 링크 남겨드리도록 하겠습니다!
클린 아키텍처: 의존성 역전하기
There was a problem hiding this comment.
실무에서 많이 고민하는 부분이기도 한데요,, 변경되는 필드들이 너무 많아져서 이렇게 자주사용하고 있었는데 말씀해주신것처럼 양방향 의존성을 가지게되네요...
링크 주신 것 참고해서 고민해보고 수정하도록 하겠습니다 ㅎㅎㅎ
감사합니다!!
| return LineResponse.of(lineRepository.save(persistLine)); | ||
| } |
There was a problem hiding this comment.
영속화된 엔티티는 트랜잭션이 종료되기전에 더티 체킹이 일어나면서 데이터가 업데이트가 이뤄지게 되는데요.
save 호출해도 업데이트가 이뤄지기 때문에 문제되진 않겠지만
불필요한 로직이 수행될 수 있기 때문에 비효율적이지 않을까 라고 생각되는데
혹시 이부분에 대해서 어떻게 생각하시나요??🤔🤔
| @Override | ||
| List<Line> findAll(); |
There was a problem hiding this comment.
혹시 기존에 존재하는 메서드를 오버라이드 하신 이유가 있으실까요?
| ExtractableResponse<Response> response = 지하철노선_생성_요청("1호선", "indigo darken-2"); | ||
|
|
||
| // then | ||
| 지하철노선_생성_응답상태_201_검증(response); |
| public static void 지하철노선_목록_조회_응답상태_200_검증(ExtractableResponse<Response> response) { | ||
| assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); | ||
| } | ||
|
|
||
| public static void 지하철노선_조회_응답상태_200_검증(ExtractableResponse<Response> response) { | ||
| assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); | ||
| } | ||
|
|
||
| public static void 지하철노선_수정_응답상태_200_검증(ExtractableResponse<Response> response) { | ||
| assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); | ||
| } |
There was a problem hiding this comment.
응답 상태가 모두 동일하다면 코드 중복 제거를 위해 한 메서드로 재사용해보시는 방향은 어떻게 생각하시나요??🤔🤔🤔
테스트가 증가 될수록 같은 메서드가 계속 추가될 것 같아서요 😅
안녕하세요 리뷰어님,
2단계 - 지하철 노선 기능 PR 입니다.
이번에도 잘 부탁드립니다 :)