3단계 - 지하철 구간 관리 #284

DaehunGwak · 2022-01-30T21:36:23Z

모르는게 많아 오래걸렸지만
3단계 완료 지금되어 PR 드립니다!

- 테스팅용 DTO LineTestRequest 를 추가 - 위 클래스 관련된 LineTestStep, LineAcceptanceTest 부분 수정

junwoochoi

안녕하세요!
3단계 테스트 코드가 정말 깔끔하네요 👍 👍
몇가지 추가로 고민해볼만한 부분 관련해서 코멘트 달아보았는데 확인해주시기 바랍니다!
즐거운 설명절되세요!!

junwoochoi · 2022-01-31T09:02:13Z

src/main/java/nextstep/subway/applicaion/LineService.java

+            Section section = new Section(upStation, downStation, request.getDistance());
+            line.addSection(section);


Section의 생성자의 책임을 Line객체 내부로 옮겨보면 어떨까요?

확실히 Line에 의존적이니 해당 책임을 Line 으로 위임하는 것이 더 바람직하겠네요 수정하겠습니다 :)

junwoochoi · 2022-01-31T09:04:14Z

src/main/java/nextstep/subway/applicaion/LineService.java

+            downStation = stationRepository.findById(downStationId)
+                    .orElseThrow(() -> new IllegalArgumentException(
+                            "노선의 구간 초기화 중 하행선역을 찾을 수 없습니다. downStationId:" + downStationId));


중복되는 코드를 줄여볼 수 있을까요?

아래 코멘트와 같은 선에서 LineService에서 전반적으로 private메서도르 줄여볼 수 있겠네요 감사합니다 :)

junwoochoi · 2022-01-31T09:06:34Z

src/main/java/nextstep/subway/applicaion/SectionService.java

+
+@Service
+@Transactional
+public class SectionService {


Section객체는 Line과 같은 라이프사이클을 공유하고 있어서, LineService에서 모두 관리하면 중복 코드가 많이 제거될 수 있지 않을까요?

junwoochoi · 2022-01-31T09:07:33Z

src/main/java/nextstep/subway/domain/Line.java

+    @OneToMany(mappedBy = "line", fetch = FetchType.LAZY, cascade = {CascadeType.PERSIST, CascadeType.MERGE}, orphanRemoval = true)
+    private List<Section> sections = new ArrayList<>();


@Embedded 혹은 @Embeddable 어노테이션을 활용해서 응집도 높은 객체를 만들어보면 어떨까요? 👀

JPA 에서 일급컬렉션같이 활용하는 부분을 고민했었는데 임베딩할 수가 있었군요!
공유 감사합니다 :) 해당 방향으로 수정해보겠습니다.

junwoochoi · 2022-01-31T09:09:03Z

src/main/java/nextstep/subway/domain/Section.java

+    @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST)
+    private Line line;
+    @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST)
+    private Station upStation;
+    @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST)
+    private Station downStation;


cascade의 범위를 잘 고민해보면 더 좋을 것 같아요!

CascadeType에 대해 더 공부를 해봐야겠네요!
공유 감사합니다 :)

junwoochoi · 2022-01-31T09:10:04Z

src/test/java/nextstep/subway/acceptance/LineAcceptanceTest.java

+        LineTestRequest lineTestRequest = LineTestStep.지하철_노선_요청_신분당선_데이터_생성하기();

        // when
-        ExtractableResponse<Response> response = LineTestStep.지하철_노선을_생성한다(color, name);
+        ExtractableResponse<Response> response = LineTestStep.지하철_노선을_생성한다(lineTestRequest);

        // then
-        assertThat(response.statusCode()).isEqualTo(HttpStatus.CREATED.value());
-        assertThat(response.header("Location")).isNotBlank();
-        Integer responseIntegerId = response.jsonPath().get("id");
-        Long createdId = responseIntegerId.longValue();
-        assertThat(createdId).isGreaterThan(0L);
+        LineTestStep.지하철_노선_생성_성공_검증하기(response);


given when then 스탭이 간결하고, 한눈에 들어오네요! 💯

junwoochoi · 2022-01-31T09:11:09Z

src/test/java/nextstep/subway/acceptance/SectionAcceptanceTest.java

+    @Test
+    void createSection() {
+        // given
+        LineTestRequest lineRequest = LineTestStep.지하철_노선_요청_신분당선_데이터_생성하기();


클래스내 메소드들이 반복적인 샘플케이스를 만드는 경우 field 변수를 만들고 beforeEach에서 할당해주는 것도 하나의 방법입니다 :)

next-step#284 (comment)

- 영속성 전이: 부모의 연관관계를 가지 자식을 함께 저장하고 싶을 때 사용 - 내가 사용하는 Section은 이미 Line을 주체로 작성 된 상태 - 따라서, Section에서는 자식 엔티티들을 관리하고 있는 상태가 아니므로 CascadeType.PERSIST를 사용할 필요가 없음

junwoochoi

안녕하세요! 1주차 미션 모두 머지 하도록 하겠습니다!
다음 주차도 화이팅 입니다!! 🚀

junwoochoi · 2022-02-02T23:47:25Z

src/main/java/nextstep/subway/applicaion/LineService.java

