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

Jiihyun · 2026-03-28T14:16:38Z

체크 리스트

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

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

안녕하세요 베루스! 러키🍀 라고 합니다.
이번 장기 미션은 도메인 규칙이 상당히 복잡하다고 느꼈습니다..!
이로 인해 여러 분기가 생기면서 가독성이 떨어지거나 코드 중복이 발생하는 부분들을 최대한 다형성을 활용해 해결해보려고 노력했습니다.
또한 여러 클래스로 책임을 분산하다 보니 전체 흐름을 파악하기 어려워질 수 있겠다는 생각이 들었습니다.
따라서 클래스와 메서드 이름에 신경 쓰기, 메서드 분리 등 코드를 읽는 사람이 의도를 자연스럽게 이해할 수 있도록 최대한 코드에 제 의도를 담으려고 노력했습니다.

아래는 미션을 진행하며 고민한 내용입니다!
질문이 조큼,,, 많은 것 같아 중요도 태그를 통해 우선순위를 설정해 보았습니다.
참고 부탁드리며, 2주 간 잘 부탁드립니다 😄

질문 중요도 태그 규칙

[🔴 Must]
꼭 답변을 받고 싶은 질문입니다.
리뷰어의 의견이 있어야 다음 방향을 결정할 수 있습니다.

[🟢 Optional]
가능하다면 답변을 받고 싶은 질문입니다.
바쁘시면 나중에 답변해주셔도 괜찮습니다.

[⚪️ Curious]
개인적으로 궁금해 여쭤보는 질문입니다.
답변이 없어도 미션 진행에는 영향이 없습니다.

[🔴 Must][ 1. 장기 기물 설계 방식: Piece 상속 vs 행마법 interface ]

장기 기물을 설계하는 과정에서 Piece를 상속받는 구조로 구현할지, 혹은 행마법을 interface로 분리하여 전략 패턴으로 구성할지에 대해 고민했습니다.

처음에는 각 기물이 Piece를 상속받아 자신의 이동 규칙을 직접 구현하는 방식을 떠올렸습니다.
이 방식은 구조가 직관적이라는 장점이 있습니다. 예를 들어, “차(Chariot)는 기물(Piece)의 한 종류이며 자신의 이동 로직을 가진다”는 관계가 코드에서도 자연스럽게 드러나기 때문에 전체 구조를 이해하기 쉽다고 느꼈습니다.

다만 장기의 이동 규칙을 살펴보면서 일부 기물들이 이동 규칙을 부분적으로 공유한다는 점이 신경 쓰였습니다.
예를 들어,

차(車)와 포(包)는 모두 상하좌우 직진 이동을 합니다.
궁(楚/漢)과 사(士)는 이동 방식이 동일합니다.

이 구조를 그대로 상속 기반으로 구현할 경우,
Chariot(차)와 Cannon(포) 클래스 내부에서 직진 경로를 탐색하는 로직이 중복될 가능성이 있다고 생각했습니다.

따라서 행마법을 MoveStrategy와 같은 인터페이스로 분리하고, 기물이 이를 조합하는 전략 패턴 구조도 구현하였습니다.
이 방식에서는 이동 규칙을 조합 가능한 단위로 분리할 수 있기에, 차와 포처럼 직진 이동이라는 공통 행마를 재사용할 수 있다는 장점이 존재한다고 생각했습니다.

리뷰 요청 포인트

한편으로는 직선 이동과 같은 규칙이 일부 기물에서 동일하게 나타나더라도, 이를 반드시 코드 중복 문제로 봐야 하는지에 대해 고민이 되었습니다.
예를 들어 차와 포는 모두 직선 이동을 하지만, 도메인 관점에서는 각 기물이 독립적인 이동 규칙을 가진다고 볼 수도 있습니다. 또한 차의 이동 규칙이 변경되더라도 포의 규칙이 함께 변경될 가능성은 낮기 때문에, 이를 중복 코드라고 볼 수 있을까에 대한 의문이 들었습니다.

또한 제가 상속을 사용할 때 기준으로 삼는 조건들을 고려했을 때, 장기 기물 관련 상태와 행동들이 상속 사용 조건에 크게 어긋나지는 않는 것 같다는 생각도 들었습니다.

제가 상속을 고려할 수 있다고 생각하는 기준은 다음과 같습니다.

1. 상위 클래스가 상속을 고려해 설계된 경우
예를 들어 abstract 메서드로 확장 포인트를 열어 두고, final 메서드로 공통 로직을 제한하는 등 상속을 전제로 한 구조로 설계된 경우입니다.

2. 상위 클래스의 변경이 하위 클래스에 큰 영향을 주지 않는 경우
상위 클래스에 메서드가 추가되거나 내부 구현이 일부 변경되더라도, 하위 클래스가 super 호출이나 내부 상태에 강하게 의존하지 않아 별도의 수정 없이 안정적으로 동작할 수 있는 구조라고 판단되는 경우입니다.

3. 하위 객체를 부모 타입으로 치환해도 의미가 자연스럽게 유지되는 경우
부모 타입으로 사용했을 때 특정 메서드를 UnsupportedOperationException으로 막아야 하거나, 부모 클래스가 보장한 동작을 깨뜨리지 않는 구조가 아닌 경우입니다.

현재 장기 기물 구조가 위 기준을 크게 위반한다고 느껴지지는 않아서,
각 기물이 Piece를 상속받고 각 클래스에서 행마법을 직접 구현하는 구조도 충분히 가능하지 않을까?라는 생각도 가지고 있습니다.

따라서 베루스께서는 어떤 관점에서 장기 기물에 대해 상속 기반 구조와 전략 패턴 기반 구조 중 어떤 설계를 더 선호하실지 궁금합니다!

[🟢 Optional][ 2. 다형성 활용 기준 ]

다형성을 사용하는 이유가 결국 확장 가능하고 유지보수가 쉬운 구조를 만들기 위함이라고 이해하고 있습니다. 다만 구현을 하다 보니, 아직 발생하지 않은 미래의 확장 가능성까지 고려하다 보면 오버 엔지니어링이 되는 것은 아닐지에 대한 고민도 들었습니다.

특히 “어디까지를 설계로 대비하고, 어디서부터는 고려하지 않고 구현하는 것이 좋은지” 그 적절한 선을 판단하는 것이 어렵다고 느꼈습니다.

리뷰 요청 포인트

다형성을 활용하여 설계할지에 대한 판단을 할 때 어떤 기준을 가지고 결정하시나요?
확장성을 고려한 설계와 오버 엔지니어링 사이의 경계를 어떤 기준으로 판단하시는지 궁금합니다!

코드 관련 질문은 따로 코멘트 남겨두었습니다.
이 외에도 자유롭게 피드백 주시면 감사하겠습니다. 리뷰 잘 부탁드립니다🙇🏻‍♀️

Position 객체 생성 시 행 범위를 벗어난 경우, 열 범위를 벗어난 경우를 검증하는 예외 테스트를 작성 및 로직을 구현했습니다.

- 행과 열 차이 계산 테스트 추가

- 상좌우만 가능 (후진 구현x) - 행마법 테스트 추가

- 보드 초기화 테스트 추가 - 초기화 전략 구현

- 상차림 테스트 추가

- 기존 Piece 테스트 제거

- 차 행마법 테스트 추가 - Position 위치 1칸 이동 기능 추가

- 마 행마법 테스트 추가

- 상 행마법 테스트 추가 - DirectionInformation 값 객체 추가

- 경로 상 기물 없는 조건 확인하는 테스트 추가

- 경로 상 기물이 1개 있는 조건 확인하는 테스트 추가

- 기물 이동 검증 기능 테스트 추가

- 기물 이동 기능 테스트 추가

Jiihyun · 2026-03-28T15:29:07Z

src/main/java/janggi/domain/board/BoardChecker.java

