[이예진] 사다리 게임 구현 (1-5단계) #66

yaevrai · 2025-06-06T16:19:11Z

리뷰

안녕하세요 😊 이번 미션도 재미있게 구현했습니다..! 말씀해 주셨던 대로 사다리를 처리하는 게 쉽지않았어요 ..핳
이번에는 먼저 코드를 작성하기 보다 생각을 한 후에 구현을 하는 방향으로 진행해봤습니다..! 그리고 미션의 기본 요구사항을
객체지향 생활체조 원칙이라고 하는 걸 알게됐습니다🙌 이 원칙을 무의식 중에 놓치는 경우가 있었는데 (특히 depth 처리요..ㅎ)
이번 학습테스트 주제이기도 한 함수형 메서드를 통해서 쉽게 개선할 수 있는 방향을 찾은 것 같았습니다.
또 일급 컬렉션 을 제대로 활용해보려 노력했는데요.. 하지만 여전히 고민되는 부분들이 많네요..😅 지난번 팁으로 알려주신 getter의 사용이 줄었는 지를 통해 객체의 책임이 잘 분배되었는지 신경 썼는데요. 아직도 스스로 명확하게 분리하고있단 자신은 없습니다. 하핳

질문

LadderGame 클래스가 현재 너무 많은 책임을 가지고 있는 것 같기도한데요..! 게임의 시작점이 되는 부분이라 총괄 책임을 진다는 느낌으로
여러 책임을 모아뒀는데 어떻게 보이시나요??
이번 미션에서의 객체간의 책임 분배는 어땠을까요!?
enum을 사용하기위해 ResultType 객체를 생성해 전체 참가자/개인으로 분리하는 부분을 고정시켰습니다. 그런데 필드로 선언한 value값을 사용하지 않기도 하고, 출력을 할 때 한번 선택을 위해 enum을 생성한 게 올바른가? 라는 고민을 했어요. 아니면 이번에 다른 부분에서 enum을 활용할 수도 있었을까요?

이런 부분들이 계속 고민이 됩니다.. 이번에도 좋은 피드백 부탁드립니다! 🙏

기능 설명

Controller

LadderGame
- 사다리 게임의 시작점
- 참가자 입력을 받고 게임을 실행
- 결과를 출력하는 흐름을 관리

Model

GameSetup
- 게임에 필요한 기본 설정
- 참가자 수, 사다리 높이 등의 정보를 포함
Ladder
- 실제 사다리의 구조 표현
- Line들의 집합으로 구성
- 사다리를 타고 내려가는 로직 포함
Line
- 사다리 한 줄을 표현
- boolean 타입의 points로 가로선 유무를 표시
Name
- 참가자의 이름
- 이름 유효성 검증 로직 포함
Participants
- Name 객체들의 일급 컬렉션
- 참가자들의 순서와 정보를 관리
Results
- 게임의 결과를 관리
- 참가자별 결과를 매핑
ResultType (enum)

View

InputView
- 사용자로부터 입력을 받음
OutputView
- 결과 출력

yaevrai · 2025-06-06T16:24:07Z

src/test/java/ladder/LadderTest.java

