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

picetea44 · 2026-03-29T11:50:32Z

체크 리스트

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

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

안녕하세요 비밥!
장기 미션 사이클1 리뷰 잘 부탁드립니다 😊

Q1. searchRoute의 위치와 타입 캐스팅

EmptyPiece는 경로를 찾을 필요가 없기에 searchRoute 메서드를 Piece 인터페이스가 아닌 ActivePiece 추상 클래스에 배치했습니다. 이로 인해 Board에서 ((ActivePiece) piece).searchRoute(src, dest)와 같은 다운캐스팅이 발생하고 있습니다.
빈 공간이 경로를 가질 필요가 없다는 논리는 유효하지만, 실행 시점의 캐스팅이 설계상 위험한 것 같습니다. Piece에 기본 메서드로 둔 다음 EmptyPiece에서는 빈 리스트를 반환하게 두는 것이 나을까요, 아니면 현재처럼 타입을 분리하는 것이 나을까요?

Q2. 의미 없는 메서드 구현

기물마다 이동 방식이 다르다 보니, 어떤 기물은 중간 경로가 필요하고 어떤 기물은 아예 필요가 없는데, 공통 인터페이스를 맞추다 보니 중간 경로가 필요 없는 기물에서도 List.of() 같은 빈 리스트를 반환하는 메서드를 구현하게 됐습니다. 한 칸만 이동하는 기물 입장에서는 이 메서드 자체가 의미가 없어서, 지금처럼 빈 값을 반환하는 방식이 구조를 유지하는 데는 도움이 되지만, 한편으로는 특정 기물에게 불필요한 책임을 억지로 부여하는 느낌이 듭니다. 그래서 고민되는 건, 이런 경우에도 다형성을 유지하는 게 더 나은 선택인지, 아니면 기물별로 역할을 조금 더 명확하게 나눠서 의미 없는 메서드는 아예 제거하는 방향이 나은지입니다. 빈 값을 반환하는 메서드 구조가 괜찮은 설계인지에 대해서 의견을 한번 듣고 싶습니다.

Q3. 불변 객체의 Getter 사용

이번 미션에서 getter를 사용하지 않는다는 요구사항을 지키려고 최대한 객체에 메시지를 보내는 방식으로 구현을 해봤습니다. 그런데 구현을 진행하다 보니 상태가 변하지 않는 불변 객체의 경우에도 getter를 지양하는 게 맞는지에 대한 고민이 들었습니다. 내부 상태가 변경되지 않는 객체라면, 단순히 값을 조회하는 getter가 캡슐화를 크게 해치는 건 아닐 수도 있지 않을까 하는 생각이 들었습니다.
또한 단순 조회 한 번이면 해결될 로직을, getter를 쓰지 않기 위해 객체에 역할을 분산시키다 보니 오히려 메서드가 늘어나고 구현이 더 복잡해지는 느낌도 받았습니다. 따라서 getter를 지양해야 하는 명확한 이유가 있는지 궁금합니다. 또한, 실제 실무에서는 도메인 모델에서 getter를 어느 정도로 제한하는지 궁금합니다. 비즈니스 로직 구현의 편의성과 어느 정도 타협하면서 사용하는 경우가 많은지, 현업에서는 어떤 기준으로 균형을 잡는지도 궁금합니다.

Q4. 반성

현재 Board가 모든 위치 정보를 가지고 있다 보니, 이동 경로에 대한 검증도 Board에서 처리하는 게 맞다고 생각해서 구현했습니다. 그런데 그 과정에서 포의 이동 규칙(포는 포를 넘지 못한다, 반드시 하나의 기물을 넘어야 한다 등)까지 Board가 직접 알고 처리하게 됐습니다. 이렇게 두다 보니 Board가 단순히 상태를 관리하는 역할을 넘어서, 각 기물의 세부 규칙까지 알게 되는 구조가 된 것 같아 아쉬움이 남습니다. 시간 관계상 이번 단계에서는 충분히 개선하지 못했는데, 이 부분은 다음 사이클에서 역할을 더 잘 분리하는 방향으로 개선해보고 싶습니다. 혹시 이런 상황에서 책임을 어떻게 나누는 게 좋을지에 대해서도 조언을 부탁드리고 싶습니다..!

pci2676

안녕하세요. 질문에 대한 답변을 달아두었으니 확인해주세요.

조금 더 개선해보면 좋을 부분에 리뷰를 남겨두었으니 같이 확인해주세요.

pci2676 · 2026-03-30T09:12:08Z

src/main/java/domain/board/Board.java

+        this.pieces = new HashMap<>(pieces);
+    }
+
+    public void move(Position src, Position dest) {


pieces.get(src)가 null을 반환할 수 있는데요, 보드에 등록되지 않은 위치에서 이동을 시도하면 바로 다음 줄 validateCanMove에서 piece.canMove()를 호출할 때 NullPointerException이 발생합니다. pieceAt() 메서드를 사용하면 안될까요?

확실히 get()을 직접 사용하는 방식은 불필요했던 것 같습니다.

말씀해주신 것처럼, 보드에 없는 위치에서 null이 반환될 수 있고, 그 상태로 canMove()를 호출하면서 예외가 발생될 여지가 있는것 같습니다. 이를 방지하기 위해 EmptyPiece를 만들어두었는데, 제대로 활용하지 못한 부분 같네요.

pieceAt()을 사용하는 방향으로 수정하는 것이 더 안전하고 의도에도 맞는 설계같습니다. 좋은 지적 감사합니다!

pci2676 · 2026-03-30T09:12:11Z

src/main/java/domain/piece/ActivePiece.java

+        return team.forwardRowDirection();
+    }
+
+    public abstract List<Position> searchRoute(Position source, Position target);