+public interface BoardChecker {
+
+    boolean hasPieceAt(Position position);
+
+    boolean isSameCampPieceAt(Position position, Camp camp);
+
+    boolean hasSamePieceRuleAt(Position position, PieceRule pieceRule);
+}


[🔴 Must] Board와 Piece 의존성 분리 방식에 대한 고민

Piece가 Board에 직접 의존하지 않도록 하기 위해 BoardChecker 인터페이스를 도입했습니다.

Board 구현체는 getter, move 등 piece와 직접적인 연관이 없는 기능에 대한 권한을 갖고 있다고 생각했습니다. 따라서 Piece가 이동을 위해 보드에서 확인해야 하는 기능들만 모아 BoardChecker 인터페이스로 분리하고, Piece는 이 타입을 통해서만 보드를 참조하도록 구성했습니다.

[질문 🙋🏻‍♀️]
Piece가 Board의 필요한 기능에만 의존하도록 제한하려는 의도로 설계한 현재 방식에 대해 어떻게 생각하시는지 궁금합니다!
객체 간 의존성을 줄이기 위한 적절한 추상화라고 보시나요? 아니면 과도한 분리라고 생각하시나요?

현재 구조에서는 런타임에 순환 참조가 발생했기 때문에 안전 장치로 컴파일타임에 조회만 가능한 인터페이스를 분리하여 함부로 내부 상태를 수정하지 못하게 막은것이기 때문에 적절한 선택 같네요. 최고로 좋은건 순환 참조를 제거하는 방향일 것 같은데 요구사항을 적절히 구현하려면 쉽지 않을 것 같네요.

Jiihyun · 2026-03-28T15:41:27Z

src/main/java/janggi/domain/board/InitialPiecePlacement.java

+    public static Map<Position, Piece> init(ElephantSetting hanChoice, ElephantSetting choChoice) {
+        Map<Position, Piece> board = new HashMap<>();
+
+        for (InitialPiecePlacement piece : values()) {
+            board.put(new Position(piece.row, piece.column), new Piece(piece.pieceRule, piece.camp));
+        }
+
+        board.putAll(hanChoice.createElephantOrder(Camp.HAN));
+        board.putAll(choChoice.createElephantOrder(Camp.CHO));
+        return board;
+    }


[🟢 Optional] 나라에 따른 상차림 타입 안정성에 대한 고민

장기에서 상/마 배치 방식을 표현하기 위해 BoardInitializer와 InitialPiecePlacement에서 ElephantSetting을 enum으로 정의하여 사용했습니다.

다만 이 구조에서는 HAN과 CHO의 매개변수 순서가 잘못 섞여 들어가더라도, 타입이 동일하기 때문에 컴파일 단계에서 오류를 발견할 수 없다는 점이 고민이 되었습니다.

이를 해결하기 위해 나라에 따라 타입을 분리하여 타입 안정성을 강화하는 방법도 고려했습니다.
하지만 그렇게 할 경우, 구조가 복잡해지고 일부 중복 코드가 생길 수 있다고 판단해, 현재 enum 구조를 유지하기로 했습니다.

[질문 🙋🏻‍♀️]
이와 같은 상황에서 개발자가 매개변수 순서를 올바르게 사용해야 한다는 전제를 두기보다, 타입 수준에서 더 강하게 제약을 두는 설계가 더 바람직한지 궁금합니다.
아니면 현재처럼 구조의 단순함을 유지하면서, 내부 검증 로직이나 테스트로 보완하는 접근이 더 실용적인지에 대해 베루스의 의견이 궁금합니다!

저라면 구조를 단순하게하고 테스트로 보완할 것 같네요. 테스트 코드를 잘 작성하면 컴파일러가 문법적 오류를 찾아내듯이 도메인 규칙의 오류를 쉽게 찾을 수 있다고 생각합니다~

오!! 테스트를 활용할 수 있다는 생각을 하지 못했네요.
앞으로 비슷한 상황이 생긴다면 구조를 더 복잡하게 만들기보다는, 테스트 코드를 더 꼼꼼히 작성하는 방향을 먼저 고민해보겠습니다!

Jiihyun · 2026-03-28T15:50:43Z

src/main/java/janggi/domain/board/StandardBoardInitializer.java

+    @Override
+    public Map<Position, Piece> initialize() {
+        return InitialPiecePlacement.init(hanElephantSetting, choElephantSetting);
+    }


[⚪️ Curious] BoardInitializer 추상화의 필요성에 대한 고민

현재 Board 클래스는 BoardInitializer를 주입받아 내부에서 장기판을 초기화하는 방식으로 구현했습니다.
BoardInitializer는 장기판 초기화를 담당하는 인터페이스로, 특히 테스트 코드에서 기물 위치 이동하는 메서드 호출 없이 원하는 장기판 상태를 쉽게 구성할 수 있도록 하기 위해 도입했습니다.

이후 실제 게임 로직을 구현하면서 초기 배치 규칙을 관리하는 InitialPiecePlacement를 별도로 만들게 되었고, StandardBoardInitializer는 장기판의 초기 기물 배치를 직접 작성하기보다는 InitialPiecePlacement를 호출하는 역할을 하도록 구성했습니다.
이로 인해 StandardBoardInitializer가 실질적인 로직을 수행하기보다는 InitialPiecePlacement를 호출하는 단순 위임 역할에 가까운 상태가 되었습니다.

[질문 🙋🏻‍♀️]

현재처럼 Board 초기화를 위한 인터페이스(BoardInitializer)를 유지하는 것이 적절한 설계일까요?
아니면BoardInitializer를 제거하고 Board의 생성자 매개변수로 Map을 전달하는 방식이 더 단순하고 명확한 구조일까요?

아직 Board를 초기화하는 로직이 복잡하지 않기 때문에 별도로 인터페이스까지 만들어가며 구현할 필요는 없을 것 같습니다. Board에서 Map으로 전달 받는 방식이 더 깔끔할 것 같네요.

동의합니다!
현재 BoardInitializer도 특별한 역할 없이 Map을 감싸고만 있기 때문에, 인터페이스를 제거하여 복잡도를 낮추고 Board는 Map을 전달 받도록 리팩토링 하였습니다.

[질문 🙋🏻‍♀️]
다만 Map을 Board 생성 시점에 직접 주입받게 되면, 내부에서 사용하는 자료구조가 외부에 드러나 캡슐화가 약해지는 것은 아닌지 궁금합니다!!

BoardInitializer를 결국 선택한 이유도 여기에 있었습니다. 만약 보드의 내부 자료구조가 변경되더라도 외부에는 BoardInitializer만 노출되기 때문에, 변경이 발생하더라도 해당 클래스만 수정하면 되겠다고 생각했기 때문입니다.

Jiihyun · 2026-03-28T16:09:24Z

src/main/java/janggi/domain/piece/condition/MoveCondition.java