+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+public class LadderTest {


AssetJ를 통해 가독성이 좀 더 나은 테스트 코드를 작성했습니다!
미션을 진행하면서 학습테스트의 복습 필요성을 느낍니다 !

AssertJ를 사용해서 Assertion의 가독성을 높여주셨네요!

하지만 정작 테스트가 무엇을 의미하는지에 대한 가독성은 그렇게 좋은 것 같지 않네요.

테스트에는 "사다리 생성"만 적혀있는데, 이를 통해 어떠한 정보를 얻을 수 있나요?

사다리 게임의 결과를 확인할 수 있는 테스트 코드도 필요해 보여요. 😂

yaevrai · 2025-06-06T16:27:56Z

src/main/java/ladder/model/Line.java

-    for (Boolean point : points) {
-      System.out.print("|");
-      printLine(point);
+    public void draw() {


Line이란 객체에게 사다리 라인을 만드는 책임을 부여했는데요,
라인 생성 == 사다리 출력이 되는 부분이라 이 부분이 view 객체로 가야하나? 라는 생각이 들었습니다.
결과적으로는 Line클래스 내에서 처리하는 게 어떻게 라인을 생성해야하는 지에 대한 비즈니스 로직을 알 수 있고, 좀 더 명확해 보인다고 생각했습니다 😅

사다리 게임은 사용자의 입력이 완료되면(참여자, 결과), 출력하지 않아도 게임의 결과를 알 수 있어요.

콘솔에 출력되는 사다리의 모양은 그저 사람이 결과를 보기 쉽게 알 수 있도록 해주는 문자열에 불과해요.

그렇다면 사다리를 출력하는 로직이 Line에 있는 게 올바를까요?

그저 자신의 상태를 View에 전달해서, 이를 출력하도록 하면 되지 않을까요?

yaevrai · 2025-06-06T16:31:18Z

src/main/java/ladder/model/Results.java

+public class Results {
+
+    private final Participants participants;
+    private final List<String> results;


List results 게임 결과는 사용자에게 입력 받은 값을 그대로 출력해주는 역할 그 이외의 것을 하지 않아
일급컬렉션을 만드는 대신 List로 필드 선언을 했습니다. 일급 컬렉션으로 만드는 게 더 나은 선택이었을까요?
아직 기준이 모호한 듯 합니다.. ㅎㅎ

일급 컬렉션은 둘째치고, results가 가지는 List<String>이 어떤 의미를 나타내는지 의도를 전달하는 게 더 중요해 보여요.

이 경우 일급 컬렉션을 만들지 않았다면, List<Result>와 같이 타입을 명시적으로 나타내는 게 좋지 않았을까요?

yaevrai · 2025-06-06T16:37:31Z

src/main/java/ladder/model/GameSetup.java

+    private final List<String> results;
+    private final Ladder ladder;
+
+    public GameSetup(Participants participants, List<String> results, Ladder ladder) {


LadderGame 클래스에서 play()에서의 메서드 길이를 줄이기 위해서 구현 후에 게임 관련 정보를 담는 커맨드 객체 GameSetup를 만들어서 개선을 했는데요. 애초에 LadderGame 역할 분배가 더 이루어졌으면 메서드 길이를 줄이기 위한 목적으로 객체를 굳이 만들지 않아도 되었을까? 라는 생각은 들었는데 어떻게 개선해야할지 방도가 잘 생각이 안났습니다🤣 이전 미션에서도 그렇고 LadderGame 같이 시작점이 되는 메서드가 여러 역할을 갖는 것 같은데, 또 한편으론 시작점이기 때문에 여러 진행 포인트가 합쳐저 있는게 당연하지않나? 라는 생각도 들어요. 제 질문이.. 잘 이해되실지 모르겠습니다 ㅜㅜ 글렌이라면 어떻게 하셨을 거 같은지 궁금합니다!!

우선 해당 객체 때문에 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다. 라는 요구사항이 지켜지지 못한 것 같네요.

하지만 이러한 시도를 해주시고, 본인이 시도한 방법에 대해 성찰해 보시는 것은 분명 성장하는데 큰 도움이 될 거에요.

우선 저였다면, 미션의 요구사항과 결과를 보고 사다리 게임을 분석했을 것 같아요.

사다리 게임을 간단하게 분석해 보면, 사다리 게임은 사용자의 입력이 완료되면 결과는 결정적이에요.
(사다리의 생성은 랜덤하게 생성되기에 비결정적이지만, 사다리가 생성된 후에는 결과가 절대로 변하지 않아요)

또한 사다리 게임에서 가장 중요한 것은 결과가 제대로 나오는 것인데, 이를 쉽게 테스트할 수 있도록 구조를 만들 것 같아요.
(위 리뷰에서 남겼듯, 사디리의 출력은 그저 사람이 결과를 보기 쉽게 알 수 있도록 해주는 문자열에 불과해요.)

그렇다면 이를 통해, 다음과 같은 클래스를 구현했을 것 같아요.

public class Ladder { private final List<Line> lines = ... public Ladder(List<Line> lines) { // 유효성 체크 this.lines = lines; } public LadderResult calculateResult() { // 사다리 게임 로직 } } public class LadderResult { private final Map<Participant, Result> participantToResult; ... public Result getResult(Participant participant) { // 유효성 체크 return participantToResult.get(participant); } }

이 클래스를 기반으로 TDD로 구현하거나, 아니면 그저 구현해 본 뒤, 결과가 제대로 나오는 것을 확인한 뒤, 다른 요구사항을 구현하지 않았을까 싶네요.

seokjin8678

안녕하세요 예진님, 한 번에 5단계까지 구현하느라 고생 많으셨습니다! 👍👍

기존의 까다롭던 요구사항이 계속 유지되고, 새로운 요구사항이 계속 추가되어서 구현하기 힘드셨을 것 같아요.

또한 미션의 내용도 복잡해 보여, 더 그랬을 것 같네요. 😂

우선 전체적인 리뷰를 드리자면, 사다리 게임의 결과를 확인할 수 있는 테스트 코드가 없어요.

사다리 게임을 실행하지 않고도, 결과를 확인할 수 있는 테스트 코드를 만들어 주세요!

또한 요구사항 일부를 지키지 못한 곳이 보여요.

이는 리뷰를 참고하시면 되지 않을까 싶네요.

리뷰 반영해 주시고, 재요청 부탁드립니다!

LadderGame 클래스가 현재 너무 많은 책임을 가지고 있는 것 같기도한데요..! 게임의 시작점이 되는 부분이라 총괄 책임을 진다는 느낌으로 여러 책임을 모아뒀는데 어떻게 보이시나요??

아직은 이른 책이긴 한데요, 클린 코드의 저자인 로버트 마틴이 쓴 책 중 "클린 아키텍처"의 26장 메인 컴포넌트 항목에 다음과 같은 내용이 나와요.

"모든 시스템에는 최소한 하나의 컴포넌트가 존재하고, 이 컴포넌트가 나머지 컴포넌트를 생성하고, 조정하며, 관리한다. 나는 이 컴포넌트를 메인(Main)이라고 부른다. (중략) 메인을 지저분한 컴포넌트 중에서도 가장 지저분한 컴포넌트라고 생각하자"

어플리케이션을 실행하기 위해, 모든 정보를 알아야 하는 지저분한 클래스(컴포넌트)는 반드시 존재해야 해요.

그리고 이를 LadderGame이라는 이름으로 잘 구현해 주신 것 같아요. (LadderGame 클래스가 main 메서드를 가지는 Application 클래스와 같은 패키지에 있다는 것이 메인 컴포넌트가 가지는 일부 책임을 잘 분리한 것으로 보이네요.)

이번 미션에서의 객체간의 책임 분배는 어땠을까요!?

냉정하게 말씀드리면.. 적절하지 않은 객체가 몇 보여요. 😂

Line의 경우 생성할 때, 어떠한 객체에 의존되고 있나요?

또한 import에는 View에 관한 의존이 보이지 않는데, 내부에는 View에 관한 의존이 숨겨져 있네요.

그렇다면 Line은 최소 3가지의 책임을 지고 있군요.

enum을 사용하기위해 ResultType 객체를 생성해 전체 참가자/개인으로 분리하는 부분을 고정시켰습니다. 그런데 필드로 선언한 value값을 사용하지 않기도 하고, 출력을 할 때 한번 선택을 위해 enum을 생성한 게 올바른가? 라는 고민을 했어요. 아니면 이번에 다른 부분에서 enum을 활용할 수도 있었을까요?

이 경우, 오히려 enum 때문에 코드의 복잡도가 증가하고, 이러한 고민을 하느라 시간이 더 소요되었을 것 같네요.

그렇다면 이는 enum을 사용하지 않고, 그저 boolean을 반환해도 좋았을 것 같아요.

물론 이러한 생각을 해주시는 것은 반드시 해야 할 고민이라고 생각해요.

하지만 제 생각엔 오히려 enum 때문에 손해를 보았으니, boolean을 반환하도록 한 뒤, 추후 새로운 결과를 보고 싶은 타입이 생기면 그때 구현해도 늦지 않았을 것 같아요.

seokjin8678 · 2025-06-08T15:31:51Z

src/main/java/ladder/model/ResultType.java

+    }
+
+    public static ResultType from(String input) {
+        return "all".equals(input) ? ALL : SINGLE;


기존 프로그래밍 요구사항
3항 연산자를 쓰지 않는다.

seokjin8678 · 2025-06-08T15:36:57Z

src/main/java/ladder/model/Participants.java

+    public List<Name> values() {
+        return new ArrayList<>(names);
+    }


Participants 클래스는 게임의 참여자를 의미하는 클래스 같네요!

그리고 values 메서드는 참여자의 이름을 모두 반환하도록 구현되어 있네요.

하지만 values 라는 이름을 보면, 참여자의 이름 목록이라는 의미가 명확하게 전달되지 않아 보여요.

seokjin8678 · 2025-06-08T15:39:40Z

src/main/java/ladder/model/Participants.java

+    public static Participants from(String input) {
+        return new Participants(
+            Arrays.stream(input.split(","))
+                  .map(String::trim)
+                  .map(Name::new)
+                  .collect(Collectors.toList())
+        );
+    }


Participants를 생성하기 위한 정적 팩터리 메서드군요!

이 경우 입력된 문자열에 대해 ,를 기준으로 구분하여 Participants를 만들고 있는 것 같네요!

,는 View에 의존하는 값이고, 입력으로 들어온 문자열 또한 View에 의존되는 값이에요.

이 경우 도메인이 View를 직접적으로 의존하지는 않지만, 간접적으로 의존하는 것 같아요.

seokjin8678 · 2025-06-08T16:10:47Z

src/main/java/ladder/model/Line.java

+    public static Line create(int width) {
+        Random random = new Random();
+        List<Boolean> points = new ArrayList<>();
+        return new Line(IntStream.range(0, width - 1)
+            .mapToObj(i -> shouldConnect(points, i, random))
+            .collect(Collectors.toList()));
+    }
+
+    private static boolean shouldConnect(List<Boolean> points, int index, Random random) {
+        boolean connection = (index <= 0 || !points.get(index - 1)) && random.nextBoolean();
+        points.add(connection);
+        return connection;
+    }


사다리 미션의 요구사항 중 핵심 중 하나인 연결 여부를 랜덤으로 정하고, 라인이 겹치지 않도록 하는 곳을 구현하는 코드로 보이네요!

다만 이 코드를 보면 한 번에 보고 이해하기 어려워 보여요.

shouldConnect 메서드의 파라미터가 너무 많아 보이네요. (Random은 자동차 경주 게임 리뷰에서 남겼듯 다른 구현체를 사용한다면 파라미터로 넘길 필요는 없어 보여요)

create 메서드의 points 변수를 생성자로 넘길 것 같은데, 그저 이전에 연결된 상태를 보기 위한 변수네요. 이 경우 다른 이름을 붙여주는 게 더 좋을 것 같아요.

shouldConnect 메서드의 (index <= 0 || !points.get(index - 1)) && random.nextBoolean() 부분이 특히 이해하기 어렵네요.

또한 5단계 미션의 요구사항 때문에 Stream을 사용하신 것 같아요.

이 경우 Stream을 사용하는 것 보다, for문을 사용하는게 더 가독성이 좋았지 않았을까 싶네요. 😂

seokjin8678 · 2025-06-08T16:17:21Z

src/main/java/ladder/model/Line.java

-    for (Boolean point : points) {
-      System.out.print("|");
-      printLine(point);
+    public void draw() {


사다리 게임은 사용자의 입력이 완료되면(참여자, 결과), 출력하지 않아도 게임의 결과를 알 수 있어요.

콘솔에 출력되는 사다리의 모양은 그저 사람이 결과를 보기 쉽게 알 수 있도록 해주는 문자열에 불과해요.

그렇다면 사다리를 출력하는 로직이 Line에 있는 게 올바를까요?

그저 자신의 상태를 View에 전달해서, 이를 출력하도록 하면 되지 않을까요?

seokjin8678 · 2025-06-08T16:39:55Z

src/main/java/ladder/model/Results.java

+public class Results {
+
+    private final Participants participants;
+    private final List<String> results;


일급 컬렉션은 둘째치고, results가 가지는 List<String>이 어떤 의미를 나타내는지 의도를 전달하는 게 더 중요해 보여요.

이 경우 일급 컬렉션을 만들지 않았다면, List<Result>와 같이 타입을 명시적으로 나타내는 게 좋지 않았을까요?

seokjin8678 · 2025-06-08T16:58:52Z

src/main/java/ladder/model/GameSetup.java

+    private final List<String> results;
+    private final Ladder ladder;
+
+    public GameSetup(Participants participants, List<String> results, Ladder ladder) {


우선 해당 객체 때문에 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다. 라는 요구사항이 지켜지지 못한 것 같네요.

하지만 이러한 시도를 해주시고, 본인이 시도한 방법에 대해 성찰해 보시는 것은 분명 성장하는데 큰 도움이 될 거에요.

우선 저였다면, 미션의 요구사항과 결과를 보고 사다리 게임을 분석했을 것 같아요.

사다리 게임을 간단하게 분석해 보면, 사다리 게임은 사용자의 입력이 완료되면 결과는 결정적이에요.
(사다리의 생성은 랜덤하게 생성되기에 비결정적이지만, 사다리가 생성된 후에는 결과가 절대로 변하지 않아요)

또한 사다리 게임에서 가장 중요한 것은 결과가 제대로 나오는 것인데, 이를 쉽게 테스트할 수 있도록 구조를 만들 것 같아요.
(위 리뷰에서 남겼듯, 사디리의 출력은 그저 사람이 결과를 보기 쉽게 알 수 있도록 해주는 문자열에 불과해요.)

그렇다면 이를 통해, 다음과 같은 클래스를 구현했을 것 같아요.

public class Ladder { private final List<Line> lines = ... public Ladder(List<Line> lines) { // 유효성 체크 this.lines = lines; } public LadderResult calculateResult() { // 사다리 게임 로직 } } public class LadderResult { private final Map<Participant, Result> participantToResult; ... public Result getResult(Participant participant) { // 유효성 체크 return participantToResult.get(participant); } }

이 클래스를 기반으로 TDD로 구현하거나, 아니면 그저 구현해 본 뒤, 결과가 제대로 나오는 것을 확인한 뒤, 다른 요구사항을 구현하지 않았을까 싶네요.

seokjin8678 · 2025-06-08T17:02:09Z

src/main/java/ladder/controller/LadderGame.java

+    }
+
+    private void printGameResults(String result, Results gameResults) {
+        if (ResultType.ALL.equals(ResultType.from(result))) {


저는 enum을 비교할 때, equals 보다 ==을 사용하는 것을 권장해요.

이유는 다음과 같은 코드로 변경하고, 어플리케이션을 실행해 보세요.

ResultType.ALL.equals(TimeUnit.HOURS)

그리고 결과를 확인하신 뒤, 다음과 같이 변경해 보시고 어플리케이션을 실행해 보세요.

ResultType.ALL == TimeUnit.HOURS

seokjin8678 · 2025-06-08T17:37:47Z

src/test/java/ladder/LadderTest.java

+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+public class LadderTest {


AssertJ를 사용해서 Assertion의 가독성을 높여주셨네요!

하지만 정작 테스트가 무엇을 의미하는지에 대한 가독성은 그렇게 좋은 것 같지 않네요.

테스트에는 "사다리 생성"만 적혀있는데, 이를 통해 어떠한 정보를 얻을 수 있나요?

사다리 게임의 결과를 확인할 수 있는 테스트 코드도 필요해 보여요. 😂

seokjin8678 · 2025-06-08T17:38:09Z

src/test/java/ladder/LadderTest.java

+        assertThat(results).hasSize(4);
+        assertThat(results)
+            .allMatch(position -> position >= 0 && position < 4);


Suggested change

assertThat(results).hasSize(4);

assertThat(results)

.allMatch(position -> position >= 0 && position < 4);

assertThat(results)

.hasSize(4)

.allMatch(position -> position >= 0 && position < 4);

yaevrai added 5 commits June 6, 2025 19:58

docs: 요구사항 문서 작성

7686f8c

feat(level1): 사다리 출력

88dea60

feat(level2): 사다리 생성

7196c8e

feat(level3): 사다리 타기

15d8a2e

feat(level4,5): 게임 실행 + 리팩토링

4a6b1c7

yaevrai force-pushed the yaevrai branch from 6649037 to 4a6b1c7 Compare June 6, 2025 16:20

yaevrai commented Jun 6, 2025

View reviewed changes

seokjin8678 suggested changes Jun 8, 2025

View reviewed changes

[이예진] 사다리 게임 구현 (1-5단계) #66

Are you sure you want to change the base?

[이예진] 사다리 게임 구현 (1-5단계) #66

Uh oh!

Conversation

yaevrai commented Jun 6, 2025

리뷰

질문

기능 설명

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seokjin8678 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!