Step2 #2324

bb-worm · 2025-04-05T12:25:02Z

https://edu.nextstep.camp/s/Ie5Dwep0/ls/UGd9q4uG

사다리 생성 미션 리뷰 요청합니다.

고민했던 건 아래와 같습니다.

Optional을 어디에 써야할지 감이 잘 오지 않습니다. 적용하기 좋은 곳이 있을까요?
Point 클래스를 만들어서 이웃한 Point와의 연결 유무를 가지도록 했습니다. 원래는 좌우로 연결 유무를 가지도록 하려고 했는데 너무 복잡해지는 거 같아서.. 일단 오른쪽 연결 유무만 갖도록 했습니다. 그런데 사용자가 이를 알고 사용해야 하니 뭔가 찜찜하더라고요. 더 좋은 방법이 있을까요?
그리고 위처럼 Point가 오른쪽 연결 유무를 갖도록 하니, 마지막 Point는 오른쪽에 이웃한 Point가 없어도 연결되는 경우가 있습니다. 처음에는 생성될 때 막으려고 했는데요, 그러면 필드 값을 set 하는 것 같아서 좀 아닌 거 같더라고요. 아직 구현하지는 않았지만 나중에 실행 단계에서는 마지막 위치인지 확인하는 조건절을 넣을 생각인데요. 애초에 생성 단계에서 막아주는 게 나을까요?
추가 고민점은 코드에 코멘트로 달아놓겠습니다.

- computer version 가져올 때 optional 사용

bb-worm · 2025-04-05T12:26:21Z

src/main/java/ladder/Main.java