Q1. searchRoute의 위치와 타입 캐스팅
EmptyPiece는 경로를 찾을 필요가 없기에 searchRoute 메서드를 Piece 인터페이스가 아닌 ActivePiece 추상 클래스에 배치했습니다. 이로 인해 Board에서 ((ActivePiece) piece).searchRoute(src, dest)와 같은 다운캐스팅이 발생하고 있습니다.
빈 공간이 경로를 가질 필요가 없다는 논리는 유효하지만, 실행 시점의 캐스팅이 설계상 위험한 것 같습니다. Piece에 기본 메서드로 둔 다음 EmptyPiece에서는 빈 리스트를 반환하게 두는 것이 나을까요, 아니면 현재처럼 타입을 분리하는 것이 나을까요?

이야기하신대로 다운캐스팅을 런타임에 하는것은 그리 좋은 방향으로 보이지는 않아요. EmptyPiece에서는 빈리스트를 반환하도록 하는게 좋을것 같네요.

그리고 EmptyPiece에서 빈 리스트를 반환하는 게 "의미 없는 구현"처럼 느껴지실 수 있지만, Board 의 canMove()에서 이미 false를 반환하고 있으니 searchRoute까지 도달하지않을것 같네요. 그렇다면 EmptyPiece의 searchRoute가 실제로 호출될 일도 없어보여요.

말씀해주신 “의미 없는 구현”에 대해 많이 고민하고 있었는데, 이 부분을 짚어주셔서 다시 한 번 생각해보게 되었습니다. 특히 이걸 설계로 어떻게 풀어내야 할지 고민이 많아지면서 방향을 잡는 데 어려움을 느꼈습니다.

비밥이 말씀해주신 것처럼 EmptyPiece의 searchRoute()는 실제로 호출되지 않을 가능성이 높다고 이해했습니다. 그렇다면 이런 경우에는 “의미 없는 구현”에 대한 경계를 어느 정도까지 유지? 또는 어느 정도까지는 허용 가능한 수준인지 고민이 됩니다. 호출되지 않는다는 전제가 있다면, 해당 구현을 두는 것에 대해 어느 정도 유연하게 생각해도 괜찮을지 여쭤보고 싶습니다.

또한 다운캐스팅을 줄이기 위해 일부 메서드를 상위 타입으로 끌어올리는 리팩토링을 진행했습니다. 그 과정에서 아래와 같이 instanceof를 활용한 구현을 추가하게 되었습니다.

public boolean isAlly(Piece other) { return other instanceof ActivePiece activePiece && this.team == activePiece.team; }

이 부분은 other가 실제 기물인지(= ActivePiece인지)를 구분하기 위한 의도로 작성했습니다. instanceof가 다형성을 해칠 수 있는 경우에는 지양해야 한다고 알고 있는데, 현재와 같은 상황에서는 그 사용이 크게 문제되지 않는다고 판단했습니다.

비밥은 이 부분에 대해 어떻게 보시는지 의견을 들어보고 싶습니다.

의미없는 구현 처럼 느껴졌을 뿐이지 저는 최소한의 방어로직이라고 판단했던것 같습니다. 실제로 완전 의미없는 구현이라 판단된다면 구현하지 않는게 가장 좋겠죠? 설계의 방향을 바꾸거나요.

instanceof는 지양해주세요. 메세지를 던지는 방향으로 개선해보시면 좋을것 같습니다. 아시다시피 추상화를 해두었는데 추상화를 깨는 코드를 작성한것과 다름이 없으니까요.

사실 설계의 방향이 이 상황을 발생시켰다고 볼 수 있을것 같은데요. EmptyPiece 가 없었다면 이렇게 instanceof를 통해 클래스 타입을 확인하지 않아도 됐을것 같네요.

저는 세가지 방법이 떠오르는데

EmptyPiece 제거

메서드를 Piece로 올리고 Team에 EmptyPiece 를 위한 개념 추가

별도의 Team 구분 메서드를 만들고 isAlly에서 호출하기

2번 방법은 Team에 새로운 개념을 추가하는게 EmptyPiece에 의해 Team의 구조가 흔들리는것 같아 내키지 않고

3번 방법은 메서드 호출이 꼬리 물고 깊게 들어가서 가독성이 그리 좋지 않아 내키지 않는것 같습니다.

그래서 저라면 1번의 방향을 택할것 같네요

모아도 고민해보고 더 나은방법이 있다면 구현해보세요 꼭 따라할 필요는 없습니다만 instanceof는 제거해보시죠

pci2676 · 2026-03-30T09:12:40Z

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