+
+public interface MoveCondition {
+
+    void checkPath(List<Position> path, Camp camp, BoardChecker board, PieceRule pieceRule);


[🔴 Must] 공통 인터페이스와 특정 구현체 전용 인자에 대한 고민 (MoveCondition의 PieceRule 인자)

MoveCondition 인터페이스의 구현체는 EmptyCondition, OnePieceExistsCondition 두 가지가 있습니다.

EmptyCondition에서는 PieceRule을 사용하지 않습니다.

OnePieceExistsCondition에서는 같은 종류의 기물인지 검사하기 위해 PieceRule이 필요합니다.

처음에는 인터페이스에 공통으로 사용하는 매개변수만 유지하고 싶어, 특정 구현체에서만 사용하는 인자를 제거하는 방향을 고민했습니다.
PieceRule을 인터페이스 메서드의 인자에서 제거하고, OnePieceExistsCondition이 내부 필드로 PieceRule을 가지도록 하는 방식입니다.

다만 이 경우 OnePieceExistsCondition을 생성할 때 PieceRule을 함께 주입해야 하는데, 현재 구조에서는 enum 초기화 시점 문제로 자기 자신을 인자로 넘기는 형태를 만들기 어려웠습니다.

[질문 🙋🏻‍♀️]
공통 인터페이스를 설계하다 보면 일부 구현체에서만 사용하는 인자가 생기는 경우가 있는데, 이런 상황을 어떻게 판단하고 해결하시나요?

구조 자체에는 문제가 없다고 판단되더라도 일부 구현체에서만 사용하는 매개변수라면 인터페이스에서 제거하는 것이 맞는지,
일부 구현체만 사용하더라도 인터페이스 메서드의 매개변수로 두는 것이 전체 구조상 장점이 더 크다고 판단했기에 그대로 유지하는게 좋은지 궁금합니다!

가끔 인터페이스나 추상 클래스로 상위 개념을 묶다보면 지금처럼 특정 케이스에만 필요한 매개변수가 생기기도 합니다. 개인의 판단에 따라 유지할지 말지를 정할텐데, 저는 최대한 불필요한 매개변수를 추가하지 않는 방향으로 고민해보긴 합니다. 개인적으로는 현재는 구조를 개선해 해당 문제를 해결할 수 있지 않을까 싶습니다. 다른 곳에 피드백을 남겨뒀지만 MoveCondition과 MoveStrategy를 합치고 PieceRule이라는 enum이 아닌 Piece를 상속받은 하위 클래스로 구현하면 되지 않을까 싶습니다.

상속 구조를 더 추천하시는 주된 이유나, 상속을 통해 얻을 수 있는 설계적 이점에 대해서 조금 더 조언을 구할 수 있을까요?
상속을 사용하면 piece 클래스에 camp와 pieceRule를 놓고 쓸 수 있기 때문일까요??!

저는 장기는 기물의 종류와 각 기물의 행마법이 변하지 않는 정적인 도메인이기 때문에, Enum을 활용하여 "어떤 타입은 어떤 전략을 가진다"를 미리 세팅해 두면 관리 포인트가 한 곳으로 모아져 좋다고 생각했습니다!

Jiihyun · 2026-03-28T16:23:12Z

src/main/java/janggi/dto/CampDto.java

+public record CampDto(String camp) {
+
+    public static CampDto from(Camp camp) {
+        return new CampDto(CampFormatter.format(camp));
+    }


[⚪️ Curious] DTO의 역할에 대한 고민

DTO가 단순히 하나의 값을 감싸는 구조라면, 과연 DTO를 유지하는 것이 의미 있는 설계인지에 대한 고민이 들었습니다.

DTO는 보통 Domain과 View 사이에서 완충 역할(연골 같은 역할)을 한다고 이해하고 있습니다.
View의 요구사항이 변경되더라도 DTO만 수정하면 되어 Domain까지 변경의 영향이 전파되는 것을 막아줄 수 있기에, 두 계층 간 결합도를 느슨하게 유지할 수 있다는 장점이 있다고 생각했습니다.

다만 현재 상황에서는 View에서 Camp 도메인의 값 하나만 그대로 필요로 하는 상태라서, DTO가 실질적인 역할을 하기보다는 단순히 값을 한 번 더 감싸는 구조가 되어 불필요한 클래스만 추가되고 복잡도가 증가한 것은 아닐지 고민이 되었습니다.

[질문 🙋🏻‍♀️]

View에서 Domain 값 하나만 그대로 사용하는 경우에도 계층 분리를 위해 DTO를 유지하는 것이 좋은지,
아니면 현재 단계에서는 DTO 없이 Domain 값을 직접 전달하는 구조가 더 단순하고 적절한 선택인지 궁금합니다.

Piece에서도 DTO를 사용하고 있는 상황이라, 현재는 CampDto의 역할이 단순하더라도 일관성을 위해 DTO를 유지하는 것이 나은지도 고민이 됩니다.
한편으로는 지금은 값이 하나뿐이지만, 이후 View에서 필요한 값이 늘어나 DTO가 자연스럽게 확장될 가능성도 있지 않을까라는 생각도 들었습니다.

이처럼 현재의 단순함과 미래 확장 가능성 사이에서 DTO를 유지할지 판단하는 기준이 있다면 조언 부탁드립니다!

DTO는 뷰 계층인가요? 도메인 계층인가요? 아니면 두개의 계층에 포함되지 않은 회색지대인가요? 개인적으로 DTO 자체를 레벨1에서는 사용할 필요가 없다고 생각합니다. 인프라가 복잡하지 않은 콘솔 애플리케이션에서는 DTO를 둘 만큼 데이터 전송이 복잡하지 않아서요. 심플하게 도메인을 전달해도 된다고 생각해요. GPT에게 한번 DTO를 사용하는 이유가 무엇인지 그리고 간단한 콘솔 애플리케이션에서 필요할지를 물어보면 좋을 것 같아요~

DTO는 뷰와 도메인 사이의 결합을 느슨하게 유지하고, 뷰 변경이 도메인까지 영향을 미치지 않도록 보호하는 역할을 합니다.

보드를 출력할 때도 도메인의 모든 값을 전달하기보다는 필요한 값만 전달하기 위해 DTO를 사용했습니다. 다만 camp는 값이 그대로 전달되어도 되지만, 구조적 통일성을 위해 DTO를 사용했었습니다.

하지만 단순히 통일성을 맞추기 위해 Camp → CampDto와 같은 불필요한 변환 로직을 추가하면, 코드가 늘어나고 유지보수가 복잡해져 장점보다는 단점이 더 클 것 같네요!!
따라서 CampDto를 제거하였습니다!!

Jiihyun · 2026-03-28T17:43:31Z

src/main/java/janggi/domain/board/InitialPiecePlacement.java

+import janggi.domain.piece.PieceRule;
+import java.util.HashMap;
+import java.util.Map;
+


[⚪️ Curious] 초기 장기 기물 배치 데이터 주입 방식: enum vs CSV

현재 장기의 초기 기물 배치를 표현하기 위해 InitialPiecePlacement enum 내부에 각 기물의 초기 위치 정보를 상수로 정의해 두었습니다.

초기 배치 데이터를 외부 CSV 파일로 분리하여 읽어오는 방식도 고려했습니다. 다만 두 방식을 비교하면서 다음과 같은 관점에서 고민하게 되었습니다.

1. 오류 발견 시점
만약 데이터에 SOLDIER를 SOLDER로 잘못 작성하거나 좌표 범위를 벗어난 값(99)을 입력하여 오타가 발생한다면, enum 방식의 경우 타입 기반으로 관리되기 때문에 컴파일 타임에 오류를 발견할 가능성이 높습니다.
반면 CSV 방식은 문자열 기반 데이터이기 때문에 런타임에 파싱하는 과정에서야 오류가 드러날 가능성이 있습니다.

따라서 이 차이가 개발 과정에서 오류를 더 빠르게 발견할 수 있는지라는 측면에서 의미가 있다고 판단했습니다.

2. 유연성과 데이터 변경 빈도
enum 방식은 코드에 직접 정의되는 만큼 데이터 변경 시 코드 수정과 재배포가 필요하다는 단점이 있습니다. 반면 CSV 방식은 외부 파일만 수정하면 되기 때문에 유연하게 변경할 수 있다는 장점이 있습니다.
다만 장기 게임의 규칙을 고려했을 때, 대부분의 기물 초기 배치는 수백 년 동안 거의 변하지 않은 고정된 규칙에 가깝다고 생각했습니다.

따라서 실제로는 데이터가 자주 변경될 가능성이 낮은 데이터라고 판단했습니다.

이러한 특성을 고려했을 때, 유연성 측면에서는 enum이 다소 불리할 수 있지만 컴파일 타임 안정성, 타입 안전성, IDE 지원 등의 장점이 더 크다고 판단하여 enum 방식을 선택했습니다.

[질문 🙋🏻‍♀️]
초기 기물 배치 데이터를 enum으로 관리하는 방식과 외부 데이터(CSV 등)로 분리하는 방식, 혹은 그 외의 방식 중 어떤 방식이 좋다고 생각하시는지 궁금합니다!

러키의 생각과 동일하게 CSV로 저장하는건 과하다고 생각합니다. enum으로 관리해도 충분할 것 같아요.

verus-j

안녕하세요 러키~ 리뷰어 베루스입니다.
먼저 테스트 케이스와 연관된 피드백 남겨드렸습니다. 확인부탁드려요~
피드백 반영 후 리뷰 요청 다시 주시면 프로덕션 코드 위주로 피드백 남겨드릴게요.
남겨주신 질문은 프로덕션 코드 리뷰 때 답변드리겠습니다~

verus-j · 2026-03-30T03:23:50Z

src/main/java/janggi/domain/board/ElephantSetting.java

+    public static ElephantSetting findElephantSettingBy(String command) {
+        return Arrays.stream(values())
+                .filter(element -> element.command.equals(command))
+                .findFirst()
+                .orElseThrow(() -> new IllegalArgumentException(ExceptionMessage.INVALID_ELEPHANT_SETTING.getMessage()));
+    }


ElephantSetting을 찾는 테스트 케이스가 없습니다~

반영하였습니다!!
test: ElephantSetting 테스트 추가

verus-j · 2026-03-30T03:24:37Z

src/main/java/janggi/domain/piece/strategy/DirectionInformation.java

+    public int calculateColDirection() {
+        if (colDifference == 0) {
+            return 0;
+        }
+        return colDifference / Math.abs(colDifference);
+    }


colDifference가 0인 테스트 케이스가 없습니다~

difference 값에 따른 메서드 결과를 확인하는 테스트를 추가하여 검증하였습니다!

test: DirectionInformation 테스트 추가

verus-j · 2026-03-30T03:25:21Z

src/main/java/janggi/domain/piece/strategy/HorseStrategy.java

+        if ((absRowDifference != MIN_ABS_DELTA || absColDifference != MAX_ABS_DELTA)
+                && (absRowDifference != MAX_ABS_DELTA || absColDifference != MIN_ABS_DELTA)) {
+            throw new IllegalArgumentException(ExceptionMessage.INVALID_HORSE_MOVE.getMessage());
+        }


absColDifference != MIN_ABS_DELTA 만 true인 테스트 케이스가 없습니다~

오 그렇네요,,반영하였습니다!

test: HorseStrategy 테스트 추가

verus-j · 2026-03-30T03:25:48Z

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

+    public Camp next() {
+        if (this == CHO) {
+            return HAN;
+        }
+        return CHO;
+    }


Camp를 전환하는 테스트 케이스가 없습니다~

반영하였습니다!

test: Camp 테스트 추가

verus-j · 2026-03-30T03:29:44Z

src/test/java/janggi/domain/board/InitialPiecePlacementTest.java

+    @Test
+    void 한_상마상마_초_마상마상_으로_보드를_초기화한다() {
+        // given
+        BoardInitializer initializer = new StandardBoardInitializer(
+                ElephantSetting.LEFT_ELEPHANT, ElephantSetting.RIGHT_ELEPHANT);
+        Map<Position, Piece> expectedBoard = createExpectedBoard();
+        expectedBoard.putAll(createChoLeftHanRightBoard());
+        // when
+        Map<Position, Piece> board = initializer.initialize();
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(board).hasSize(32);
+            assertSoftly.assertThat(board).isEqualTo(expectedBoard);
+        });
+    }