@@ -30,8 +30,7 @@ public LineResponse saveLine(LineRequest request) {

        Line line = new Line(request.getName(), request.getColor());
        if (Objects.nonNull(upStation) && Objects.nonNull(downStation)) {
-            Section section = new Section(upStation, downStation, request.getDistance());
-            line.addSection(section);
+            line.addSection(upStation, downStation, request.getDistance());


junwoochoi · 2022-02-02T23:50:35Z

src/test/java/nextstep/subway/acceptance/SectionAcceptanceTest.java

+        super.setUp();
+        lineRequest = LineTestStep.지하철_노선_요청_신분당선_데이터_생성하기();
+        lineId = LineTestStep.지하철_노선_생성한_후_아이디_추출하기(lineRequest);


junwoochoi · 2022-02-02T23:51:22Z

src/main/java/nextstep/subway/domain/Sections.java

+@Embeddable
+public class Sections {


Sections 내부에 도메인 로직이 응집되서 유지보수가 더 용이해졌을 것 같네요 👍

junwoochoi · 2022-02-02T23:51:33Z

src/main/java/nextstep/subway/domain/Sections.java

+
+@Embeddable
+public class Sections {
+    public static final int MIN_SECTION_SIZE = 1;


매직넘버 분리 💯

junwoochoi · 2022-02-02T23:52:36Z

src/main/java/nextstep/subway/applicaion/LineService.java

+        Station upStation = findStationById(request.getUpStationId());
+        Station downStation = findStationById(request.getDownStationId());
+
+        Section addedSection = line.addSection(upStation, downStation, request.getDistance());


현재는 section을 추가하는 책임과 해당 section을 리턴하는 책임 모두 가지고 있어서, 두 메소드로 분리해서 호출해 사용하면 더 가독성이 좋을 것 같습니다 :)

DaehunGwak added 13 commits January 27, 2022 01:14

refactor: 테스트 검증 스텝 분리 후 리팩터링

077170d

next-step#235 (comment)

refactor: 지하철역_생성_후_아이디_추출하기 스텝 추가 및 리팩터링

fbb1ad4

feat: 지하철 노선 생성 시 필요한 인자 추가하기

a8d232c

- 테스팅용 DTO LineTestRequest 를 추가 - 위 클래스 관련된 LineTestStep, LineAcceptanceTest 부분 수정

refactor: test step 패키지 변경

ff93404

feat: 지하철 구간 등록 기능 성공 시나리오 추가

8dd0d4f

fix: 노선 생성 시 구간 초기화 안되어 수정

0625f32

fix: 지하노선 삭제 시 Section 기본 생성자 없어 실패 문제 수정

2e17426

refactor: Station 엔티티 불필요한 노선 필드 삭제

e0cb80d

feat: 구간 등록 시 상행선역이 노선의 하행 종점역이 아닐 때 실패 기능 추가

e70b4c5

feat: 구간 등록 시 하행역이 해당 노선에 등록되어 있을 때 실패 기능 추가

ab0aafb

feat: 구간 제거 기능 추가

b5614c8

refactor: 구간 삭제 시 orphanRemoval 활용하도록 리팩터링

97dc9a6

feat: 등록된 구간을 통해 역 목록 조회 기능 추가

ad15266

junwoochoi requested changes Jan 31, 2022

View reviewed changes

DaehunGwak added 6 commits February 2, 2022 12:27

refactor: Section 생성 책임을 Line 에 위임

b9038b8

next-step#284 (comment)

refactor: SectionService 메서드를 LineService 메서드로 이동

5348df9

next-step#284 (comment)

refactor: LineService 내 중복되는 조회 메서드 추출

1667215

next-step#284 (comment)

refactor: Section 인수테스트 공통 부분 BeforeEach 로 추출

5c6fd7e

next-step#284 (comment)

refactor: Sections 추출 및 리팩터링

8e36e80

next-step#284 (comment)

junwoochoi approved these changes Feb 2, 2022

View reviewed changes

junwoochoi merged commit b3e4da7 into next-step:daehungwak Feb 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3단계 - 지하철 구간 관리 #284

3단계 - 지하철 구간 관리 #284

DaehunGwak commented Jan 30, 2022

junwoochoi left a comment

junwoochoi Jan 31, 2022

DaehunGwak Jan 31, 2022

junwoochoi Jan 31, 2022

DaehunGwak Jan 31, 2022

junwoochoi Jan 31, 2022

junwoochoi Jan 31, 2022

DaehunGwak Jan 31, 2022

junwoochoi Jan 31, 2022

DaehunGwak Jan 31, 2022

junwoochoi Jan 31, 2022

junwoochoi Jan 31, 2022

junwoochoi left a comment

junwoochoi Feb 2, 2022

junwoochoi Feb 2, 2022

junwoochoi Feb 2, 2022

junwoochoi Feb 2, 2022

junwoochoi Feb 2, 2022

		Section section = new Section(upStation, downStation, request.getDistance());
		line.addSection(section);

		@OneToMany(mappedBy = "line", fetch = FetchType.LAZY, cascade = {CascadeType.PERSIST, CascadeType.MERGE}, orphanRemoval = true)
		private List<Section> sections = new ArrayList<>();

3단계 - 지하철 구간 관리 #284

3단계 - 지하철 구간 관리 #284

Conversation

DaehunGwak commented Jan 30, 2022

junwoochoi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junwoochoi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment