[🚀 사이클1 - 미션 (보드 초기화 + 기물 이동)] 에버 미션 제출합니다. by Chocoding1 · Pull Request #265 · woowacourse/java-janggi

Chocoding1 · 2026-03-30T06:47:31Z

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?

어떤 부분에 집중하여 리뷰해야 할까요?

1. View의 모든 메서드를 static으로 변환한 것

변환한 이유는 다음과 같습니다.

굳이 상태가 없고, 유틸리티성 클래스처럼 느껴졌습니다.
정상 입력이 될 때까지 retry하는 ActionExecutor를 추가했는데, 이 때에도 OutputView를 사용할 때 객체를 전달해주는 것이 무의미하다고 느껴졌습니다.
하지만 확장성 측면을 생각해서, 객체로 두는게 나은가 고민도 됩니다... 지금 규모에서는 괜찮을까요? 아니면 지금 바로 챙겨야 할 확장성일까요?

2. Position 관리에 대해

Position은 좌표값을 가지는 불변 객체이고, 값 기반 비교를 위해 equals/hashCode를 재정의했습니다.
현재는 여러 곳에서 새로 생성해서 Map의 key로 사용하고 있습니다.
Position을 “도메인 값 객체”로 봐도 되는지, 아니면 너무 범용적으로 쓰고 있는지 리뷰어님의 의견이 궁금합니다.

3. move()와 canMove()의 책임 분리에 대해

현재 코드는 canMove()로 먼저 이동 가능 여부를 검증한 뒤, 그 다음 move()를 호출하는 구조입니다.
이 방식은 호출하는 쪽이 “이동 가능 여부를 먼저 확인해야 한다”는 사실을 알아야 한다는 점에서 고민이 되었습니다.
move() 내부에서 canMove()를 호출하도록 만들 수도 있다고 생각했는데, 그렇게 하면 검증과 상태 변경이 한 메서드에 함께 들어가면서 책임이 섞인다는 생각을 했습니다.
저는 지금은 검증(canMove)과 상태 변경(move)을 분리하는 쪽을 선택했는데, 이 분리가 적절한지 궁금합니다.

특히 아래 관점에서 의견을 듣고 싶습니다.

호출하는 쪽이 검증 순서를 알아야 하는 구조가 괜찮은지
move()가 내부적으로 검증까지 책임지는 편이 더 자연스러운지
현재처럼 canMove()와 move()를 분리했을 때 책임이 더 명확해지는지

4. 새로운 객체 생성에 대해

이번 구현에서는 어디까지 가변으로 두고, 어디부터 불변으로 둘지 고민이 있었습니다.
현재는 JanggiGame만 가변 객체로 두고, Turn, Board 등 하위 도메인 객체는 새 객체를 반환하는 방식으로 불변을 의도로 관리하고 있습니다.
추후 기보 조회 기능을 추가할 계획으로 Turn마다 보드판을 가지고 있게 하려고 설계했기 때문입니다.
다만 이런 식으로 설계하다보니 각각의 Turn 내부의 기록이 외부에서 바뀌지 않게 하기 위해 매번 새로운 객체를 생성해서 반환하도록 하고 있습니다. 이렇게 매번 새로운 객체를 생성하는 방식이 최선의 방법인지 궁금합니다.

Arachneee

안녕하세요 에버!

리뷰어 백호입니다. 😄

미션하느라 고생하셨습니다. 객체의 역할과 책임에 대해 좀 더 고민해보면 좋을 것 같아요. 코멘트 확인부탁드려요~

View의 모든 메서드를 static으로 변환한 것
변환한 이유는 다음과 같습니다. ...
하지만 확장성 측면을 생각해서, 객체로 두는게 나은가 고민도 됩니다... 지금 규모에서는 괜찮을까요? 아니면 지금 바로 챙겨야 할 확장성일까요?

객체로 두면 어떤 확장성을 가질 수 있을까요??

Position 관리에 대해 ...
Position을 “도메인 값 객체”로 봐도 되는지, 아니면 너무 범용적으로 쓰고 있는지 리뷰어님의 의견이 궁금합니다.

에버는 도메인이 뭐라고 생각하시나요? 도메인 값 객체는 왜 만드는 것일까요? 관련 코멘트도 많이 남겨놨습니다!

move()와 canMove()의 책임 분리에 대해
저는 지금은 검증(canMove)과 상태 변경(move)을 분리하는 쪽을 선택했는데, 이 분리가 적절한지 궁금합니다.

에버는 책임이 뭐라고 생각하시나요? 자신의 상태를 변경하는 검증이 없는데 그걸 책임이 있다고 할 수 있을까요?

새로운 객체 생성에 대해
이번 구현에서는 어디까지 가변으로 두고, 어디부터 불변으로 둘지 고민이 있었습니다. ...
다만 이런 식으로 설계하다보니 각각의 Turn 내부의 기록이 외부에서 바뀌지 않게 하기 위해 매번 새로운 객체를 생성해서 반환하도록 하고 있습니다. 이렇게 매번 새로운 객체를 생성하는 방식이 최선의 방법인지 궁금합니다.

에버는 객체를 왜 불변으로 두셨나요? 가변 객체와의 장단점은 무엇일까요??

Arachneee · 2026-03-31T12:33:03Z

src/main/java/janggi/domain/piece/Piece.java

@@ -0,0 +1,16 @@
+package janggi.domain.piece;
+
+import janggi.domain.Board;


Piece가 Board를 의존하고 Board는 Piece를 의존하고 있네요. 이런 순환참조가 이루어지면 어떤 문제가 있을까요?

추후에 Board의 메서드명이 변경되면 Piece에서도 변경을 해줘야 하는 문제가 발생할 것 같습니다.

순환 참조를 제거하기 위해 Board가 필요한 로직들을 Board로 옮기고 그 로직에서 필요한 정보들을 Piece로부터 받아오도록 수정했습니다!
그러나 현재 로직이 깔끔하다는 생각은 들지 않는데요, 혹시 더 나은 방법이 있다면 알려주시면 감사하겠습니다:)

Arachneee · 2026-03-31T12:35:57Z

src/main/java/janggi/domain/piece/Cha.java

+    private boolean isSamePosition(int startX, int startY, int endX, int endY) {
+        return startX == endX && startY == endY;
+    }
+
+    private boolean isStraightDirection(int startX, int startY, int endX, int endY) {
+        return startX == endX || startY == endY;
+    }


좌표가 같은지 직선인지 판단하는 것은 누구의 책임일까요? 판단하는데 어떤 정보가 필요하고 해당 정보는 누가 제일 잘 알고있는지 생각해보면 좋을 것 같아요!

좌표 관련 연산은 좌표 정보를 가지고 있는 Position에 요청하는 것이 더 적합하다고 생각하여 수정했습니다!

Arachneee · 2026-03-31T12:37:34Z

src/main/java/janggi/domain/piece/Gung.java