+    String display(PieceAppearance colorizer);
+
+    default boolean isCannon() {
+        return false;


isCannon()이 Piece 인터페이스에 default 메서드로 들어가 있는데요, 이렇게 하면 특정 구현체를 인터페이스 레벨에서 알고 있는 셈이 됩니다. 기물 종류가 늘어날 때마다 isXxx() 메서드가 추가될 수 있는 구조이기도 하고요.

경로 검증 책임을 기물에게 위임하면, Board가 "이 기물이 포인지" 물어볼 필요 자체가 없어질 수 있습니다. 한번 고민해보시면 좋겠어요.

그리고 default 메서드는 새로운 인터페이스를 추가할 때 하위호환성을 지키기 위함이니 지금은 올바른 사용법으로 생각되지는 않는것 같아요

이번에 기물로 경로 검증 책임을 옮겨봤습니다. 검증 책임이 바뀌니 확실히 isCannon() 의 필요성이 없어진게 보였습니다.

다만 구현 과정에서 instanceof를 사용하게 되었는데 이게 적절한 사용법 이였을까요?
제가 생각한 instanceof의 허용 가능한 기준은 다음과 같이 생각합니다.

“다형성으로 해칠 수 있는 행위인가?" 아니면 "다형성의 범위를 벗어난 시스템적, 구조적 처리를 위함인가?”

Cannon은 더 이상 확장을 진행하지 않는 말단 클래스로 저는 정의하였고 이 과정에서 포가 포를 인지하기 위해 instanceof를 사용하게 되었습니다.이 부분은 다형성을 해치는 사용이라기보다는, 타입 기반으로 구분이 필요한 상황이라고 판단했습니다.

혹시 제가 잘못 판단한 부분이 있다면 지적해주시면 감사하겠습니다.

포 규칙때문에 어디로 옮겨도 비슷한 느낌이 나긴하네요

특정 타입을 물어보는 메서드보단 특정 책임을 물어보는 메서드 네이밍으로 변경만하셔도 괜찮을것 같습니다.

네이밍을 통해 타입을 묻는것과 책임을 묻는것은 이름만 달라보일지언정 그 네이밍이 갖고있는 영향력은 다르다고 생각해요.

as-is: is{type}
to-be: can{action}

pci2676 · 2026-03-30T09:12:50Z

src/main/java/domain/board/AbstractBoardFactory.java

+            4, new YanggwimaFactory()
+    );
+
+    public static AbstractBoardFactory from(int input) {


factories.get(input)이 유효하지 않은 숫자(예: 5, 0)가 들어오면 null을 반환하는데요, 이 null이 그대로 createFormation() 호출 시점에 NullPointerException으로 터지게 됩니다. 잘못된 입력에 대해 의미 있는 예외 메시지를 줄 수 있도록 여기서 방어해주는 게 좋겠어요.

문제 상황을 정리해보면 아래와 같이 이해했습니다.

처음에는 InputView에서 이미 1~4에 대한 검증을 하고 있기 때문에, 도메인에서 이를 다시 검증하는 것이 중복이라고 생각했습니다.
다만 다시 생각해보니 두 가지 문제가 있다고 느꼈습니다.

다른 사용자가 이 코드를 사용할 경우, 의도하지 않은 값이 들어올 수 있다는 점
도메인이 입력 형식(예: 1,2,3,4)에 의존하게 되어, 입력 요구사항이 바뀌면 도메인까지 수정해야 하는 구조라는 점

이 부분을 해결하기 위해 다음과 같이 변경해보았습니다.

FormationType이라는 enum을 도입해서, 도메인에서는 해당 타입만 다루도록 제한했습니다.
FormationConverter라는 별도의 클래스를 두어, 입력값을 FormationType으로 변환하는 책임을 분리했습니다. 이를 통해 입력 형식이 바뀌더라도 해당 부분만 수정하면 되도록 했습니다.
또한, 명시적으로 null에 대한 처리도 추가했습니다.

여기서 몇 가지 고민이 있습니다.

이런 식으로 별도의 변환(유틸) 클래스를 두는 방향이 괜찮은지
이 클래스의 위치를 어디에 두는 게 적절할지 (컨트롤러가 변환 책임을 가진다고 보고 controller에 둘지, 입력과 가깝다고 보고 view에 둘지)
기존에 InputView에서 수행하던 유효성 검사를 그대로 유지하는 것이 맞는지

이 방향이 괜찮은 접근인지, 그리고 더 나은 설계 방향이 있다면 비밥의 의견을 듣고 싶습니다.

아 view에서 들어온걸 도메인타입으로 컨버팅하는 역할이군요

좋은것 같습니다 👍

실제로 저희팀에서도 팀에서 관리하지 않는 외부 시스템과 데이터를 주고받을때 이런 계층을 두어서 도메인에서 사용하는 데이터 형태로 변환하고 사용합니다.

view에 의존적이던 값, 특정 클라이언트에 의존적인 부분이 도메인을 침범하지 않게 되어 도메인이 더 견고해지는 효과가 있는데 잘 적용해주신것 같네요.

이런 부분을 anti corruption layer 라고 부르기도 하는데 시간나시면 한번 찾아서 읽어보시면 좋을것 같습니다.

pci2676 · 2026-03-30T09:12:58Z

src/main/java/domain/piece/Elephant.java

+
+public class Elephant extends ActivePiece {
+
+    private final List<Integer> dx = List.of(2, 3, -2, -3, -3, -2, 2, 3);


dx, dy가 인스턴스 필드로 선언되어 있어서 Elephant 객체가 생성될 때마다 새로운 리스트가 만들어지는데요, 이 값들은 모든 상(象) 인스턴스에서 동일하니까 static final로 선언하면 불필요한 객체 생성을 줄일 수 있습니다. 앞에서 언급한 부분과 같은데요, Horse, General, Guard, Soldier에서도 동일하게 발견됩니다.

그리고 여기도 축약하지 말아주세요 아마 delta겠죠?

아 정말 그렇네요. 미처 생각하지 못했던 부분인데 짚어주셔서 감사합니다!

말씀해주신 것처럼 이 값들은 모든 인스턴스에서 동일하게 사용되니 static final로 선언하는 것이 더 적절한 것 같습니다. 불필요한 객체 생성도 줄일 수 있을 것 같고요.

그리고 네이밍도 dx, dy보다는 얼마나 떨어져 있는가의 offset이 더 의도를 잘 드러내는 것 같아서 함께 수정해보았습니다.

pci2676 · 2026-03-30T09:13:58Z

src/test/java/domain/position/PositionTest.java

+    }
+
+    @Test
+    void 경계값_최대_범위의_위치를_생성한다() {


예외 테스트에서 isInstanceOf(IllegalArgumentException.class)만 검증하고 메시지를 확인하지 않고 있는데요, Row와 Column 모두 IllegalArgumentException을 던지기 때문에 행이 잘못된 건지 열이 잘못된 건지 이 테스트만으로는 구분이 안 됩니다. hasMessage() 또는 hasMessageContaining()으로 의도한 예외 지점을 특정해주시면 더 견고한 테스트가 될 것 같아요.

생각해보지 못했던 부분이였네요. 확실히 어떤 부분에 의해서 오류가 터지는지 구분이 안되었던 것 같습니다. 조언 너무 감사드립니다. 확실히 이런 상황이 거짓 음성에 해당할 수 있는 경우 였군요. 말씀해주신대로 특정 예외 상황을 지정할 수 있는 정보를 함께 테스트에 포함시킬 수 있도록 연습해보겠습니다!

pci2676 · 2026-03-30T09:17:36Z

src/main/java/domain/piece/Soldier.java

+    @Override
+    public List<Position> searchRoute(Position source, Position target) {
+        return List.of();
+    }


Q2. 의미 없는 메서드 구현
기물마다 이동 방식이 다르다 보니, 어떤 기물은 중간 경로가 필요하고 어떤 기물은 아예 필요가 없는데, 공통 인터페이스를 맞추다 보니 중간 경로가 필요 없는 기물에서도 List.of() 같은 빈 리스트를 반환하는 메서드를 구현하게 됐습니다. 한 칸만 이동하는 기물 입장에서는 이 메서드 자체가 의미가 없어서, 지금처럼 빈 값을 반환하는 방식이 구조를 유지하는 데는 도움이 되지만, 한편으로는 특정 기물에게 불필요한 책임을 억지로 부여하는 느낌이 듭니다. 그래서 고민되는 건, 이런 경우에도 다형성을 유지하는 게 더 나은 선택인지, 아니면 기물별로 역할을 조금 더 명확하게 나눠서 의미 없는 메서드는 아예 제거하는 방향이 나은지입니다. 빈 값을 반환하는 메서드 구조가 괜찮은 설계인지에 대해서 의견을 한번 듣고 싶습니다.

여기 때문에 위와 같은 질문을 주신것 같네요. 그런데 searchRoute는 움직임의 경로를 나타내는데 기물들이 src에서 dest로 갈수 없는 상황일때에도 빈 리스트를 응답해야하지 않나요? 메서드의 이름과 실제 동작이 저는 약간 거리감이 있는것 같아보이네요.

그리고 src, dest 축약하지 말아주세요

맞는 말씀인 것 같습니다. searchRoute()는 항상 canMove()를 먼저 통과한 유효한 대상에 대해서만 호출되기 때문에, 단순히 “경로를 탐색한다”기보다는 “유효한 입력에 대해 계산을 수행한다”는 의미가 더 가깝다고 느꼈습니다.

그래서 메서드 이름을 calculateRoute()로 변경해보았는데, 이런 방향의 네이밍이 적절한지 궁금합니다. 혹시 제가 잘못 이해하고 있는 부분이 있다면 지적해주시면 감사하겠습니다.

경로를 찾아서 알려주는 느낌이라 지금도 그렇게 어색하다는 느낌은 없는데요. 굳이 저라면 뭐라고 지었을까 하면 findRoute 정도가 떠오르네요 별반 차이없죠? ㅋㅋ..

pci2676 · 2026-03-30T09:23:43Z

src/main/java/domain/game/Turn.java

+    public Turn next() {
+        if (current == Team.CHO) {
+            return new Turn(Team.HAN);
+        }
+        return new Turn(Team.CHO);
+    }


Q3. 불변 객체의 Getter 사용
이번 미션에서 getter를 사용하지 않는다는 요구사항을 지키려고 최대한 객체에 메시지를 보내는 방식으로 구현을 해봤습니다. 그런데 구현을 진행하다 보니 상태가 변하지 않는 불변 객체의 경우에도 getter를 지양하는 게 맞는지에 대한 고민이 들었습니다. 내부 상태가 변경되지 않는 객체라면, 단순히 값을 조회하는 getter가 캡슐화를 크게 해치는 건 아닐 수도 있지 않을까 하는 생각이 들었습니다.
또한 단순 조회 한 번이면 해결될 로직을, getter를 쓰지 않기 위해 객체에 역할을 분산시키다 보니 오히려 메서드가 늘어나고 구현이 더 복잡해지는 느낌도 받았습니다. 따라서 getter를 지양해야 하는 명확한 이유가 있는지 궁금합니다. 또한, 실제 실무에서는 도메인 모델에서 getter를 어느 정도로 제한하는지 궁금합니다. 비즈니스 로직 구현의 편의성과 어느 정도 타협하면서 사용하는 경우가 많은지, 현업에서는 어떤 기준으로 균형을 잡는지도 궁금합니다.

이 질문에 대한 답변을 어디다 달지 고민하다가 여기에 답변드릴게요. 아마 무분별한 getter를 사용하는것을 막기 위해서 일것 같은데요. 가령 디미터 법칙을 어기는 경우가 발생할 수 도 있을것 같네요.

얼핏 보았을때 대부분 잘 지켜주신것 같긴한데 지금과 같은 경우를 막기 위함으로 보여요. 질문으로 미루어 보았을 때 캡슐화를 지키기 위함이라는것은 이미 충분히 인지하고 계신것 같네요. 지금과 같이 Turn에서 Team에 대한 구현 상세가 노출되는 경우를 막으라는거지 무조건 getter는 안돼 는 아니라고 생각해요.

실무에서도 getter를 써양하는 경우도 분명 존재하구요. ex. 직렬화, 역직렬화를 위해서 등..

가령 지금 미션의 경우 view를 위해서라면 getter메서드가 나올수 밖에 없을것 같아요

아하 답변 감사드립니다!
view를 위해서 쓰는 부분은 불가피한 경우가 있었군요.

이번에 컨트롤러를 공부하면서 DTO에 대해서도 역할을 다시 공부해봤습니다.
혹시 DTO에 대한 도입은 현 상황에서 어떨지 궁금합니다. DTO를 사용한다고 해서 무조건 getter를 안쓰게 되진 않을 수 있지만, getter의 사용을 최대한 줄일 수 있다는 점, 뷰에게 직접 모델을 던져주는 방식에 대비해 뷰와 모델의 연결을 끊을 수 있다는 점 등 이점이 있다고 생각합니다.

굳이 지금 단계에서는 dto라는 개념을 쓰실 필요는 없을것 같아요.

지금은 도메인에서 문제를 어떻게 풀어서 코드를 구현할지에 대해 집중하는게 더 좋을것 같습니다.

그리고 보통 dto를 만들어도 도메인이 dto를 의존하지 않고 dto가 도메인을 의존하도록 하기 때문에

값 전달을 위한 getter는 대부분 생겨나기 마련이긴해요. 값 객체로 모든 값을 전달할 수 있는게 아니라면요

pci2676 · 2026-03-30T09:26:55Z

src/main/java/domain/board/Board.java

+        if (piece.isCannon()) {
+            validateCannonRoute(route, dest);
+        } else {
+            validateIntermediateRoute(route);
+        }


Q4. 반성
현재 Board가 모든 위치 정보를 가지고 있다 보니, 이동 경로에 대한 검증도 Board에서 처리하는 게 맞다고 생각해서 구현했습니다. 그런데 그 과정에서 포의 이동 규칙(포는 포를 넘지 못한다, 반드시 하나의 기물을 넘어야 한다 등)까지 Board가 직접 알고 처리하게 됐습니다. 이렇게 두다 보니 Board가 단순히 상태를 관리하는 역할을 넘어서, 각 기물의 세부 규칙까지 알게 되는 구조가 된 것 같아 아쉬움이 남습니다. 시간 관계상 이번 단계에서는 충분히 개선하지 못했는데, 이 부분은 다음 사이클에서 역할을 더 잘 분리하는 방향으로 개선해보고 싶습니다. 혹시 이런 상황에서 책임을 어떻게 나누는 게 좋을지에 대해서도 조언을 부탁드리고 싶습니다..!

이 부분이겠죠? 저는 이번단계에서 고치고 넘어가면 좋을것 같은데요ㅎㅎ.. 지금 move에서 validate하는 대부분의 것들이 각 piece 구현체 내부로 가려지도록 해보면 어떨까요.

아시겠지만 move 메서드가 포인지 아닌지를 if-else로 분기하고 있습니다. 새로운 특수 이동 규칙을 가진 기물이 추가되면 이 분기가 계속 늘어나겠죠?

"경로에 있는 기물들을 검증하는 책임"을 각 기물이 스스로 가지게 하면 어떨까요? 예를 들어 기물에게 경로상의 기물 목록을 전달하고, 자기 규칙에 맞는지 스스로 판단하게 하는 방식이요. Board는 "경로에 뭐가 있는지 조회"만 하고, "그게 유효한지 판단"은 기물에게 위임하는 구조를 생각해보시면 좋겠습니다.

해당 부분 기물에게 부여하는 리팩토링을 진행해봤습니다. 이런 리팩토링 작업이 익숙하지 않아서 다소 오래 걸렸지만.. 비밥이 조언해주신 부분이 정말 많은 힌트가 되었던 것 같습니다. 정말 감사합니다..!!

pci2676 · 2026-03-30T09:28:46Z

src/main/java/controller/JanggiController.java

+import view.InputView;
+import view.OutputView;
+
+public class JanggiController {


질문인데요. controller는 어디서 착안하신 네이밍일까요? 왜 controller인가요?

이번에 비밥이 질문해주신 덕분에 컨트롤러가 어떤 역할을 가져야 하는지에 대해 다시 정리해보았습니다.

자료를 찾아보는 과정에서 글마다 설명이 조금씩 달라서 기준을 잡는 데 생각보다 어려움을 느꼈고, 그래서 나름대로 기준을 세워 정리해봤습니다.

우선 컨트롤러는 애플리케이션의 진입점이자 흐름을 제어하는 역할이라고 이해했습니다. 외부(View, 클라이언트)와 내부(Model, Service) 사이에서 중간 다리 역할을 하면서, 입력을 도메인이 이해할 수 있는 형태로 변환하고, 반대로 결과를 외부에 맞게 변환하는 “번역가” 역할을 한다고 생각했습니다. 또한 잘못된 요청을 초기에 걸러내고 흐름을 제어하는 “문지기” 역할도 함께 가진다고 정리했습니다.

서비스 레이어의 유무에 따라서도 역할의 무게가 달라진다고 보았습니다. 서비스가 없는 구조에서는 컨트롤러가 도메인 객체들을 직접 조합하면서 흐름을 이끄는 역할까지 담당하게 되고, 서비스 레이어가 있는 경우에는 그 책임을 서비스에 위임하고 컨트롤러는 라우팅에 집중하는 구조가 더 적절하다고 이해했습니다.

또 하나 고민했던 부분은, 컨트롤러가 처리 결과를 View로 어떻게 전달해야 하는지였습니다. 도메인 모델을 그대로 넘기는 방식은 View가 도메인 내부를 알게 되면서 결합도가 높아진다고 느꼈고, 이를 피하기 위해 DTO로 변환해서 전달하는 방식이 더 적절하다고 판단했습니다. DTO를 사용하면 도메인을 보호할 수 있고, View와의 의존성도 끊을 수 있어서 변경에 더 유연하게 대응할 수 있다고 생각했습니다.

이렇게 나름대로 기준을 세워 정리해보았는데, 혹시 제가 놓치고 있거나 잘못 이해한 부분이 있다면 비밥의 의견을 들어보고 싶습니다.

아 저는 지금 controller에 대해 교육을 받은건가 해서 여쭤본거였는데요

지금은 이 부분보다 다른 부분에 집중하시는게 더 낫지 않나 싶습니다.

찾아보신 내용들은 잘 정리해서 계속해서 발전시키면 좋을것 같습니다. 👍

pci2676 · 2026-03-30T09:40:36Z

src/main/java/domain/position/Row.java

+        if (value <= 0 || value > 10) {
+            throw new IllegalArgumentException("행의 위치는 1-10 사이에 있어야 합니다.");
+        }


상수로 빼도 될 것 같네요.

충분히 상수로 두어도 괜찮은 값이였던것 같습니다! 바로 수정해두겠습니다

pci2676 · 2026-03-30T09:41:08Z

src/main/java/domain/piece/EmptyPiece.java

+
+import domain.position.Position;
+
+public class EmptyPiece implements Piece {


싱글톤 객체를 사용해볼까요?

이번에 싱글톤을 공부해보면서 여러가지 구현 방법이 있다는 것도 알았습니다.

특히 여러 개의 쓰레드가 동시에 요청하면 여러개의 싱글톤 객체가 생길 여지가 있어서 이를 어떻게 해결하려고 사람들이 고군분투해왔는지 그 과정이 인상 깊었던 것 같습니다. 현재 EmptyPiece에서는 Bill-pugh 방식으로 구성해봤는데요, JVM 레벨에서 쓰레드 안정성을 지원해서 성능과 효율을 둘 다 챙긴 방식이라고 배웠습니다. 만약 여러 사용자가 동시에 사용한다라는 요구사항이 생길 때 꼭 신경써야하는 부분이 아닐까? 생각하여 해당 방식으로 구성해봤는데, 제가 잘 적용한 부분인지 궁급합니다!!

비밥! 늦게 회신을 남겨드려 죄송합니다. 제 나름대로 납득할 수 있는 이유를 찾고, 관련된 자료를 찾아보며 정리하는 과정이 생각보다 오래 걸렸습니다. 단순히 피드백을 반영하는 데서 그치기보다 스스로 이해하고 설명할 수 있는 수준까지 가져가고 싶어서 시간이 조금 더 필요했던 것 같습니다.

아직 전체 일정이 어떻게 될지는 정확히 모르겠지만 만약 선택 미션(3주차)에서도 이어서 계속 더 발전시킬 수 있는 기회가 있다면 끝까지 파고들어보고 싶습니다. 이번에 고민했던 부분들이 단순한 구현 문제로 그치는 것보단 설계에 대한 기준을 세우는 과정이라고 느껴져서 가능하다면 더 깊게 가져가 보고 싶습니다.

그리고 한 가지 더 여쭤보고 싶습니다. 남은 레벨1 기간 동안 제가 어떤 부분에 집중하면 좋을지에 대해 조언을 얻고 싶습니다. 지금까지 진행하면서 객체지향 설계에 대해 나름대로 고민해왔다고 생각하지만, 아직 기준이 명확하게 잡혔다기보다는 상황에 따라 많이 흔들리는 느낌도 있습니다. 이런 상태에서 조금 더 단단한 기준을 만들기 위해 도움이 될 만한 서적이나, 직접 부딪혀보면서 배울 수 있는 과제가 있다면 추천해주실 수 있는게 있을지 궁금합니다!!

가능하다면 이 정도는 꼭 짚고 넘어가면 좋겠다라고 생각하시는 부분이나, 지금 단계에서 제가 보완하면 좋을 지점이 있다면 함께 말씀해주시면 정말 큰 도움이 될 것 같습니다.

남겨주신 리뷰 덕분에 많이 성장했습니다. 감사합니다!!

특히 여러 개의 쓰레드가 동시에 요청하면 여러개의 싱글톤 객체가 생길 여지가 있어서 이를 어떻게 해결하려고 사람들이 고군분투해왔는지 그 과정이 인상 깊었던 것 같습니다. 현재 EmptyPiece에서는 Bill-pugh 방식으로 구성해봤는데요, JVM 레벨에서 쓰레드 안정성을 지원해서 성능과 효율을 둘 다 챙긴 방식이라고 배웠습니다. 만약 여러 사용자가 동시에 사용한다라는 요구사항이 생길 때 꼭 신경써야하는 부분이 아닐까? 생각하여 해당 방식으로 구성해봤는데, 제가 잘 적용한 부분인지 궁급합니다!!

요거 런타임에 호출될때 싱글톤 객체 생성하겠다 인것 같네요. 저는 이정도라면 그냥 static 필드를 EmptyPiece에 두어도 괜찮아보여요. 지금 방식은 인스턴스 생성비용이 비싼데 안쓰일수도 있으면 좀 더 의미있는 방식으로 보이네요

그리고 한 가지 더 여쭤보고 싶습니다. 남은 레벨1 기간 동안 제가 어떤 부분에 집중하면 좋을지에 대해 조언을 얻고 싶습니다. 지금까지 진행하면서 객체지향 설계에 대해 나름대로 고민해왔다고 생각하지만, 아직 기준이 명확하게 잡혔다기보다는 상황에 따라 많이 흔들리는 느낌도 있습니다. 이런 상태에서 조금 더 단단한 기준을 만들기 위해 도움이 될 만한 서적이나, 직접 부딪혀보면서 배울 수 있는 과제가 있다면 추천해주실 수 있는게 있을지 궁금합니다!!

지금은 많은 크루들이랑 의견을 나누는게 가장 많고 빠르게 얻어갈 수 있는 부분이 있을것 같아요. 아무래도 같은 시선에 있는 사람들이랑 이야기를하면 컨텍스트 불일치가 적어서 상대방이 이야기하는것이 더 잘 이해가 되는것 같아서요.

하나의 미션이 다 끝나고 나면(ex. 장기미션) 이번에 작성한 코드중에 개선을 더하고 싶은 부분이나 고민이 많이 되는 부분을 잘 기억한다음 다른 크루들은 어떻게 구현했는지 살펴보는것도 도움이 될 것 같아요. 중요한건 미션이 끝난다음이어야 하는데요. 그래야 스스로 고민을 많이해서 깨닫는 부분이 많은것 같아요.

책은 이것저것 많을것 같긴한데.. 테스트 관련해서는 단위 테스트라는 책은 꼭 읽어보시면 좋을것 같아요. 저도 두번? 세번 읽은것 같은데 읽을때마다 다르게 보이는 면이 생기기도 했던것 같아서 좋은책 같습니다.

그리고 명서 클린아키텍처, 클린코드 추천드립니다.

마지막으로 이번미션과 앞으로의 미션을 수행하면서 개인적으로 항상 염두하고 코드를 작성하셨으면 하는 부분은
변경에 유연하고 확장에 용이한 구조를 생각하며 코드를 작성하셨으면 좋겠습니다. OCP 에요.

현업에서 가장 공수가 많이 드는 작업이 OCP를 지키지 않아 발생하는 일들이 많은것 같아서요. 지금부터 습관과 경험을 잘 만들어두시면 나중에 큰 도움이 되지 않을까 싶습니다.

…폴트 제거

pci2676

질문에 대한 답변과 추가 리뷰를 남겨두었어요.

리뷰에 대한 부분은 코드를 전반적으로 검토하셔서 비슷한 부분이 있다면 같이 개선해주세요.

pci2676 · 2026-04-02T15:50:34Z

src/main/java/domain/piece/ActivePiece.java

+
+    @Override
+    public boolean isAlly(Piece other) {
+        return other instanceof ActivePiece activePiece && this.team == activePiece.team;


다시 개선해보시죠~

pci2676 · 2026-04-02T15:50:40Z

src/main/java/domain/piece/Cannon.java

+        int count = 0;
+        for (Piece piece : piecesOnRoute) {
+            if (piece instanceof Cannon) {
+                throw new IllegalArgumentException("포는 포를 넘지 못합니다.");


여기도요~

pci2676 · 2026-04-02T15:50:46Z

src/main/java/domain/game/JanggiGame.java

+    }
+
+    public void move(Position source, Position destination) {
+        board.move(source, destination);


board.move()를 호출한 뒤 바로 turn = turn.next()를 하고 있는데요, 현재 턴의 기물인지 검증하는 로직이 보이지 않아요. 상대방 기물을 내 턴에 움직일 수 있는 상태인 것 같은데, 이 부분은 어디서 검증되고 있나요?

pci2676 · 2026-04-02T15:50:55Z

src/main/java/domain/position/Row.java

+    private void validate(int value) {
+        if (value <= 0 || value > 10) {
+            throw new IllegalArgumentException("행의 위치는 1-10 사이에 있어야 합니다.");
+        }


여기 상수 선언한거 사용하지 않고있는것 같아요?

pci2676 · 2026-04-02T15:51:05Z

src/main/java/domain/piece/Soldier.java

+import java.util.List;
+
+public class Soldier extends ActivePiece {
+    private static final List<Integer> rowOffsets = List.of(-1, 1);


상수 네이밍 법칙이 있지 않나요?

isValidSideMove 를 보았는데요

필드 이름과 비교 대상이 교차되어 있어서 읽는 사람 입장에서 혼란스러워요

조금더 간결하게 표현할 순 없을까요?

그리고 depth 조건을 위배했네요

pci2676 · 2026-04-02T15:51:14Z

src/main/java/view/InputView.java

+public class InputView {
+    private static final int MOVE_INPUT_COUNT = 4;
+    private final Scanner scanner = new Scanner(System.in);
+


initialFormation의 반환 타입이 Integer(래퍼)로 되어 있는데요, null을 반환할 의도가 있는게 아니라면 int(원시 타입)로 충분해 보여요.

pci2676 · 2026-04-02T15:51:22Z

src/main/java/domain/piece/PieceType.java

+    GENERAL(0.0, List.of(5), General::new),
+    PHO(7.0, List.of(2, 8), Cannon::new),
+    BYEONG(2.0, List.of(1, 3, 5, 7, 9), Soldier::new),
+    EMPTY(0.0, List.of(), team -> EmptyPiece.getInstance()),;


EMPTY 타입이 PieceType enum에 포함되어 있는데요, "빈 칸"이 기물의 한 종류인지 생각해보면 조금 어색한 부분이 있어보이네요. PieceType이 기물의 점수(score)와 초기 배치 열(initialColumns)을 갖고 있는데, EMPTY는 이 두 가지 모두 의미가 없거든요. EmptyPiece의 toString()을 위해서만 존재하는 것 같은데, 이 enum 값이 여기 있는 게 적절한지 한번 고민해보시면 좋겠어요.

pci2676 · 2026-04-02T16:08:52Z

src/main/java/domain/piece/Elephant.java

+    public List<Position> calculateRoute(Position source, Position target) {
+        if (source.columnDiff(target) == -3) {
+            Position mid = source.addPosition(0, 1);
+            return List.of(mid, mid.middlePosition(target));
+        }
+        if (source.columnDiff(target) == 3) {
+            Position mid = source.addPosition(0, -1);
+            return List.of(mid, mid.middlePosition(target));
+        }
+        if (source.rowDiff(target) == -3) {
+            Position mid = source.addPosition(1, 0);
+            return List.of(mid, mid.middlePosition(target));
+        }
+        Position mid = source.addPosition(-1, 0);
+        return List.of(mid, mid.middlePosition(target));
+    }


절차적으로 모든 방향에 대해 나열이 된것 같은데 추상화할수 없을까요?

이 부분은 쉽지 않을수는 있는데요 너무 어렵다 싶으면 안하셔도 되어요.

picetea44 and others added 30 commits March 25, 2026 15:31

docs: readme tset

cb93bfc

feat: Position 도메인 구현

0f8fb4d

feat: 기물 관련 도메인 구현

ec1afcb

feat: 초기 보드 구성 및 추상화 변경

81434bd

feat: 열과 행 비교 메서드 추가

4a7f437

feat: 실제 이동 기물 도메인 구현

ee11a83

feat: 실제 이동 기물 도메인 구현

b91473b

feat: 마,포,차 유효 이동 범위 구현

5ce6271

feat: 상, 궁, 졸, 사 유효 이동 범위 구현

9198a53

test: 기물별 테스트 구현

7e8456c

feat: 초기 formation 입력 기능 구현

9179f23

feat: BoardFactory 기능 추가

68304cd

feat: 배치 전략 구현

0d17919

feat: 차림 입력 후 보드 출력 구현

ccfa0e4

refactor: 공백 수정

95ed613

feat: Cha 이동 경로 구현1

212f889

feat: 포,차,궁,사 이동 경로 반환 메서드 구현

b586fd8

test: 궁,사 테스트 케이스 작성

f1ee429

feat: 마 기물 이동 경로 반환 로직 구현

bf366d0

feat: 상 기물 이동 경로 반환 메서드 구현

6db9326

feat: 병 기물 이동 경로 반환 메서드 구현

fbb1143

feat: 흐름 제어 테스트

99c9e0e

feat: 컨트롤러 수정

55ee04d

feat: 흐름 테스트 구현

b387b44

refactor: 기물 위치 이동 입력에 대한 파싱 책임 분리 (Position)

06de619

refactor: Turn 개념 도입

e8388ee

refactor: Board 내부 로직 캡슐화 진행

804af3d

refactor: 팀 판별 로직 개선 및 이동 규칙 과정 수정

08f237b

refactor: 네이밍 수정

3d7803c

refactor: 보드 출력시 열과 행 숫자 출력

91ee3f1

gahyeonnni and others added 10 commits March 28, 2026 21:07

refactor: 출력 메세지 수정

6f4a476

refactor: 도메인 패키지 재분류

9dec8bf

refactor: 메서드 로직 흐름 수정

45de3b0

refactor: 재시도 포함 게임 흐름 수정

9dbbbdd

feat: 포 이동 로직 구현

16ad9c7

refactor: AbstractBoardFactory if 분기 제거

7bc74d0

test: 기물 관련 테스트 수정

5343fff

test: Board 클래스 관련 Test 작성

036506b

test: Board, Turn, Position 누락 테스트 작성

78593ee

docs: 기능 요구사항 정리

32db79b

pci2676 requested changes Mar 30, 2026

View reviewed changes

pci2676 reviewed Mar 30, 2026

View reviewed changes

picetea44 added 15 commits March 31, 2026 17:48

refactor: EmptyPiece 싱글톤 화

7ef7e9d

refactor: Row, Column 매직 넘버 제거

ccae384

refactor: 오탈자, 에러출력 책임 아웃풋 뷰에 위임

494396b

refactor: dx, dy 축약어 삭제 및 static 추가

168cc30

refactor: 컨트롤러 책임 분리 및 JanggiGame 클래스 추가

b526872

refactor: Board null 반환 가능성 제거 및 축약어 제거

726d94c

refactor: Board에 불필요한 다운캐스팅 제거, searchRoute() Piece로 승격, isCannon() 디…

28ebeff

…폴트 제거

refactor: AbstractBoardFactory 입력 책임 제거

745799b

refactor: ActivePiece isSameTeam 제거 및 캐스팅 제거

8bcf443

refactor: PositionTest 구체적인 예외 메세지 확인 적용

9c423a6

refactor: 전체 파일 축약어 제거

b1455ff

refactor: searchRoute 이동 판별 책임 기물에 위임

e73c692

refactor: searchRoute -> calculateRoute로 메서드 명 변경

d151d75

refactor: isCannon 메서드 제거

92b4ee9

test: 무의미한 테스트 검증 개선

2466494

pci2676 requested changes Apr 2, 2026

View reviewed changes


		public class Elephant extends ActivePiece {

		private final List<Integer> dx = List.of(2, 3, -2, -3, -3, -2, 2, 3);


		import domain.position.Position;

		public class EmptyPiece implements Piece {

Conversation

picetea44 commented Mar 29, 2026

체크 리스트

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

Q1. searchRoute의 위치와 타입 캐스팅

Q2. 의미 없는 메서드 구현

Q3. 불변 객체의 Getter 사용

Q4. 반성

Uh oh!

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

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!