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

jwoo-o · 2023-07-11T08:15:36Z

안녕하세요 step3 지하철노선 구간 추가,삭제 기능 pr올립니다.

line 도메인에 Sections이라는 일급컬렉션으로 도메인 기능을 분리하여 등록,삭제를 처리하였습니다.

testrace

안녕하세요 진우님 😃
Step 클래스를 분리하면서 인수 조건과 인수 테스트의 가독성이 좋아진 것 같아요 👍
관련해서 몇 가지 코멘트 남겨두었는데 확인해 주세요

testrace · 2023-07-12T04:50:51Z

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

+@Embeddable
+public class Sections {
+
+    @OneToMany(mappedBy = "line", orphanRemoval = true, cascade = CascadeType.ALL)
+    private List<Section> sections;


일급 컬렉션 활용 👍

testrace · 2023-07-12T04:53:35Z

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

+        List<Station> stations = sections.stream()
+                .map(Section::getUpStation)
+                .collect(Collectors.toList());
+        setLastStations(stations);


stream의 flatMap 메서드를 활용해 보셔도 좋을 것 같아요 😃

testrace · 2023-07-12T05:05:58Z

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

+    public void addSection(Section section) {
+        this.sections.addSection(section);
+    }


addSection 메서드를 수행하는 line 객체와
인자로 받은 Section 내부의 line 필드가 서로 다르면 어떻게 될까요?
인자로 받은 Section의 line을 재할당하면 좋을 것 같은데 진우님 생각은 어떠신가요?
section.setLine(this);

testrace · 2023-07-12T05:09:09Z

src/test/java/subway/line/LineAcceptanceTest.java

        //then
-        assertThat(lines).hasSize(2);
-        assertThat(lines).containsExactly(신분당선.get("name"), 분당선.get("name"));
+        LineStep.지하철노선을_확인(Arrays.asList(신분당선, 분당선));


메서드명을 한글로 작성해주셔서 가독성이 좋아진 것 같아요 👍
메서드명에서 사용된 용어들을 인수 조건과 통일하면 어떨까요?
지하철노선을_추가 -> 지하철_노선_생성
지하철노선을_확인 -> 지하철_노선_목록_조회

testrace · 2023-07-12T05:11:05Z

src/test/java/subway/line/LineAcceptanceTest.java

+    /**
+     * Given 지하철역 및 지하철 노선을 생성하고 지하철 구간을 추가하고
+     * When 새로운 구간을 연장하면
+     * Then 해당 지하철 노선의 연장된 구간의 정보를 확인할 수 있다.
+     */


조건이 2개 이상이면 Given을 2개 써도 좋을 것 같아요 😃

Suggested change

/**

* Given 지하철역 및 지하철 노선을 생성하고 지하철 구간을 추가하고

* When 새로운 구간을 연장하면

* Then 해당 지하철 노선의 연장된 구간의 정보를 확인할 수 있다.

*/

/**

* Given 지하철역 및 지하철 노선을 생성하고

* Given 지하철 구간을 추가하고

* When 새로운 구간을 연장하면

* Then 해당 지하철 노선의 연장된 구간의 정보를 확인할 수 있다.

*/

testrace · 2023-07-12T05:14:49Z

src/test/java/subway/line/LineAcceptanceTest.java

+    /**
+     * Given 지하철역 및 지하철 노선을 생성하고 지하철 구간을 추가하고
+     * When 새로운 구간을 등록할 때
+     * Then 새로운 구간의 상행역은 해당 노선에 등록되어있는 하행 종점역이 아닐경우 에러가 발생한다
+     */


테스트 코드의 when 절과 인수 조건 주석의 when을 일치 시키면 좋을 것 같은데 진우님 생각은 어떠신가요?

Suggested change

/**

* Given 지하철역 및 지하철 노선을 생성하고 지하철 구간을 추가하고

* When 새로운 구간을 등록할 때

* Then 새로운 구간의 상행역은 해당 노선에 등록되어있는 하행 종점역이 아닐경우 에러가 발생한다

*/

/**

* Given 지하철역 및 지하철 노선을 생성하고 지하철 구간을 추가하고

* When 새로운 구간의 상행역은 해당 노선에 등록되어있는 하행 종점역이 아닌 구간을 추가하면

* Then 구간 등록에 실패한다

*/

testrace · 2023-07-12T05:15:33Z

src/test/java/subway/line/LineAcceptanceTest.java

+    /**
+     * Given 지하철역 및 지하철 노선을 생성, 지하철 구간을 2개를 추가 하고
+     * When 구간을 축소 할 경우
+     * Then 해당 구간은 삭제가 된다.
+     */


인수 조건에서 사용하신 연장, 축소라는 표현이 인상적이네요 👍 👍

testrace · 2023-07-12T05:17:15Z

src/test/java/subway/line/LineStep.java

+import java.util.List;
+import java.util.Map;
+
+public class LineStep {


Step 클래스 분리 💯

jwoo-o added 4 commits July 11, 2023 17:03

doc : update README.md

30b5295

refect : 지하철역 인수테스트 step으로 분리

ecc4272

feat : 지하철노선 구간 추가,삭제 기능 개발

5e67bf1

test : 지하철 구가 추가,삭제 인수테스트

c84d171

testrace requested changes Jul 12, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jwoo-o commented Jul 11, 2023

testrace left a comment

testrace Jul 12, 2023

testrace Jul 12, 2023

testrace Jul 12, 2023

testrace Jul 12, 2023

testrace Jul 12, 2023

testrace Jul 12, 2023

testrace Jul 12, 2023

testrace Jul 12, 2023

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

Are you sure you want to change the base?

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

Conversation

jwoo-o commented Jul 11, 2023

testrace 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