+    private static void makeLadder() {
+        People people = InputView.inputPeople();
+        int ladderHeight = InputView.inputLadderHeight();
+        Ladder ladder = new Ladder(ladderHeight, people, (idx, preConnected) -> !preConnected && new Random().nextBoolean());


일부러 람다 사용해보려고 ConnectStrategy를 람다로 표현했습니다. 그런데 오히려 코드 읽기가 어려워지는 거 같아서.. 이런 경우엔 따로 클래스로 빼는 게 나을까요?

클래스도 좋고 변수로 선언해도 좋을 것 같아요

Suggested change

Ladder ladder = new Ladder(ladderHeight, people, (idx, preConnected) -> !preConnected && new Random().nextBoolean());

final ConnectStrategy defaultStrategy = (idx, preConnected) -> !preConnected && RANDOM.nextBoolean();

final Ladder ladder = new Ladder(ladderHeight, people, defaultStrategy);

Optional을 어디에 써야할지 감이 잘 오지 않습니다. 적용하기 좋은 곳이 있을까요?

null을 활용한 설계가 아니라면 억지로 사용하지 않으셔도 됩니다 😃
기능 목록을 정의하실 때 마지막 포인트는 null 이다. 라는 내용이 있었는데요.
실제로 null을 활용했다면 Optional을 활용할 수 있는 지점이 아닐까 싶어요.

testrace

안녕하세요 태룡님 😃
2단계 구현 잘해주셨네요 👍
커밋 흐름이 인상적이었습니다!
몇 가지 코멘트 남겨두었는데 확인해 주시고 다시 리뷰 요청해 주세요.

testrace · 2025-04-06T04:25:57Z

README.md

+### 사다리(생성)
+- [x] 라인 길이를 전달받아 사다리 라인을 생성한다.
+- [x] 각 사다리 라인은 이웃한 곳과 이어져있을 수 있다. 
+- [x] 각 사다리 라인은 겹치지 않게 이어져 있어야 한다.
+- ~~[x] 라인의 각 포인트는 왼쪽/오른쪽 연결 유무를 갖는다.~~
+- [x] 라인의 각 포인트는 오른쪽 연결 유무만 갖는다.
+- [x] 사다리 높이와 라인 길이를 전달받아 사다리를 생성한다.
+- [x] 사람 이름은 최대 5글자까지만 가능하다.


기능 목록 👍

testrace · 2025-04-06T04:30:57Z

src/main/java/ladder/Main.java

+    private static void makeLadder() {
+        People people = InputView.inputPeople();
+        int ladderHeight = InputView.inputLadderHeight();
+        Ladder ladder = new Ladder(ladderHeight, people, (idx, preConnected) -> !preConnected && new Random().nextBoolean());


클래스도 좋고 변수로 선언해도 좋을 것 같아요

Suggested change

Ladder ladder = new Ladder(ladderHeight, people, (idx, preConnected) -> !preConnected && new Random().nextBoolean());

final ConnectStrategy defaultStrategy = (idx, preConnected) -> !preConnected && RANDOM.nextBoolean();

final Ladder ladder = new Ladder(ladderHeight, people, defaultStrategy);

Optional을 어디에 써야할지 감이 잘 오지 않습니다. 적용하기 좋은 곳이 있을까요?

null을 활용한 설계가 아니라면 억지로 사용하지 않으셔도 됩니다 😃
기능 목록을 정의하실 때 마지막 포인트는 null 이다. 라는 내용이 있었는데요.
실제로 null을 활용했다면 Optional을 활용할 수 있는 지점이 아닐까 싶어요.

testrace · 2025-04-06T04:32:32Z

src/main/java/ladder/domain/Ladder.java

+    public Ladder(int height, People people, ConnectStrategy connectStrategy) {
+        this.lines = IntStream.range(0, height)
+                .mapToObj(i -> new Line(people.size(), connectStrategy))
+                .collect(Collectors.toList());
+    }


생성자는 유효성 검증과 변수 초기화에 집중하고,
생성 로직은 정적 팩토리 메서드나 별도의 클래스로 분리하면 어떨까요?

public Ladder(List<Line> lines) { // validate this.lines = lines; } public static Ladder create(int height, People people, ConnectStrategy connectStrategy) { final List<Line> lines = ...; return new Ladder(lines); }

testrace · 2025-04-06T04:32:37Z

src/main/java/ladder/domain/Line.java

+import java.util.List;
+import java.util.Objects;
+
+public class Line {


Line 객체는 특별한 기능이 없는 것은 같아요
Points와 어떤 차이가 있나요?

testrace · 2025-04-06T04:32:42Z

src/main/java/ladder/domain/Line.java

+    public Line(List<Point> points) {
+        this.points = new Points(points);
+    }
+
+    public Line(int size, ConnectStrategy connectStrategy) {
+        this.points = new Points(size, connectStrategy);
+    }


주 생성자를 두어 호출하면 어떨까요?

public Line(Points points) { this.points = points; } public Line(List<Points> points) { this(new Points(points)); }

testrace · 2025-04-06T04:33:00Z

src/main/java/ladder/domain/People.java

+import java.util.stream.Collectors;
+
+public class People {
+    private final static String DELIMITER = ",";


파싱과 관련된 로직은 view 또는 controller로 이동하면 어떨까요?
People 이라는 도메인은 구분기호에 따라 역할이 달라지지 않을 것 같아요.

testrace · 2025-04-06T04:33:04Z

src/main/java/ladder/domain/Person.java

+    private final String name;
+
+    public Person(String name) {
+        if (name.length() > 5) {


Suggested change

if (name.length() > 5) {

if (name.length() > NAME_MAX_LENGTH) {

testrace · 2025-04-06T04:33:09Z

src/main/java/ladder/domain/Point.java

+public class Point {
+    private final boolean rightConnected;


Point 클래스를 만들어서 이웃한 Point와의 연결 유무를 가지도록 했습니다. 원래는 좌우로 연결 유무를 가지도록 하려고 했는데 너무 복잡해지는 거 같아서.. 일단 오른쪽 연결 유무만 갖도록 했습니다. 그런데 사용자가 이를 알고 사용해야 하니 뭔가 찜찜하더라고요. 더 좋은 방법이 있을까요?

그리고 위처럼 Point가 오른쪽 연결 유무를 갖도록 하니, 마지막 Point는 오른쪽에 이웃한 Point가 없어도 연결되는 경우가 있습니다. 처음에는 생성될 때 막으려고 했는데요, 그러면 필드 값을 set 하는 것 같아서 좀 아닌 거 같더라고요. 아직 구현하지는 않았지만 나중에 실행 단계에서는 마지막 위치인지 확인하는 조건절을 넣을 생각인데요. 애초에 생성 단계에서 막아주는 게 나을까요?

좌우연결을 가진 상태로 끝까지 구현해 보셨어도 좋았을 것 같아요.

마지막 질문에 대한 답변을 드리자면 생성 단계에서 막아주는게 좋지 않을까 생각이 드는데요 😃
질문 내용으로 보아 여러 시도와 많은 고민 하신 것 같아 약간의 참고용 힌트를 드리면 어떨까 싶어요.
(힌트일뿐 정답이 아닙니다)

Point first = Point.first(); Point second = first.next(); Point third = second.next(); Point forth = third.last();

Point가 처음, 중간, 마지막에 대한 값을 스스로 결정할 수 있다면 어떨까요?

testrace · 2025-04-06T04:33:48Z

src/main/java/ladder/domain/Points.java

+    public void validate() {
+        IntStream.range(0, points.size()-1)
+                .filter(this::continuousConnected)
+                .findFirst()
+                .ifPresent(i -> {
+                    throw new IllegalArgumentException("각 포인트는 연속적으로 연결될 수 없음");
+                });
+    }


Point가 좌우연결을 가진 구조라면 유효성 검증도 단순화할 수 있을 것 같아요

new Point(true, true); // 생성 불가

testrace · 2025-04-06T04:38:22Z

src/test/java/ladder/LineTest.java

+    @Test
+    void 연속적으로_연결되면_예외() {
+        assertThatThrownBy(() -> new Line(4, (idx, preConnected) -> true)).isInstanceOf(IllegalArgumentException.class);


예외 테스트 👍
예외 메시지까지 함께 검증하면 좋을 것 같아요.

Point, Points, Ladder, Person 에 대한 단위 테스트도 추가하면 어떨까요?

kakao-finn-h added 8 commits April 5, 2025 15:43

feat: 전체 테스트 코드 통과

0421b95

- computer version 가져올 때 optional 사용

chore: 메시지 보내는 메서드 사용

b1014ad

feat: 사다리 라인 생성

1702140

feat: 사다리 라인이 이웃한 곳과 이어져있도록 생성

3e3dfdc

feat: 사다리 라인은 연속적으로 이어져있지 않아야 함

dd877b8

feat: 라인의 각 포인트는 왼쪽/오른쪽 연결 유무를 갖음

400b0a9

refactor: 라인의 각 포인트는 오른쪽 연결 유무만 갖도록 변경

76cac9c

feat: 사용자 입력을 전달받아 사다리 생성 후 출력

348199e

bb-worm commented Apr 5, 2025

View reviewed changes

feat: 사용자 이름 길이는 최대 5자

7c642d4

testrace requested changes Apr 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Step2 #2324

Step2 #2324

bb-worm commented Apr 5, 2025 •

edited

Loading

bb-worm Apr 5, 2025

testrace Apr 6, 2025

testrace left a comment

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

testrace Apr 6, 2025

	Ladder ladder = new Ladder(ladderHeight, people, (idx, preConnected) -> !preConnected && new Random().nextBoolean());
	final ConnectStrategy defaultStrategy = (idx, preConnected) -> !preConnected && RANDOM.nextBoolean();
	final Ladder ladder = new Ladder(ladderHeight, people, defaultStrategy);

	if (name.length() > 5) {
	if (name.length() > NAME_MAX_LENGTH) {

Step2 #2324

Are you sure you want to change the base?

Step2 #2324

Conversation

bb-worm commented Apr 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bb-worm commented Apr 5, 2025 •

edited

Loading