상과 마의 세팅을 테스트하는 케이스인데 상/마 세팅을 하는 createChoLefHanRightBoard는 테스트 코드 안에서 구현된 로직이네요. StandardBoardInitializerTest는 테스트 코드를 검증하기 위한 테스트 케이스일까요?

해당 테스트는 테스트 코드를 검증하기 위한 케이스라기보다는, 프로덕션 코드인 initializer.initialize()의 결과가 제가 기대한 값과 일치하는지를 확인하기 위한 의도로 작성했습니다. 그래서 기대값을 createChoLeftHanRightBoard()와 같은 메서드를 통해 생성하도록 구성했습니다.

[질문 🙋🏻‍♀️]
기대값을 별도의 메서드로 구성하기보다는,
테스트 코드에서 직접 하드코딩 형태로 명시적으로 작성하는 것이 더 적절하다는 의미에서 남겨주신 리뷰일까요?

아 비교를 위해서 expectedBoard에 직접 넣어준거군요. 테스트 코드를 잘못 봤네요~

verus-j · 2026-03-30T03:39:23Z

src/test/java/janggi/domain/piece/strategy/MultiStepStraightStrategyTest.java

+
+    @ParameterizedTest
+    @MethodSource("createPositionsAndPath")
+    void 차와_포는_한_방향으로만_1칸_이상_이동_할_수_있다(Position source, Position destination, int size, List<Position> expectedPath) {


MultiStepStraightStrategy는 말그대로 직선으로 여러칸 이동하는 것과 연관된 로우한 레벨의 클래스 같습니다. 차와 포라는 추상화된 개념을 알 필요가 없을 것 같아요. 테스트 케이스 명을 테스트 대상자와 동일한 추상화 수준으로 수정하면 좋을 것 같습니다~

MultiStepStraightStrategy는 말씀해주신 것처럼 직선으로 여러 칸 이동하는 규칙만을 담당하도록 추상화하였기에, 테스트 명에 포&차와 같은 구체적인 기물 명이 들어간다면 추상화 수준이 맞지 않게 되는군요!!

MultiStepStraightStrategTest, 그리고 SingleStepStraightStrategyTest 도 함께 동일한 추상화 수준이 되도록 메서드명 변경하였습니다!

refactor: MultiStepStraightStrategyTest, SingleStepStraightStrategyTe…

verus-j · 2026-03-30T03:40:11Z

src/test/java/janggi/domain/piece/strategy/SingleStepStraightStrategyTest.java

+
+    @ParameterizedTest
+    @MethodSource("successMovePositions")
+    void 궁과_사는_상하좌우_1칸_이동한다(Position source, Position destination) {


여기도 좀 더 추상화된 개념이 아닌 로우한 레벨의 SingleStepStraightStrategy에만 집중하는 테스트 케이스 명으로 수정하면 좋을 것 같아요~

verus-j · 2026-03-30T03:44:39Z

src/test/java/janggi/domain/piece/strategy/SoldierStrategyTest.java

+
+class SoldierStrategyTest {
+
+    private final MoveStrategy strategy = new SoldierStrategy();


Strategy와 연관된 테스트코드들을 계속 살펴보니 추상화 수준이 다들 다른 것 같습니다. 상, 마, 병에 대한 높은 추상화 수준의 Strategy와 추상화 수준이 조금 낮은 단순 직선 혹은 한칸 이동에 대한 Strategy가 같은 추상화 수준으로 다루다보니 어색한 생각이 드네요. Strategy와 연관된 추상화 수준을 동일하게 맞추는 것도 좋을 것 같아요~

verus-j · 2026-03-30T03:46:45Z

src/test/java/janggi/domain/TurnTest.java

+    @Test
+    void 초나라_차례가_끝나면_한나라_차례가_된다() {
+        // given
+        Turn turn = new Turn();
+        Camp firstTurn = turn.currentTurn();
+        // when
+        turn.finishTurn();
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(firstTurn).isEqualTo(Camp.CHO);
+            assertSoftly.assertThat(turn.currentTurn()).isEqualTo(Camp.HAN);
+        });
+    }


테스트 케이스 명과 실제 테스트 코드가 다른 것 같네요~

해당 테스트에서 finishTurn() 호출 후 차례가 CHO → HAN으로 변경되는 흐름을 검증하려는 목적이었고,
이를 확인하기 위해 처음 turn 진행 시에는 현재 차례가 CHO이고, 이후 turn이 끝나면 현재 차례가 HAN이 되는지 함께 검증하도록 작성했습니다.

따라서 저는 테스트명이 실제 코드를 잘 표현한다고 생각하는데,
어느부분에서 테스트명과 실제 테스트 코드가 다르다고 느끼셨는지 조금 더 자세히 말씀해 주실 수 있을까요?!

아 제가 잘못 봤었네요! 테스트 명에 맞게 잘 작성하셨습니다.

verus-j · 2026-03-30T03:47:47Z

src/test/java/janggi/domain/piece/PieceTest.java

+class PieceTest {
+
+    @Test
+    void 같은_룰이_적용되는_기물인지_확인한다() {


같은 룰이라는 표현이 애매한 것 같습니다. 포에만 해당하는 내용이다보니 좀 더 포와 연관된 테스트 케이스로 이름을 수정하면 좋을 것 같아요.

해당 테스트는 포(CANNON)에 맞춰진 동작을 검증하기보다는, 동일한 PieceRule을 가지는 기물인지 확인하는 메서드의 동작을 검증하는 것이 목적이었습니다.
따라서 포는 단순히 예시로 사용된 것이기에, 특정 기물에 종속된 의미보다는 메서드의 역할이 드러나도록 조금 더 추상화된 테스트 케이스 명으로 변경했습니다! (같은_전략_및_이동_조건이_적용되는_기물인지_확인한다())

…st 테스트 명 추상화

Jiihyun

리뷰 감사합니다 베루스!!

TDD를 진행하며 테스트 코드를 충분히 추가했다고 생각했는데, 놓친 부분이 꽤 있었네요..
말씀해주신 부분은 보완하여 반영했습니다.

이번 리뷰를 통해 테스트 코드에서도 추상화 수준에 맞는 메서드 이름 짓는 것과 중복 코드를 줄이는 방법에 대해 고민해볼 수 있었습니다. 테스트 코드 역시 프로덕션 코드만큼이나 유지보수가 중요하고 살아있는 문서로서의 역할을 하기 때문에, 더욱 신경 써야 한다는 점을 배울 수 있었습니다!

행마법 전략마다 추상화 수준이 다른 부분(관련 코멘트 2개)은,,,조금 더 생각이 필요하여, 테스트 코드 관련 피드백 먼저 반영 후 리뷰 요청 드렸습니다!!

Jiihyun · 2026-03-30T08:31:10Z

src/test/java/janggi/domain/board/InitialPiecePlacementTest.java

+    @Test
+    void 한_상마상마_초_마상마상_으로_보드를_초기화한다() {
+        // given
+        BoardInitializer initializer = new StandardBoardInitializer(
+                ElephantSetting.LEFT_ELEPHANT, ElephantSetting.RIGHT_ELEPHANT);
+        Map<Position, Piece> expectedBoard = createExpectedBoard();
+        expectedBoard.putAll(createChoLeftHanRightBoard());
+        // when
+        Map<Position, Piece> board = initializer.initialize();
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(board).hasSize(32);
+            assertSoftly.assertThat(board).isEqualTo(expectedBoard);
+        });
+    }


해당 테스트는 테스트 코드를 검증하기 위한 케이스라기보다는, 프로덕션 코드인 initializer.initialize()의 결과가 제가 기대한 값과 일치하는지를 확인하기 위한 의도로 작성했습니다. 그래서 기대값을 createChoLeftHanRightBoard()와 같은 메서드를 통해 생성하도록 구성했습니다.

[질문 🙋🏻‍♀️]
기대값을 별도의 메서드로 구성하기보다는,
테스트 코드에서 직접 하드코딩 형태로 명시적으로 작성하는 것이 더 적절하다는 의미에서 남겨주신 리뷰일까요?

Jiihyun · 2026-03-30T09:22:33Z

src/test/java/janggi/domain/piece/condition/EmptyConditionTest.java

+    @Test
+    void 이동하려는_경로에_기물이_존재하면_예외가_발생한다() {
+        //given
+        List<Position> path = List.of(
+                new Position(0, 0),
+                new Position(0, 1),
+                new Position(0, 2),
+                new Position(0, 3),
+                new Position(0, 4),
+                new Position(0, 5)
+        );
+        Camp camp = Camp.HAN;
+
+        BoardInitializer boardInitializer = new EmptyConditionTestBoardInitializer();
+        Board board = new Board(boardInitializer);
+        //when & then
+        assertThatThrownBy(() -> condition.checkPath(path, camp, board, PieceRule.CHARIOT))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage(ExceptionMessage.PATH_NOT_EMPTY.getMessage());
+    }


동의합니다!
기존에는 EmptyConditionTestBoardInitializer라는 테스트 전용 클래스를 사용하고 있었고, 보드 데이터가 해당 클래스 내부에 정의되어 있어 테스트 코드만 봐서는 경로에 기물이 존재하는 상황인지 바로 파악하기 어려웠던 것 같습니다.

따라서 해당 클래스를 제거하고, 람다를 사용해 보드 초기 상태를 테스트 코드에서 직접 정의하도록 변경했습니다.
이렇게 하니 경로 중간에 기물이 배치된 상황이 테스트 코드에서 바로 드러나 조금 더 읽기 쉬워진 것 같네요!!

Jiihyun · 2026-03-30T12:03:18Z

src/test/java/janggi/domain/piece/strategy/ElephantStrategyTest.java

+        List<Position> path = strategy.findPath(source, destination, Camp.HAN);
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(path).hasSize(expectedPath.size());
+            assertSoftly.assertThat(path).containsExactlyElementsOf(expectedPath);
+        });


그렇네요!

destination에 대한 정보가 expectedPath 마지막에 위치해 있으므로
destination을 expectedPath의 마지막 값으로 가져오는 방식이 중복 코드도 제거하고, 데이터도 일관되게 유지할 수 있을 것 같습니다!

관련하여 MultiStepStraightStrategyTest, HorseStrategyTest도 중복된 정보를 넘기지 않도록 수정하였습니다!

Jiihyun · 2026-03-30T12:47:20Z

src/test/java/janggi/domain/piece/strategy/MultiStepStraightStrategyTest.java

+
+    @ParameterizedTest
+    @MethodSource("createPositionsAndPath")
+    void 차와_포는_한_방향으로만_1칸_이상_이동_할_수_있다(Position source, Position destination, int size, List<Position> expectedPath) {


MultiStepStraightStrategy는 말씀해주신 것처럼 직선으로 여러 칸 이동하는 규칙만을 담당하도록 추상화하였기에, 테스트 명에 포&차와 같은 구체적인 기물 명이 들어간다면 추상화 수준이 맞지 않게 되는군요!!

MultiStepStraightStrategTest, 그리고 SingleStepStraightStrategyTest 도 함께 동일한 추상화 수준이 되도록 메서드명 변경하였습니다!

refactor: MultiStepStraightStrategyTest, SingleStepStraightStrategyTe…

Jiihyun · 2026-03-30T13:03:12Z

src/test/java/janggi/domain/TurnTest.java

+    @Test
+    void 초나라_차례가_끝나면_한나라_차례가_된다() {
+        // given
+        Turn turn = new Turn();
+        Camp firstTurn = turn.currentTurn();
+        // when
+        turn.finishTurn();
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(firstTurn).isEqualTo(Camp.CHO);
+            assertSoftly.assertThat(turn.currentTurn()).isEqualTo(Camp.HAN);
+        });
+    }


해당 테스트에서 finishTurn() 호출 후 차례가 CHO → HAN으로 변경되는 흐름을 검증하려는 목적이었고,
이를 확인하기 위해 처음 turn 진행 시에는 현재 차례가 CHO이고, 이후 turn이 끝나면 현재 차례가 HAN이 되는지 함께 검증하도록 작성했습니다.

따라서 저는 테스트명이 실제 코드를 잘 표현한다고 생각하는데,
어느부분에서 테스트명과 실제 테스트 코드가 다르다고 느끼셨는지 조금 더 자세히 말씀해 주실 수 있을까요?!

Jiihyun · 2026-03-30T13:10:42Z

src/test/java/janggi/domain/piece/PieceTest.java

+class PieceTest {
+
+    @Test
+    void 같은_룰이_적용되는_기물인지_확인한다() {


해당 테스트는 포(CANNON)에 맞춰진 동작을 검증하기보다는, 동일한 PieceRule을 가지는 기물인지 확인하는 메서드의 동작을 검증하는 것이 목적이었습니다.
따라서 포는 단순히 예시로 사용된 것이기에, 특정 기물에 종속된 의미보다는 메서드의 역할이 드러나도록 조금 더 추상화된 테스트 케이스 명으로 변경했습니다! (같은_전략_및_이동_조건이_적용되는_기물인지_확인한다())

Jiihyun · 2026-03-30T13:13:58Z

src/main/java/janggi/domain/board/ElephantSetting.java

+    public static ElephantSetting findElephantSettingBy(String command) {
+        return Arrays.stream(values())
+                .filter(element -> element.command.equals(command))
+                .findFirst()
+                .orElseThrow(() -> new IllegalArgumentException(ExceptionMessage.INVALID_ELEPHANT_SETTING.getMessage()));
+    }


반영하였습니다!!
test: ElephantSetting 테스트 추가

Jiihyun · 2026-03-30T13:15:37Z

src/main/java/janggi/domain/piece/strategy/DirectionInformation.java

+    public int calculateColDirection() {
+        if (colDifference == 0) {
+            return 0;
+        }
+        return colDifference / Math.abs(colDifference);
+    }


difference 값에 따른 메서드 결과를 확인하는 테스트를 추가하여 검증하였습니다!

test: DirectionInformation 테스트 추가

Jiihyun · 2026-03-30T13:16:01Z

src/main/java/janggi/domain/piece/strategy/HorseStrategy.java

+        if ((absRowDifference != MIN_ABS_DELTA || absColDifference != MAX_ABS_DELTA)
+                && (absRowDifference != MAX_ABS_DELTA || absColDifference != MIN_ABS_DELTA)) {
+            throw new IllegalArgumentException(ExceptionMessage.INVALID_HORSE_MOVE.getMessage());
+        }


오 그렇네요,,반영하였습니다!

test: HorseStrategy 테스트 추가

Jiihyun · 2026-03-30T13:16:29Z

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

+    public Camp next() {
+        if (this == CHO) {
+            return HAN;
+        }
+        return CHO;
+    }


반영하였습니다!

test: Camp 테스트 추가

verus-j

안녕하세요 러키~
테스트 코드 쪽 피드백을 잘 반영해주셨네요. 프로덕션 코드 위주로 피드백 남겨드렸으니 확인부탁드려요~

verus-j · 2026-03-31T13:36:24Z

src/main/java/janggi/domain/Position.java

+    public static Position from(List<Integer> rawPosition) {
+        validatePositionSize(rawPosition);
+        return new Position(rawPosition.get(0), rawPosition.get(1));
+    }
+
+    private static void validatePositionSize(List<Integer> position) {
+        if (position.size() != POSITION_SIZE) {
+            throw new IllegalArgumentException(ExceptionMessage.INVALID_INPUT_FORMAT.getMessage());
+        }
+    }


Position을 생성할 때 size를 검증하는 테스트가 없네요~

추가하였습니다!!
refactor: Position format 테스트 추가

정상적으로 생성하는 테스트 케이스는 없네요~

verus-j · 2026-03-31T13:39:21Z

src/main/java/janggi/domain/piece/strategy/ElephantStrategy.java

+
+    private void validateElephantMovement(DirectionInformation directionInfo) {
+        if ((directionInfo.calculateAbsRowDifference() != MIN_ABS_DELTA
+                || directionInfo.calculateAbsColDifference() != MAX_ABS_DELTA)


directionInfo.calculateAbsColDiffernce() != MAX_ABS_DELTA가 false인 테스트 케이스가 없네요~

verus-j · 2026-03-31T13:55:59Z

src/main/java/janggi/domain/board/InitialPiecePlacement.java

+    public void placeOn(Map<Position, Piece> board) {
+        board.put(position, piece);
+    }


출력 인자에 대해 아시나요? 클린 코드 관점에서 출력 인자가 어떤 문제점을 가지고 있는지 학습해보면 좋을 것 같습니다~

[ 질문 🙋🏻‍♀️]
출력 인자의 문제점이 현재 board를 인자로 받아 board를 수정하기 때문에 직관적이지 않고 어색하게 느껴지는 것일까요??

InitialPiecePlacement의 값들을 가져올 때 게터를 사용하지 않으려고 위와 같은 형태가 되었는데, 아래와 같이 리팩토링 하여 해결했습니다!

public static Board initialize(ElephantFormation hanFormation, ElephantFormation choFormation) { Map<Position, Piece> board = new HashMap<>(); for (InitialPiecePlacement placement : values()) { board.put(placement.position, placement.piece); } board.putAll(hanFormation.placeElephantSetUpPieces()); board.putAll(choFormation.placeElephantSetUpPieces()); return new Board(board); }

출력 인자를 사용하면 인자로 전달하는 모든 메서드의 내부를 하나하나를 확인하며 변경점을 찾아야하니 코드 분석이 어려워질 수 있습니다. 잘 수정하셨어요~

verus-j · 2026-03-31T13:57:21Z

src/main/java/janggi/domain/board/InitialPiecePlacement.java

+import janggi.domain.piece.PieceRule;
+import java.util.HashMap;
+import java.util.Map;
+


러키의 생각과 동일하게 CSV로 저장하는건 과하다고 생각합니다. enum으로 관리해도 충분할 것 같아요.

verus-j · 2026-03-31T13:59:59Z

src/main/java/janggi/domain/board/StandardBoardInitializer.java

+    @Override
+    public Map<Position, Piece> initialize() {
+        return InitialPiecePlacement.init(hanElephantSetting, choElephantSetting);
+    }


아직 Board를 초기화하는 로직이 복잡하지 않기 때문에 별도로 인터페이스까지 만들어가며 구현할 필요는 없을 것 같습니다. Board에서 Map으로 전달 받는 방식이 더 깔끔할 것 같네요.

verus-j · 2026-03-31T14:49:29Z

src/main/java/janggi/domain/board/InitialPiecePlacement.java

+    public static Map<Position, Piece> init(ElephantSetting hanChoice, ElephantSetting choChoice) {
+        Map<Position, Piece> board = new HashMap<>();
+
+        for (InitialPiecePlacement piece : values()) {
+            board.put(new Position(piece.row, piece.column), new Piece(piece.pieceRule, piece.camp));
+        }
+
+        board.putAll(hanChoice.createElephantOrder(Camp.HAN));
+        board.putAll(choChoice.createElephantOrder(Camp.CHO));
+        return board;
+    }


저라면 구조를 단순하게하고 테스트로 보완할 것 같네요. 테스트 코드를 잘 작성하면 컴파일러가 문법적 오류를 찾아내듯이 도메인 규칙의 오류를 쉽게 찾을 수 있다고 생각합니다~

verus-j · 2026-03-31T14:53:56Z

src/test/java/janggi/domain/TurnTest.java

+    @Test
+    void 초나라_차례가_끝나면_한나라_차례가_된다() {
+        // given
+        Turn turn = new Turn();
+        Camp firstTurn = turn.currentTurn();
+        // when
+        turn.finishTurn();
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(firstTurn).isEqualTo(Camp.CHO);
+            assertSoftly.assertThat(turn.currentTurn()).isEqualTo(Camp.HAN);
+        });
+    }


아 제가 잘못 봤었네요! 테스트 명에 맞게 잘 작성하셨습니다.

verus-j · 2026-03-31T14:56:30Z

src/test/java/janggi/domain/board/InitialPiecePlacementTest.java

+    @Test
+    void 한_상마상마_초_마상마상_으로_보드를_초기화한다() {
+        // given
+        BoardInitializer initializer = new StandardBoardInitializer(
+                ElephantSetting.LEFT_ELEPHANT, ElephantSetting.RIGHT_ELEPHANT);
+        Map<Position, Piece> expectedBoard = createExpectedBoard();
+        expectedBoard.putAll(createChoLeftHanRightBoard());
+        // when
+        Map<Position, Piece> board = initializer.initialize();
+        // then
+        SoftAssertions.assertSoftly(assertSoftly -> {
+            assertSoftly.assertThat(board).hasSize(32);
+            assertSoftly.assertThat(board).isEqualTo(expectedBoard);
+        });
+    }


아 비교를 위해서 expectedBoard에 직접 넣어준거군요. 테스트 코드를 잘못 봤네요~

verus-j · 2026-03-31T15:02:27Z

src/main/java/janggi/dto/CampDto.java

+public record CampDto(String camp) {
+
+    public static CampDto from(Camp camp) {
+        return new CampDto(CampFormatter.format(camp));
+    }


DTO는 뷰 계층인가요? 도메인 계층인가요? 아니면 두개의 계층에 포함되지 않은 회색지대인가요? 개인적으로 DTO 자체를 레벨1에서는 사용할 필요가 없다고 생각합니다. 인프라가 복잡하지 않은 콘솔 애플리케이션에서는 DTO를 둘 만큼 데이터 전송이 복잡하지 않아서요. 심플하게 도메인을 전달해도 된다고 생각해요. GPT에게 한번 DTO를 사용하는 이유가 무엇인지 그리고 간단한 콘솔 애플리케이션에서 필요할지를 물어보면 좋을 것 같아요~

verus-j · 2026-03-31T15:08:18Z

src/main/java/janggi/domain/piece/condition/MoveCondition.java

+
+public interface MoveCondition {
+
+    void checkPath(List<Position> path, Camp camp, BoardChecker board, PieceRule pieceRule);


가끔 인터페이스나 추상 클래스로 상위 개념을 묶다보면 지금처럼 특정 케이스에만 필요한 매개변수가 생기기도 합니다. 개인의 판단에 따라 유지할지 말지를 정할텐데, 저는 최대한 불필요한 매개변수를 추가하지 않는 방향으로 고민해보긴 합니다. 개인적으로는 현재는 구조를 개선해 해당 문제를 해결할 수 있지 않을까 싶습니다. 다른 곳에 피드백을 남겨뒀지만 MoveCondition과 MoveStrategy를 합치고 PieceRule이라는 enum이 아닌 Piece를 상속받은 하위 클래스로 구현하면 되지 않을까 싶습니다.

Jiihyun

안녕하세요 베루스!
바쁘신 와중에도 꼼꼼하게 리뷰해 주셔서 감사합니다.

리뷰를 받고 추상화 구조를 여러 방식으로 시도해보았습니다.
리뷰 덕분에 로직이 이전보다 깔끔해졌지만, 앞으로 새로운 기물이 추가되거나 요구사항이 변경될 때 OCP가 잘 지켜질 수 있을지는 여전히 고민되는 부분입니다.

추상화 설계 관련해서 리뷰 및 조언 더 남겨주시면, 이번에도 열심히 공부하여 반영해 보도록 하겠습니다!🙇🏻‍♀️

Jiihyun · 2026-04-02T11:28:08Z

src/main/java/janggi/domain/Position.java

+    public static Position from(List<Integer> rawPosition) {
+        validatePositionSize(rawPosition);
+        return new Position(rawPosition.get(0), rawPosition.get(1));
+    }
+
+    private static void validatePositionSize(List<Integer> position) {
+        if (position.size() != POSITION_SIZE) {
+            throw new IllegalArgumentException(ExceptionMessage.INVALID_INPUT_FORMAT.getMessage());
+        }
+    }


추가하였습니다!!
refactor: Position format 테스트 추가

Jiihyun · 2026-04-02T12:23:45Z

src/main/java/janggi/domain/board/InitialPiecePlacement.java

+    public static Map<Position, Piece> init(ElephantSetting hanChoice, ElephantSetting choChoice) {
+        Map<Position, Piece> board = new HashMap<>();
+
+        for (InitialPiecePlacement piece : values()) {
+            board.put(new Position(piece.row, piece.column), new Piece(piece.pieceRule, piece.camp));
+        }
+
+        board.putAll(hanChoice.createElephantOrder(Camp.HAN));
+        board.putAll(choChoice.createElephantOrder(Camp.CHO));
+        return board;
+    }


오!! 테스트를 활용할 수 있다는 생각을 하지 못했네요.
앞으로 비슷한 상황이 생긴다면 구조를 더 복잡하게 만들기보다는, 테스트 코드를 더 꼼꼼히 작성하는 방향을 먼저 고민해보겠습니다!

Jiihyun · 2026-04-02T12:45:01Z

src/main/java/janggi/domain/piece/condition/MoveCondition.java

+
+public interface MoveCondition {
+
+    void checkPath(List<Position> path, Camp camp, BoardChecker board, PieceRule pieceRule);


상속 구조를 더 추천하시는 주된 이유나, 상속을 통해 얻을 수 있는 설계적 이점에 대해서 조금 더 조언을 구할 수 있을까요?
상속을 사용하면 piece 클래스에 camp와 pieceRule를 놓고 쓸 수 있기 때문일까요??!

저는 장기는 기물의 종류와 각 기물의 행마법이 변하지 않는 정적인 도메인이기 때문에, Enum을 활용하여 "어떤 타입은 어떤 전략을 가진다"를 미리 세팅해 두면 관리 포인트가 한 곳으로 모아져 좋다고 생각했습니다!

Jiihyun · 2026-04-02T13:05:19Z

src/main/java/janggi/dto/CampDto.java

+public record CampDto(String camp) {
+
+    public static CampDto from(Camp camp) {
+        return new CampDto(CampFormatter.format(camp));
+    }


DTO는 뷰와 도메인 사이의 결합을 느슨하게 유지하고, 뷰 변경이 도메인까지 영향을 미치지 않도록 보호하는 역할을 합니다.

보드를 출력할 때도 도메인의 모든 값을 전달하기보다는 필요한 값만 전달하기 위해 DTO를 사용했습니다. 다만 camp는 값이 그대로 전달되어도 되지만, 구조적 통일성을 위해 DTO를 사용했었습니다.

하지만 단순히 통일성을 맞추기 위해 Camp → CampDto와 같은 불필요한 변환 로직을 추가하면, 코드가 늘어나고 유지보수가 복잡해져 장점보다는 단점이 더 클 것 같네요!!
따라서 CampDto를 제거하였습니다!!

verus-j

안녕하세요 러키~
피드백 반영하느라 고생하셨습니다. 몇가지 추가 피드백 남겨놨습니다. 확인부탁드려요~

verus-j · 2026-04-03T14:55:12Z

src/main/java/janggi/domain/piece/strategy/EmptySlidingRule.java

+import janggi.domain.piece.PieceRule;
+import java.util.List;
+
+public class EmptySlidingRule extends SlidingRule {


상속 구조가 너무 복잡해지는 것 같습니다. 일단 중복 코드를 고려하지말고 중간 단계인 SlidingRule을 제거해보면 좋을 것 같아요.

verus-j · 2026-04-03T14:56:23Z

src/main/java/janggi/domain/piece/strategy/JumpingSlidingRule.java

+            if (!board.hasPieceAt(position)) {
+                continue;
+            }


continue를 사용하지 않도록 수정하면 좋을 것 같아요. 제어 흐름이 동일하지 않으면 코드를 분석하기 어렵습니다. 모든 원소가 동일한 흐름을 같도록 통일해보시죠.

verus-j · 2026-04-03T14:58:55Z

src/main/java/janggi/domain/piece/strategy/DiagonalStepRule.java

+    }
+
+    private void validateDistance(DirectionInformation direction) {
+        if (!direction.hasAbsDifferences(diagonalDistance, STRAIGHT_DISTANCE + diagonalDistance)) {


distance와 difference를 통일하면 좋을 것 같네요.

verus-j · 2026-04-03T15:01:16Z

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

+    HORSE(new DiagonalStepRule(1)),
+    CANNON(new JumpingSlidingRule()),
+    GUARD(new SingleStepRule(false)),
+    ELEPHANT(new DiagonalStepRule(2)),


마와 상이 1칸 전진이라는 조건이 현재는 동일하지만 한 종류만 2칸 전진으로 바뀌면 서로에 영향을 주는 변경을 해야하는 것 같네요. 어떻게 하면 개선할 수 있을지 고민해보면 좋을 것 같아요~

MODUGGAGI added 30 commits March 24, 2026 20:51

docs(README): 기능 요구사항 작성

1e2353f

feat: Position 생성 시 행,열 예외 처리 구현

153cf04

Position 객체 생성 시 행 범위를 벗어난 경우, 열 범위를 벗어난 경우를 검증하는 예외 테스트를 작성 및 로직을 구현했습니다.

feat: 행과 열 차이 계산 기능 구현

12fb008

- 행과 열 차이 계산 테스트 추가

feat: 병의 행마법 계산 로직 구현

a6b1208

- 상좌우만 가능 (후진 구현x) - 행마법 테스트 추가

refactor: 소스 코드 패키지로 이동

a590747

feat: 보드 초기화 기능 구현

85cedc8

- 보드 초기화 테스트 추가 - 초기화 전략 구현

refactor: Position VO 전환

0d04301

feat: 상차림 기능 구현

053ccf3

- 상차림 테스트 추가

feat: 졸/병 후진 예외 처리 구현

c07bf10

feat: 졸/병 후진 예외 처리 구현가

23e026f

test: 행마법 테스트 추가

a461255

- 기존 Piece 테스트 제거

feat: 차 행마법 기능 구현

84cf947

- 차 행마법 테스트 추가 - Position 위치 1칸 이동 기능 추가

docs: 구현한 기능 완료 처리

c8f9421

refactor: 전략 이름 수정

4df7f3d

refactor: 경로 생성 로직 최적화

462827a

refactor: 테스트 메서드 이름 수정

99c17c1

feat: 차,포 제자리 이동 예외 추가

d78aef3

feat: 마 이동 구현

b3b9198

- 마 행마법 테스트 추가

feat: 상 행마법 기능 구현

f92203f

- 상 행마법 테스트 추가 - DirectionInformation 값 객체 추가

refactor: DirectionInformation 값 객체 적용

3f62291

refactor: DirectionInformation 값 객체 적용

49914a9

refactor: 클래스 이름 PieceRule로 변경

dfb2afe

refactor: 생성자에서 MoveStrategy 제거

7ef3df4

feat: 경로 상 기물이 없어야 이동할 수 있는 조건 확인 기능 구현

d8fa73b

- 경로 상 기물 없는 조건 확인하는 테스트 추가

feat: 경로 상 기물이 1개 있어야 이동할 수 있는 조건 확인 기능 구현

74efd5a

- 경로 상 기물이 1개 있는 조건 확인하는 테스트 추가

refactor: MoveCondition 매개변수 추가

4fe8ab9

feat: MoveCondition 추가

0eabf55

feat: 기물 이동 검증 기능 구현

5445082

- 기물 이동 검증 기능 테스트 추가

feat: 보드 기물 이동 기능 구현

dc41b10

- 기물 이동 기능 테스트 추가

refactor: 상차림 로직 수정

a13c511

Jiihyun commented Mar 28, 2026

View reviewed changes

Jiihyun added 3 commits March 30, 2026 11:43

refactor: PieceFormatter와 CampFormatter enum으로 변경 및 switch문 제거

766e481

refactor: InitialPiecePlacement 인스턴스 변수 축소

024b08b

test: Turn 테스트 추가

53d93d6

verus-j requested changes Mar 30, 2026

View reviewed changes

Jiihyun added 9 commits March 30, 2026 14:29

test: ElephantSetting 테스트 추가

cd30819

test: DirectionInformation 테스트 추가

c8ebd7f

test: Camp 테스트 추가

38f9425

test: HorseStrategy 테스트 추가

6ef4515

refactor: EmptyConditionTestBoardInitializer 제거

e60c9e6

refactor: OnePieceExistsCondition 테스트이름 추상화

da6cd23

refactor: ElephantStrategyTest 내 불필요한 테스트 매개변수 제거

6325339

refactor: MultiStepStraightStrategyTest, SingleStepStraightStrategyTe…

69e6abb

…st 테스트 명 추상화

refactor: PieceTest 내 테스트명 변경

12a3328

Jiihyun commented Mar 30, 2026

View reviewed changes

Jiihyun added 2 commits March 31, 2026 14:24

refactor: GameRule 상수 클래스 제거

70cab43

refactor: 보드 초기화 시 상차림 타입 변경

423ea0f

verus-j requested changes Mar 31, 2026

View reviewed changes

Jiihyun added 9 commits April 1, 2026 10:04

refactor: 행마법 전략 추상화 레벨 맞도록 변경

ba43328

refactor: BoardInitializer 클래스 삭제

c4d4de5

refactor: elephantSetUpFormat에서 ElephantFormation 생성하도록 변경

038079b

refactor: CampDto 제거

121f2b8

refactor: 출발지, 목적지 검증 위치 변경

2b06742

refactor: 이동 거리 구하는 로직 분리

d8188fc

refactor: 이동 전략 메서드 분리

619d82b

refactor: 행마법 전략 추상화 수준 동일하게 변경

5518268

refactor: Position format 테스트 추가

e1138ec

Jiihyun commented Apr 2, 2026

View reviewed changes

verus-j requested changes Apr 3, 2026

View reviewed changes


		public interface MoveCondition {

		void checkPath(List<Position> path, Camp camp, BoardChecker board, PieceRule pieceRule);


		class SoldierStrategyTest {

		private final MoveStrategy strategy = new SoldierStrategy();

Conversation

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

Uh oh!

체크 리스트

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

[🔴 Must][ 1. 장기 기물 설계 방식: Piece 상속 vs 행마법 interface ]

리뷰 요청 포인트

[🟢 Optional][ 2. 다형성 활용 기준 ]

리뷰 요청 포인트

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

[🔴 Must] Board와 Piece 의존성 분리 방식에 대한 고민

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

[🟢 Optional] 나라에 따른 상차림 타입 안정성에 대한 고민

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

[⚪️ Curious] BoardInitializer 추상화의 필요성에 대한 고민

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jiihyun 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!

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

Uh oh!

Choose a reason for hiding this comment

[🔴 Must] 공통 인터페이스와 특정 구현체 전용 인자에 대한 고민 (MoveCondition의 PieceRule 인자)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jiihyun 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!

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

Uh oh!

Choose a reason for hiding this comment

[⚪️ Curious] DTO의 역할에 대한 고민

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

[⚪️ Curious] 초기 장기 기물 배치 데이터 주입 방식: enum vs CSV

Uh oh!

Choose a reason for hiding this comment

Uh oh!

verus-j 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

Jiihyun commented Mar 28, 2026 •

edited

Loading

Jiihyun Mar 28, 2026 •

edited

Loading

Jiihyun Mar 28, 2026 •

edited

Loading

Jiihyun Mar 28, 2026 •

edited

Loading

Jiihyun Apr 2, 2026 •

edited

Loading

Jiihyun Mar 28, 2026 •

edited

Loading

Jiihyun Apr 2, 2026 •

edited

Loading

Jiihyun Mar 28, 2026 •

edited

Loading

Jiihyun Mar 28, 2026 •

edited

Loading

Jiihyun Mar 30, 2026 •

edited

Loading

Jiihyun left a comment •

edited

Loading