+    private boolean isSamePosition(int distanceX, int distanceY) {
+        return distanceX == 0 && distanceY == 0;
+    }
+
+    private boolean isOneStep(int distanceX, int distanceY) {


여기도 이 메소드는 누가 제일 잘 제공할 수 있을지 고민해보면 좋을 것 같아요!
Cha와 중복된 메소드도 있네요.

해당 로직도 좌표 정보를 가지고 있는 Position 객체에서 담당하는 것이 적합하다고 생각하여 Position 객체로 옮겼습니다!

Arachneee · 2026-03-31T12:38:06Z

src/main/java/janggi/domain/piece/Jol.java

+            .findFirst();
+    }
+
+    private boolean isSamePosition(int distanceX, int distanceY) {


Arachneee · 2026-03-31T12:39:12Z

src/main/java/janggi/domain/piece/Jol.java

+    private List<MovePath> createPaths() {
+        if (teamType == TeamType.HAN) {
+            return List.of(
+                new MovePath(List.of(Delta.createDown())),
+                new MovePath(List.of(Delta.createLeft())),
+                new MovePath(List.of(Delta.createRight()))
+            );
+        }
+        return List.of(
+            new MovePath(List.of(Delta.createUp())),
+            new MovePath(List.of(Delta.createLeft())),
+            new MovePath(List.of(Delta.createRight()))
+        );
+    }


매번 객체를 만들 필요없이 미리 만들어놔도 좋을 것 같아요. 다른 Piece의 경로 리뷰는 생략합니다.

말씀해주신대로 프로그램 실행 중 바뀌지 않는 값을 가진 객체를 매번 생성하는 것은 비효율적이라고 생각이 들어, 각 클래스에 List를 상수로 가지도록 수정했습니다.
이렇게 함으로써 MovePath객체가 프로그램 실행 시 한 번만 생성되는 것이 가능해졌습니다.

처음에는 PieceType이라는 enum 클래스의 필드로 List를 반환하는 함수형 인터페이스를 만들까도 고려했습니다. 그러나 각 기물의 이동 방식이 수정될 때 변경해야할 곳의 개수를 따졌을 때, List를 enum 클래스의 필드에 두나, 각 기물 클래스의 상수로 두나 변경할 곳의 개수는 차이가 없다는 생각이 들어, 상대적으로 구현이 간단한 상수를 사용하기로 결정했습니다.

혹시 제가 생각한 방법 외에도 다른 방법이 있다면 알려주시면 감사하겠습니다!

Arachneee · 2026-03-31T12:49:49Z

src/main/java/janggi/domain/Board.java

+    private Board createMovedBoard(TeamType currentTeamType, Team updatedCurrentTeam, Team updatedOpponentTeam) {
+        if (currentTeamType == TeamType.CHU) {
+            return new Board(updatedCurrentTeam, updatedOpponentTeam);
+        }
+        return new Board(updatedOpponentTeam, updatedCurrentTeam);
+    }


Board를 불변객체로 만들면 어떤 장점이 있나요??

현재 List을 통해 기보를 기록하도록 설계를 했고, 각 Turn 안에는 턴마다의 Board 상태가 필드로 존재합니다.
이 Board를 불변 객체로 만들면 다른 Turn에서의 Board 변경이 이전 Turn의 Board에 영향을 주지 않게 되고, 이로 인해 각 턴에서의 Board 상태를 온전히 보존할 수 있습니다!

Arachneee · 2026-03-31T12:50:20Z

src/main/java/janggi/domain/Delta.java

@@ -0,0 +1,52 @@
+package janggi.domain;
+
+public class Delta {


Delta와 Position은 어떤 차이가 있을까요?

Delta는 동등성을 보장하지 않은 이유는 뭘까요?

Position은 기물들의 위치 좌표를 나타내는 객체이고, Delta는 단위 방향을 나타내는 객체입니다.
Delta 객체끼리 비교하는 코드가 없어서 따로 동등성을 보장하지 않아도 된다고 생각했습니다.
그러나 Delta 객체는 내부의 값이 같으면 같다고 보는 객체이기 때문에 동등성을 보장하는 것이 맞다는 생각이 들었습니다.

Delta를 enum 클래스로 변경하면서 동등성을 보장하도록 수정했습니다!

추가로 내부 값을 기준으로 동등성을 판단한다는 면에서는 Position과 Delta의 차이는 없다는 생각이 들었는데, 혹시 틀렸다면 알려주시면 감사하겠습니다.

Arachneee · 2026-03-31T12:52:26Z

src/main/java/janggi/domain/Delta.java

+    public static Delta createUp() {
+        return new Delta(0, 1);
+    }


createUp을 호출할 때마다 Delta 인스턴스를 계속 새로 만들어야할까요?

Delta 객체는 한 번 생성되면 바뀌지 않는 객체이기 때문에 enum 클래스로 변경하여 계속해서 새롭게 생성할 필요가 없도록 수정했습니다!

Arachneee · 2026-03-31T12:53:46Z

src/main/java/janggi/domain/MovePath.java

+    private int totalDx() {
+        return path.stream()
+            .mapToInt(Delta::dx)
+            .sum();
+    }
+
+    private int totalDy() {
+        return path.stream()
+            .mapToInt(Delta::dy)
+            .sum();
+    }


MovePath보다 더 잘 수행할 수 있는 객체가 있을 것 같아요!

MovePath가 List를 가지고 있는 일급 컬렉션이라고 생각해서 Delta들의 합을 구하는 로직은 MovePath에서 담당하는 것이 적합하다는 생각이 드는데요,
제가 미처 생각하지 못 한 부분이 있는지 말씀해주시면 감사하겠습니다!

Arachneee · 2026-03-31T12:54:40Z

src/main/java/janggi/domain/MovePath.java

+    public boolean matches(int dx, int dy) {
+        return totalDx() == dx && totalDy() == dy;
+    }
+
+    public boolean matchesDirection(int dx, int dy) {


Chocoding1 · 2026-04-03T14:00:59Z

안녕하세요 에버!

리뷰어 백호입니다. 😄

미션하느라 고생하셨습니다. 객체의 역할과 책임에 대해 좀 더 고민해보면 좋을 것 같아요. 코멘트 확인부탁드려요~

View의 모든 메서드를 static으로 변환한 것
변환한 이유는 다음과 같습니다. ...
하지만 확장성 측면을 생각해서, 객체로 두는게 나은가 고민도 됩니다... 지금 규모에서는 괜찮을까요? 아니면 지금 바로 챙겨야 할 확장성일까요?

객체로 두면 어떤 확장성을 가질 수 있을까요??

View를 객체로 두면 추후에 콘솔 화면이 아닌 다른 종류의 화면으로 바뀌었을 때 다형성을 활용하여 객체만 갈아끼워도 프로그램이 돌아갈 수 있습니다. 그러나 static 메서드의 경우 오버라이딩이 불가능하기 때문에 다형성을 활용하지 못 하는 단점이 존재할 것이라고 생각합니다.

Position 관리에 대해 ...
Position을 “도메인 값 객체”로 봐도 되는지, 아니면 너무 범용적으로 쓰고 있는지 리뷰어님의 의견이 궁금합니다.

에버는 도메인이 뭐라고 생각하시나요? 도메인 값 객체는 왜 만드는 것일까요? 관련 코멘트도 많이 남겨놨습니다!

도메인은 코드를 짜기 전에 이해해야 하는 특정 분야의 지식이라고 생각합니다. 그리고 단순한 객체에 도메인 지식이 들어가는 순간 도메인 객체가 된다고 생각합니다.
값 객체는 내부 속성이 같으면 같은 객체인 특성을 가진 객체라고 생각합니다.
그리고 이 값 객체에 도메인 지식이 녹여진 것을 도메인 값 객체라고 생각합니다.

현재 Position 객체에 녹여진 도메인 지식은 "Position은 좌표로 이루어져 있어야 하면 각 입력값은 숫자여야 한다" 정도입니다.
Position이 꼭 두 개의 숫자로 이루어지지 않아도 된다는 점에서 현재 Position에 있는 도메인 지식은 그 가치를 한다고 생각하지만,
장기의 직접적인 도메인 규칙(좌표 범위 검증)이 들어가 있지 않다는 점에서 과연 장기만을 위한 Position일까라는 생각도 듭니다.

이런 상태의 Position 객체를 View단에서 직접 사용하는 것이 과연 적합한 것일지 궁금합니다.

move()와 canMove()의 책임 분리에 대해
저는 지금은 검증(canMove)과 상태 변경(move)을 분리하는 쪽을 선택했는데, 이 분리가 적절한지 궁금합니다.

에버는 책임이 뭐라고 생각하시나요? 자신의 상태를 변경하는 검증이 없는데 그걸 책임이 있다고 할 수 있을까요?

canMove를 통해 목적지 좌표를 재입력받을지 여부를 결정하기 때문에 분리하는 것이 적절하다는 결론을 내렸습니다!

새로운 객체 생성에 대해
이번 구현에서는 어디까지 가변으로 두고, 어디부터 불변으로 둘지 고민이 있었습니다. ...
다만 이런 식으로 설계하다보니 각각의 Turn 내부의 기록이 외부에서 바뀌지 않게 하기 위해 매번 새로운 객체를 생성해서 반환하도록 하고 있습니다. 이렇게 매번 새로운 객체를 생성하는 방식이 최선의 방법인지 궁금합니다.

에버는 객체를 왜 불변으로 두셨나요? 가변 객체와의 장단점은 무엇일까요??

각 Turn의 정보들이 다른 Turn에 의해 바뀌는 것을 방지하기 위해 불변 객체로 두었습니다.
다만 이렇게 불변 객체로 두는 경우 매 턴마다 새롭게 객체를 생성해서 반환해야 하기 때문에 성능에 지장이 있진 않을까 했습니다.
그러나 지금처럼 각 턴이 독립적으로 존재하기 위해서는 불변 객체를 사용하는 방법이 적절하다는 생각이 듭니다.

boyekim and others added 30 commits March 25, 2026 16:16

docs: 기능명세 추가

187d101

feat: Team 인터페이스 추가

c9434e9

feat: Team 인터페이스를 상속받는 초나라, 한나라 도메인 추가

b6cb083

feat: Board 도메인 추가

3ea93c4

feat: 기물을 표현하는 Piece 인터페이스 추가

5542ff4

feat: 방향을 관리하는 Delta 도메인 추가

e01fb6a

feat: Delta의 집합 도메인 추가

9dab556

feat: 기물의 점수와 한글 이름을 관리하는 enum 추가

911e4ea

feat: 한나라와 초나라를 명시하는 enum 추가

6b40cfd

feat: 기물 별 이동 판단 로직 구현

a969e12

feat: 기물 묶음 도메인 추가

f637cef

feat: 위치 객체 추가

974e9ae

refactor: score 필드 값 제거

4f8bd85

feat: 경기를 관리할 Turn 도메인 추가

e0bc871

feat: 위치정보 반환 메서드 추가

4d70770

feat: 장기판 초기화 기능

3ea6046

feat: 기물 좌표 입력 및 반환 기능 구현

8bdc49c

feat: 장기 이동 기능 추가

02e3cc8

feat: 장기 타입 반환 구현부 추가

af928a6

refactor: findTeamPiece 파라미터 수정 및 메서드 분리

c5900f0

refactor: 클래스명 Turns -> JanggiGame 변경

57b9d55

refactor: 클래스명 Turns -> JanggiGame 변경

9e4a136

refactor: View 메서드 static 메서드로 변경

829e685

refactor: 초기 JanggiGame 생성 로직 수정

08a72d1

refactor: 기물 이동 시 기물 조회 로직 수정

0b38b64

feat: 턴 명시 출력 기능 추가

d0a9671

feat: 한 턴 종료 이후 보드 현황 출력 기능 추가

106c6ad

chore: 주석 삭제

40bdea8

refactor: 빠진 검증부 추가

74fc4e6

refactor: 사용하지 않는 메서드 삭제 및 모호한 메서드명 변경

3d674d3

Chocoding1 added 6 commits March 31, 2026 15:55

refactor: 미사용 메서드 제거

0026eb7

refactor: 중복 로직 메서드 추출

4590338

refactor: 메서드 순서 변경

544c68f

refactor: Piece 인터페이스에서 불필요 메서드 제거 및 테스트 코드 수정

baa91d4

refactor: 불필요 테스트 코드 제거 및 테스트 코드 수정

1c198af

refactor: BoardSpot에서 Position 필드 제거

aa9d572

Arachneee requested changes Mar 31, 2026

View reviewed changes

Chocoding1 added 22 commits April 1, 2026 14:13

refactor: 동일 위치 검증 로직 Position 객체로 이전

e3d00ea

refactor: 한 칸만 갔는지 확인하는 메서드 Position 객체로 이전

8f0f014

refactor: 직선 이동 체크 로직 Position 객체로 이동

95a3b05

refactor: delta 계산 로직 Position 객체 내부로 이동

604b712

refactor: Delta는 바뀌지 않기 때문에 enum 클래스로 변경

53f1b21

refactor: MovePath객체 매번 생성하지 않도록 상수 변수로 수정

d4aa367

refactor: Piece interface를 추상 클래스로 변경하여 중복 메서드 및 중복 필드 제거

31c58d9

refactor: 메서드 순서 변경

dfac78c

refactor: 에러 메시지 수정

6fbbd6e

chore: 메서드명 수정

871c469

refactor: 기물 이동 가능 여부 검증 로직 수정

3ee750c

refactor: 중복 메서드 추상 클래스로 이동

5f067b4

chore: 빈 줄 제거

26e1f8b

refactor: 미사용 메서드 제거

079cf1e

refactor: 중복 코드 제거를 위해 예외 발생 위치 변경

b0619a0

refactor: Pieces 클래스 제거 & Team 인터페이스 추상 클래스로 변경

bceaae4

refactor: getter에 방어적 복사 적용

9c14ad8

refactor: DTO 내부에 팩토리 메서드를 추가해 도메인에서 DTO 내부 구조를 모르도록 수정

5ab4d46

chore: 메서드 순서 변경

ad0ab80

refactor: 로직 메서드 추출

1270f39

refactor: 추상 클래스를 생성하여 중복 로직 제거

ba02709

refactor: Piece와 Board 간의 순환 참조 제거

2ffdaf4

		@@ -0,0 +1,16 @@
		package janggi.domain.piece;

		import janggi.domain.Board;

Conversation

Chocoding1 commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

체크 리스트

어떤 부분에 집중하여 리뷰해야 할까요?

1. View의 모든 메서드를 static으로 변환한 것

2. Position 관리에 대해

3. move()와 canMove()의 책임 분리에 대해

4. 새로운 객체 생성에 대해

Uh oh!

Arachneee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chocoding1 Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Chocoding1 Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Chocoding1 Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Chocoding1 commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Chocoding1 commented Mar 30, 2026 •

edited

Loading

Chocoding1 Apr 3, 2026 •

edited

Loading

Chocoding1 Apr 1, 2026 •

edited

Loading

Chocoding1 Apr 2, 2026 •

edited

Loading