[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 아이큐 미션 제출합니다. by e9ua1 · Pull Request #335 · woowacourse/java-janggi

e9ua1 · 2026-04-08T10:43:27Z

체크 리스트

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

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

궁성 규칙 설계와 도메인과 인프라의 경계 를 중점적으로 피드백해 주시면 좋겠습니다.
궁성 규칙을 도입하면서 기물과 궁성의 책임을 어떻게 나눌지,
DB를 처음 연동하면서 도메인이 DB를 모르게 하려면 어디서 어떻게 경계를 그어야 하는지 계속 고민했습니다.

1. 궁성(Palace) 도메인 객체 설계

궁성 규칙을 구현하면서 Palace를 별도 도메인 객체로 분리했습니다.

public enum Palace {
    HAN(1, 3, 4, 6, 2, 5),
    CHO(8, 10, 4, 6, 9, 5);

    public boolean contains(Position position) { ... }
    public boolean isOnDiagonalLine(Position position) { ... }
    public boolean canMoveDiagonally(Position from, Position to) { ... }
    public static boolean isDiagonalMove(Position from, Position to) { ... }
    public static List<Position> getDiagonalPath(Position from, Position to) { ... }
}

General, Guard는 AbstractPiece.ownPalace()를 통해 자신의 팀 궁성을 참조하고,
Chariot, Cannon, Soldier는 Palace.isDiagonalMove()로 양쪽 궁성을 모두 확인합니다.

고민했던 점은 AbstractPiece가 Palace를 알아야 하는가 입니다.
기물이 자신의 궁성을 직접 참조하는 게 자연스러운지, 아니면 Board가 궁성 규칙을 판단해야 하는지 의견을 듣고 싶습니다.

2. DB 설계 — 트랜잭션 단위 및 업데이트 방식

게임 저장 시 janggi_game INSERT와 piece INSERT(90개)를 하나의 트랜잭션으로 묶었습니다.

public long save(Board board, Team currentTeam) {
    try (Connection conn = DBConnector.getConnection()) {
        conn.setAutoCommit(false);
        long gameId = saveGame(conn, currentTeam);
        savePieces(conn, gameId, board.showBoard());
        conn.commit();
        return gameId;
    } catch (SQLException e) {
        throw new IllegalStateException("[ERROR] 게임 저장 실패", e);
    }
}

매 턴마다 update() 시에도 updateTurn + deletePieces + savePieces를 하나의 트랜잭션으로 묶었습니다.

고민했던 점은 매 턴마다 90개 기물을 DELETE + INSERT하는 방식이 적절한가입니다.
현재는 구현 단순성을 위해 전체를 교체하는 방식을 택했는데,
게임이 여러 개 동시에 돌아가는 상황을 고려하면 변경된 기물(from, to 2개)만 UPDATE하는 방식이 나은지
트레이드오프에 대한 의견을 듣고 싶습니다.

3. Repository 패턴 — DAO 없이 구현체에서 직접 JDBC 처리

GameRepository (interface, domain)
└── GameRepositoryImpl (JDBC 직접 처리, infrastructure)

DAO 계층 없이 GameRepositoryImpl이 JDBC 처리까지 담당하도록 했습니다.
미션 규모에서는 DAO를 별도로 두는 게 과하다고 판단했는데, 이 판단이 합리적인지 의견을 듣고 싶습니다.

4. EmptyPiece의 팀 저장 방식

DB에 기물을 저장할 때 EmptyPiece는 팀이 없어서 "EMPTY"라는 특수값을 저장합니다.

public static String toTeamString(Piece piece) {
    if (piece.isEmptyPiece()) return "EMPTY";
    if (piece.isSame(Team.HAN)) return "HAN";
    return "CHO";
}

복원 시에는 type이 EMPTY면 team 값을 무시하고 EmptyPiece를 반환합니다.
이 방식 대신 EmptyPiece를 아예 저장하지 않고 없는 칸은 빈 칸으로 처리하는 희소 표현 방식도 고민했는데,
현재 방식의 장단점에 대해 의견을 듣고 싶습니다.

5. 점수 계산 — 덤(핸디캡) 적용 방식

초나라가 먼저 두는 규칙에 따라 한나라 점수에 1.5점 덤을 적용했습니다.

public Score applyHanBonus() {
    return new Score(value + HAN_BONUS);
}

Board.calculateScore()에서 팀이 HAN이면 자동으로 덤을 적용합니다.
덤 적용 책임이 Score에 있는 게 맞는지, 아니면 호출부에서 명시적으로 적용하는 게 나은지 의견을 듣고 싶습니다.

6. 게임 이력 관리 — 현재 상태만 저장 vs 이동 기록 누적

현재는 매 턴마다 보드의 현재 상태를 덮어씁니다. 때문에 게임 중 비정상 종료 시
마지막 저장 이전 상태로 돌아갈 방법이 없고, 무르기 같은 기능 추가도 불가능합니다.

이동 기록을 누적하는 movement 테이블을 추가하는 방식도 고민했습니다.

CREATE TABLE movement (
    id             BIGINT NOT NULL AUTO_INCREMENT,
    janggi_game_id BIGINT NOT NULL,
    source         VARCHAR(8) NOT NULL,
    destination    VARCHAR(8) NOT NULL,
    PRIMARY KEY (id)
);

이 경우 복원 시 초기 배치에서 이동 기록을 순서대로 재생해야 하는데,
현재 방식(스냅샷)과 이동 기록 누적 방식의 트레이드오프가 궁금합니다.

7. FakeGameRepository와 실제 DB 테스트의 괴리

public class FakeGameRepository implements GameRepository {
    private final Map<Long, Board> boardStore = new HashMap<>();
}

FakeGameRepository는 Board 객체를 그대로 저장하는 반면,
실제 GameRepositoryImpl은 직렬화(도메인 → DB) + 역직렬화(DB → 도메인) 과정이 있습니다.
이 과정에서 발생할 수 있는 버그를 FakeGameRepository로는 잡을 수 없습니다.

실제 DB 연동 테스트(통합 테스트)와 Fake 테스트의 적절한 균형을 어떻게 잡아야 하는지,
그리고 현재 미션 수준에서 통합 테스트까지 작성하는 게 적절한지 의견을 듣고 싶습니다.

8. DB 연결 방식 — 매 트랜잭션마다 새 Connection 생성

현재 DBConnector.getConnection()이 매 트랜잭션마다 새 커넥션을 생성합니다.

public static Connection getConnection() {
    return DriverManager.getConnection(URL, USER, PASSWORD);
}

실제 서비스에서는 커넥션 생성 비용이 높아 HikariCP 같은 커넥션 풀을 사용하는데,
현재 미션 수준에서 커넥션 풀 도입이 필요한지, 아니면 현재 방식으로 충분한지 의견을 듣고 싶습니다.

syoun602

안녕하세요 아이큐~

사이클2 아주 잘 구현해주셨네요 👍 질문에 대한 대답은 코멘트에 남겨놓았으니 확인 부탁드려요~

또한, 원활한 2단계 리뷰를 위해 step1이 머지된 브랜치를 기준으로 rebase해서 제출해주세요. 현재는 1단계 구현 내용도 함께 보여서 이번 단계에서 실제로 어떤 부분이 확장되었는지 보려면 제가 직접 커밋을 제어해야 하는데, rebase의 장점을 한번 찾아봅시다.

다음 요청 전에 적용 부탁드려요 😀

syoun602 · 2026-04-08T20:41:14Z

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

+    private Board(Map<Position, Piece> pieces) {
        this.pieces = pieces;
    }

+    public static Board from(Map<Position, Piece> pieces) {
+        return new Board(pieces);
+    }


public 생성자를 열어두는 것과 정적 팩터리 메서드를 사용하는 것은 어떤 차이가 있나요?

public 생성자를 열어두는 것과 정적 팩터리 메서드를 사용하는 것은 어떤 차이가 있나요?

public 생성자는 누구나 new Board(pieces)로 생성할 수 있어서 생성 방식을 제어하기 어렵습니다.
정적 팩토리 메서드 Board.from(pieces)를 사용하면 생성 의도를 명확히 표현할 수 있고,
이후 캐싱이나 검증 로직 추가 시 팩토리 메서드 내부만 수정하면 되어 변경에 더 유연합니다.
또한 생성자를 private으로 제한해 초기 생성(BoardFactory)과 복원(Repository) 경로를 통해서만 생성되도록 강제할 수 있어, 생성 경로를 명확하게 관리할 수 있다는 점에서 선택했습니다.

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

syoun602 · 2026-04-08T20:49:28Z

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

        return showBoard();
    }

+    public Score calculateScore(Team team) {


로직을 너무 흩어져 있는 것 같아요. 기물 점수는 pieceType이, 합산은 board가, 덤 적용은 score가 하고 최종 승패는 game이 진행하고 있어요. 누구에게 어떻게 제공해서 역할을 좀 더 모아볼 수 있을지 고민해봅시다.

로직을 너무 흩어져 있는 것 같아요. 기물 점수는 pieceType이, 합산은 board가, 덤 적용은 score가 하고 최종 승패는 game이 진행하고 있어요. 누구에게 어떻게 제공해서 역할을 좀 더 모아볼 수 있을지 고민해봅시다.

점수 계산 책임이 PieceType, Board, Score, JanggiGame에 분산되어 있었습니다.
ScoreResult 객체를 도입해 두 팀의 점수와 승패 판정(isHanWin, isChoWin, isDraw)을 한 곳으로 응집했습니다. Board.calculateScoreResult()가 최종 결과를 반환하고, JanggiGame은 출력만 담당하도록 개선했습니다.

syoun602 · 2026-04-08T20:50:04Z

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

+                .filter(piece -> piece.isSame(team))
+                .mapToInt(piece -> piece.getType().getScore())
+                .sum();
+        Score score = Score.of(sum);


이 부분도 그렇고 전반적으로 정적 팩터리를 사용하시는 이유가 궁금합니다.
new Score(10) vs Score.from(10)

또한, from 과 of 사용에 기준이 없는 것 같네요

이 부분도 그렇고 전반적으로 정적 팩터리를 사용하시는 이유가 궁금합니다. new Score(10) vs Score.from(10)

또한, from 과 of 사용에 기준이 없는 것 같네요

정적 팩토리를 사용한 이유는 생성자를 private으로 제한하여 생성 경로를 명확히 하고,
이후 검증이나 캐싱 로직 추가 시 내부만 수정하면 되는 유연함을 얻기 위해서입니다.
of와 from의 사용 기준은 다음과 같습니다.
of는 인자를 그대로 감쌀 때 (예: Score.of(72.5), Position.of(1, 5)),
from은 변환이 수반될 때 (예: Position.from("25") → 문자열 파싱, Board.from(map))로 구분하여 사용했습니다.

우선 검증에 대해서는 말씀하신 것처럼 생성 경로를 통제하고 싶더라도 결국 주생성자에서 항상 검증해야 된다고 생각해요. 주생성자는 어떤 방식으로 객체를 만들더라도 항상 거치게 되는 마지막 보호 장치니까요. 그리고 캐싱은 지금 사용하고 있지 않은데, 그걸 고려한다는게 크게 와닿지는 않네요.

마지막으로 of와 from의 기준 자체는 충분히 이해되지만, 현재 코드에서 이 이름들이 정말 생성 의도를 잘 드러내고 있는지는 한 번 더 생각해보면 좋을 것 같아요. 저는 개인적으로 무분별하게 정적 팩터리 메서드를 늘리기보다는 하나의 안전한 주생성자를 두고 필요한 경우에만 의미 있는 부생성자나 정적 팩터리를 추가하는 방식을 선호합니다. 지금 Score만 봤을때 정적 팩터리의 어떤 장점이 녹아들었는지 잘 모르겠습니다.

단순히 이펙티브 자바에서 정적 팩터리를 선호한다고 해서 일괄적으로 적용하기보다는 왜 이 경우에 생성자보다 정적 팩터리가 더 적절한지까지 함께 설명 가능해야 의미가 있다고 생각해요. 이펙티브 자바에서 말하는 정적 팩터리도 항상 생성자보다 우월하다는 취지라기보다, 이름을 부여할 수 있고 생성 방식을 통제할 수 있는 등 분명한 이점이 있을 때 선택지로 고려하라는 의미라고 생각해요.

syoun602 · 2026-04-08T20:56:45Z

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

+    }
+
+    protected void validatePalaceMove(Position from, Position to, String errorMessage) {
+        Palace palace = ownPalace();


우선 메서드 이름이 palace를 반환할 것 같은 느낌은 안들어요 🤔

고민해볼 포인트는 AbstractPiece에 ownPalace()가 들어가면서, 상위 타입이 팀뿐 아니라 궁성이라는 비교적 구체적인 장기 규칙까지 함께 알고 있는 구조가 됐어요. 현재 규모에서는 구현을 단순하게 만드는 장점도 있지만, 추상 기물의 책임을 어디까지로 볼지에 따라 이런 지식을 계속 상위 타입에 둘지 한 번 더 고민 정도는 해보면 좋을 것 같네요.

예를 들어 팀과 궁성의 대응 책임을 Palace 쪽으로 옮기는 방향도 있을 것 같아요. 변경하지 않는다면, palace에게 team을 주고 진영을 받는 메서드가 있는게 더 읽기 좋아보입니다.

우선 메서드 이름이 palace를 반환할 것 같은 느낌은 안들어요 🤔

고민해볼 포인트는 AbstractPiece에 ownPalace()가 들어가면서, 상위 타입이 팀뿐 아니라 궁성이라는 비교적 구체적인 장기 규칙까지 함께 알고 있는 구조가 됐어요. 현재 규모에서는 구현을 단순하게 만드는 장점도 있지만, 추상 기물의 책임을 어디까지로 볼지에 따라 이런 지식을 계속 상위 타입에 둘지 한 번 더 고민 정도는 해보면 좋을 것 같네요.

예를 들어 팀과 궁성의 대응 책임을 Palace 쪽으로 옮기는 방향도 있을 것 같아요. 변경하지 않는다면, palace에게 team을 주고 진영을 받는 메서드가 있는게 더 읽기 좋아보입니다.

팀과 궁성의 매핑 책임을 Palace.of(Team)으로 이동했습니다.
ownPalace()는 Palace.of(team)에 위임만 하는 형태로 단순화하여,
AbstractPiece가 궁성 규칙의 세부사항을 직접 알지 않도록 개선했습니다.

syoun602 · 2026-04-08T21:12:09Z

src/main/java/janggi/JanggiGame.java

+    private final long gameId;

-    private JanggiGame(Board board) {
+    private JanggiGame(Board board, GameRepository gameRepository, long gameId) {


JanggiGame은 아이큐가 생각했을때 도메인인가요? 어플리케이션 계층인가요?

JanggiGame은 아이큐가 생각했을때 도메인인가요? 어플리케이션 계층인가요?

JanggiGame은 애플리케이션 계층으로 보고 있습니다.
게임의 흐름 제어(play, progressTurn)와 입출력(InputView, OutputView) 사이를 연결하는 역할을 하고 있어,
순수한 도메인 로직보다는 유스케이스를 조율하는 책임에 가깝다고 판단했습니다.
다만 Board, GameRepository 같은 도메인 객체에 직접 의존하고 있어 경계가 명확하지 않은 부분이 있는데,
이 부분에 대해 어떻게 분리하는 것이 좋을지 의견을 듣고 싶습니다.

지금은 JanggiGame은 InputView, OutputView, GameRepository까지 함께 알고 있어서 순수한 도메인 객체라고 보기는 어렵습니다. 그래서 지금 구조는 애플리케이션 객체라고 정리할 수도 있다고 생각하고 크게 이견은 없습니다~

다만 개인적으로는 게임 자체는 도메인이라고 생각해요. 턴이 어떻게 진행되는지, 언제 게임이 종료되는지, 이동이 성공했을 때 상태가 어떻게 바뀌는지 같은 흐름 자체가 장기 규칙의 일부니까요. 단순히 화면과 저장소를 연결하는 절차라기보다 게임이라는 도메인이 스스로 설명해야 하는 상태 변화에 더 가깝지 않나요?

예를 들어 이후 웹 버전까지 함께 고려한다고 했을 때, 입력을 어떻게 받고 결과를 어떻게 보여줄지는 분명 애플리케이션 계층이 바뀌는 문제인데, 게임의 흐름 자체는 그대로 유지되어야 하잖아요. 그런데 지금 구조는 그 흐름까지 InputView, OutputView, Repository와 한 객체 안에 묶여 있어서, 인터페이스가 바뀌면 결국 새로운 게임 흐름 객체를 다시 만들게 될 가능성이 커 보여요. 그래서 제 기준에서는 콘솔이든 웹이든 상관없이 공통으로 유지되어야 하는 턴 진행, 종료 판단, 상태 변화 같은 게임 규칙은 도메인에 남고, 입출력과 저장/복구 orchestration만 바깥 계층이 맡아야 한다고 생각합니다.

syoun602 · 2026-04-08T21:17:40Z

src/main/java/janggi/infrastructure/GameRepositoryImpl.java

+
+    @Override
+    public long save(Board board, Team currentTeam) {
+        try (Connection conn = DBConnector.getConnection()) {


jdbc사용과 하나의 transaction을 보장한 점 👍

DB 설계 — 트랜잭션 단위 및 업데이트 방식

게임 이력 관리 — 현재 상태만 저장 vs 이동 기록 누적

현재 미션 규모에서는 delete + insert도 충분히 합리적인 선택이라고 생각합니다. 성능보다 구현 단순성과 상태 일관성이 지금은 더 중요해보이고, 실제로 변경된 두 칸만 update하는 방식은 더 효율적일 수는 있어도 이동, 캡처, 빈칸 복구까지 함께 세심하게 관리해야 해서 복잡도가 커진다고 생각해요. 현재 규모에서는 성능 비용도 크지 않아 보여서, 지금 선택이 더 적절하다고 봅니다.

다만 실제 서비스라고 가정하면 성능도 함께 고려해야 해요. 매 턴마다 90개 기물을 전부 delete + insert하는 방식은 구현은 단순하지만, 게임 수가 많아질수록 불필요한 쓰기 비용이 커지기도 하고, 이동 히스토리를 남기거나 무르기 같은 기능까지 고려한다면 전체 상태를 매번 교체하기보다는 변경된 칸만 갱신하거나 이동 기록을 별도로 관리하는 구조가 더 좋아보입니다.

결국 이런 부분은 항상 현재 규모와 요구사항을 함께 고려해서 본인이 선택하는 부분이라고 생각해요

jdbc사용과 하나의 transaction을 보장한 점 👍

DB 설계 — 트랜잭션 단위 및 업데이트 방식

게임 이력 관리 — 현재 상태만 저장 vs 이동 기록 누적

현재 미션 규모에서는 delete + insert도 충분히 합리적인 선택이라고 생각합니다. 성능보다 구현 단순성과 상태 일관성이 지금은 더 중요해보이고, 실제로 변경된 두 칸만 update하는 방식은 더 효율적일 수는 있어도 이동, 캡처, 빈칸 복구까지 함께 세심하게 관리해야 해서 복잡도가 커진다고 생각해요. 현재 규모에서는 성능 비용도 크지 않아 보여서, 지금 선택이 더 적절하다고 봅니다.

다만 실제 서비스라고 가정하면 성능도 함께 고려해야 해요. 매 턴마다 90개 기물을 전부 delete + insert하는 방식은 구현은 단순하지만, 게임 수가 많아질수록 불필요한 쓰기 비용이 커지기도 하고, 이동 히스토리를 남기거나 무르기 같은 기능까지 고려한다면 전체 상태를 매번 교체하기보다는 변경된 칸만 갱신하거나 이동 기록을 별도로 관리하는 구조가 더 좋아보입니다.

결국 이런 부분은 항상 현재 규모와 요구사항을 함께 고려해서 본인이 선택하는 부분이라고 생각해요

현재 규모에서 delete + insert 방식이 적절하다는 점 동의합니다.
이후 이동 히스토리나 무르기 기능이 필요해지는 시점에 movement 테이블 도입을 고려하겠습니다. 감사합니다!

syoun602 · 2026-04-08T21:30:51Z

src/main/java/janggi/infrastructure/GameRepositoryImpl.java

+import java.util.Map;
+import java.util.Optional;
+
+public class GameRepositoryImpl implements GameRepository {


Repository 패턴 — DAO 없이 구현체에서 직접 JDBC 처리

Dao와 repository의 차이가 뭔가요? 차이를 명확히 알고 설계했다면 이견 없습니다 😀

Repository 패턴 — DAO 없이 구현체에서 직접 JDBC 처리

Dao와 repository의 차이가 뭔가요? 차이를 명확히 알고 설계했다면 이견 없습니다 😀

DAO는 특정 테이블 단위의 CRUD에 집중하며 ResultSet 같은 DB 구현 세부사항을 직접 다루는 반면,
Repository는 도메인 객체 단위로 추상화하여 저장소의 구현 방식을 숨깁니다.
현재 GameRepositoryImpl은 인터페이스(GameRepository)로 도메인과 경계를 나누고,
내부에서 JDBC를 직접 처리하는 구조입니다. DAO를 별도로 두면 계층이 더 명확해지지만,
현재 미션 규모에서는 단일 구현체 안에서 처리하는 것이 복잡도 대비 실익이 크지 않다고 판단해
Repository 구현체가 JDBC까지 담당하도록 했습니다.

syoun602 · 2026-04-08T21:33:16Z

src/main/java/janggi/infrastructure/PieceFactory.java

+        throw new IllegalArgumentException("[ERROR] 알 수 없는 기물 타입입니다: " + type);
+    }
+
+    public static String toTeamString(Piece piece) {


EmptyPiece의 팀 저장 방식

두 방식 중에 아이큐가 생각한 장단점은 무엇이고 왜 현재의 스냅샷 방식을 선택했는지 알려주세요

EmptyPiece의 팀 저장 방식

두 방식 중에 아이큐가 생각한 장단점은 무엇이고 왜 현재의 스냅샷 방식을 선택했는지 알려주세요

두 방식의 장단점을 비교했을 때, 희소 표현(실제 기물만 저장) 방식은 저장 공간을 절약할 수 있지만 복원 시 없는 칸을 EmptyPiece로 채우는 별도 로직이 필요하고,
null 체크가 곳곳에 생길 수 있어 EmptyPiece라는 Null Object 패턴의 장점이 희석된다고 판단했습니다.
현재 방식(전체 저장)은 90칸을 모두 저장하는 비용이 있지만,
복원 시 항상 완전한 보드 상태를 보장하고 EmptyPiece의 일관성을 유지할 수 있어
현재 규모에서는 더 적합하다고 판단해 선택했습니다.

src/main/java/janggi/infrastructure/DBConnector.java

- canMoveOnDiagonalInAnyPalace → isDiagonalMove로 네이밍 개선 - getDiagonalPathInAnyPalace → getDiagonalPath로 네이밍 개선 - AbstractPiece에 validatePalaceMove() 추출 - General/Guard의 중복된 궁성 이동 검증 로직 제거

…legalStateException 변경

- ScoreResult 도메인 객체 추가 (hanScore, choScore, 승패 판정) - Board.calculateScoreResult()로 두 팀 점수를 한번에 반환 - JanggiGame의 승패 판정 로직을 ScoreResult에 위임

e9ua1 · 2026-04-10T17:23:39Z

안녕하세요 아이큐~

사이클2 아주 잘 구현해주셨네요 👍 질문에 대한 대답은 코멘트에 남겨놓았으니 확인 부탁드려요~

또한, 원활한 2단계 리뷰를 위해 step1이 머지된 브랜치를 기준으로 rebase해서 제출해주세요. 현재는 1단계 구현 내용도 함께 보여서 이번 단계에서 실제로 어떤 부분이 확장되었는지 보려면 제가 직접 커밋을 제어해야 하는데, rebase의 장점을 한번 찾아봅시다.

다음 요청 전에 적용 부탁드려요 😀

안녕하세요 썬! 꼼꼼한 리뷰 감사합니다 😊
step1 머지 브랜치 기준으로 rebase 완료했습니다.
이번 단계에서 확장된 부분만 보이도록 정리했으니 확인 부탁드립니다!
각 코멘트에 답글과 리팩토링 반영했으니 확인해주시고, 추가 피드백 부탁드립니다!

syoun602

안녕하세요 아이큐~
이번 사이클2는 충분히 잘해주신 것 같아서 이만 머지하겠습니다!
어려운 장기 미션 하시느라 수고 많으셨어요 👍
몇 가지 고민해볼 부분을 남겨드렸는데, 혹시나 더 궁금한 부분이 있다면 언제든 DM 주세요
레벨 1 고생하셨고 다음 웹 미션들도 화이팅입니다~

syoun602 · 2026-04-11T12:43:00Z

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

+                .filter(piece -> piece.isSame(team))
+                .mapToInt(piece -> piece.getType().getScore())
+                .sum();
+        Score score = Score.of(sum);


우선 검증에 대해서는 말씀하신 것처럼 생성 경로를 통제하고 싶더라도 결국 주생성자에서 항상 검증해야 된다고 생각해요. 주생성자는 어떤 방식으로 객체를 만들더라도 항상 거치게 되는 마지막 보호 장치니까요. 그리고 캐싱은 지금 사용하고 있지 않은데, 그걸 고려한다는게 크게 와닿지는 않네요.

마지막으로 of와 from의 기준 자체는 충분히 이해되지만, 현재 코드에서 이 이름들이 정말 생성 의도를 잘 드러내고 있는지는 한 번 더 생각해보면 좋을 것 같아요. 저는 개인적으로 무분별하게 정적 팩터리 메서드를 늘리기보다는 하나의 안전한 주생성자를 두고 필요한 경우에만 의미 있는 부생성자나 정적 팩터리를 추가하는 방식을 선호합니다. 지금 Score만 봤을때 정적 팩터리의 어떤 장점이 녹아들었는지 잘 모르겠습니다.

단순히 이펙티브 자바에서 정적 팩터리를 선호한다고 해서 일괄적으로 적용하기보다는 왜 이 경우에 생성자보다 정적 팩터리가 더 적절한지까지 함께 설명 가능해야 의미가 있다고 생각해요. 이펙티브 자바에서 말하는 정적 팩터리도 항상 생성자보다 우월하다는 취지라기보다, 이름을 부여할 수 있고 생성 방식을 통제할 수 있는 등 분명한 이점이 있을 때 선택지로 고려하라는 의미라고 생각해요.

syoun602 · 2026-04-11T12:53:47Z

src/main/java/janggi/JanggiGame.java

+    private final long gameId;

-    private JanggiGame(Board board) {
+    private JanggiGame(Board board, GameRepository gameRepository, long gameId) {


지금은 JanggiGame은 InputView, OutputView, GameRepository까지 함께 알고 있어서 순수한 도메인 객체라고 보기는 어렵습니다. 그래서 지금 구조는 애플리케이션 객체라고 정리할 수도 있다고 생각하고 크게 이견은 없습니다~

다만 개인적으로는 게임 자체는 도메인이라고 생각해요. 턴이 어떻게 진행되는지, 언제 게임이 종료되는지, 이동이 성공했을 때 상태가 어떻게 바뀌는지 같은 흐름 자체가 장기 규칙의 일부니까요. 단순히 화면과 저장소를 연결하는 절차라기보다 게임이라는 도메인이 스스로 설명해야 하는 상태 변화에 더 가깝지 않나요?

예를 들어 이후 웹 버전까지 함께 고려한다고 했을 때, 입력을 어떻게 받고 결과를 어떻게 보여줄지는 분명 애플리케이션 계층이 바뀌는 문제인데, 게임의 흐름 자체는 그대로 유지되어야 하잖아요. 그런데 지금 구조는 그 흐름까지 InputView, OutputView, Repository와 한 객체 안에 묶여 있어서, 인터페이스가 바뀌면 결국 새로운 게임 흐름 객체를 다시 만들게 될 가능성이 커 보여요. 그래서 제 기준에서는 콘솔이든 웹이든 상관없이 공통으로 유지되어야 하는 턴 진행, 종료 판단, 상태 변화 같은 게임 규칙은 도메인에 남고, 입출력과 저장/복구 orchestration만 바깥 계층이 맡아야 한다고 생각합니다.

syoun602 · 2026-04-11T12:56:25Z

src/main/java/janggi/domain/position/Palace.java

+    private boolean isCornerPosition(Position position) {
+        int row = position.getRowValue();
+        int col = position.getColumnValue();
+        return (row == topLeft.getRowValue() || row == bottomRight.getRowValue())


hasSameRowValue 등처럼 객체에게 메시지를 보냅시다

syoun602 · 2026-04-11T13:02:22Z

src/main/java/janggi/domain/position/Palace.java

+        return List.of(center);
+    }
+
+    public static boolean isDiagonalMove(Position from, Position to) {


지금 이런 static 메서드는 isDiagonalMove()처럼 양쪽 궁성을 모두 확인해야 하는데 먼저 이동 시작 위치가 어느 궁성에 속하는지 찾고 해당 궁성 객체에게 판단을 위임할 수도 있겠네요. 객체로 만든만큼 유틸성 메서드를 제거해볼 수도 있을 것 같아요

syoun602 requested changes Apr 8, 2026

View reviewed changes

e9ua1 added 29 commits April 10, 2026 22:21

feat: Palace 도메인 객체 추가 (궁성 범위 및 대각선 규칙)

ff69636

feat: General/Guard 궁성 이동 규칙 적용

c6e261c

feat: Chariot/Cannon 궁성 대각선 이동 적용

7db48d4

feat: Soldier 궁성 내 전진 대각선 이동 적용

ce3a036

feat: Score 값 객체 추가

1acb581

feat: Board 점수 계산 기능 추가

eb26f67

feat: 게임 종료 시 점수 출력 및 승패 판정 기능 추가

f833fa8

docs: 궁성 규칙 및 점수 계산 기능 목록 추가

b64d64d

feat: DB 연결 설정 추가

51c39fe

feat: GameRepository 인터페이스 추가

0825773

feat: PieceFactory 추가 (DB ↔ 도메인 변환)

7d3a996

feat: GameRepositoryImpl 추가 (JDBC 구현체)

cb98270

feat: FakeGameRepository 추가 및 테스트 작성

2ae2c2b

feat: JanggiGame에 Repository 주입 및 게임 이어하기 기능 추가

eead766

build: MySQL 커넥터 의존성 추가

f65fca7

build: application 플러그인 및 mainClass 설정 추가

2b129e5

docs: 실행 방법 추가 및 DB 비밀번호 제거

e5b05ff

refactor: JanggiApplication main 메서드 단순화

8a3e097

refactor: DBConnector 예외를 IllegalStateException으로 변경

395cefe

refactor: toTeamString() 책임을 PieceFactory로 이동 및 RuntimeException → Il…

799a406

…legalStateException 변경

refactor: save/update 트랜잭션 처리 추가

09c9b3a

refactor: GameState에 PLAYING 초기 상태 추가로 while 루프 초기값 명확화

e91cdf6

refactor: Board 생성자를 private으로 변경하고 정적 팩토리 메서드 from() 추가

2ec7161

refactor: findBoardById()에 게임 존재 여부 사전 검증 추가

12945d7

refactor: BoardFactory 이중 for문을 스트림으로 변경 (depth 규칙 준수)

ca17737

refactor: movePiece()에서 입력 파싱 로직을 parsePositions()로 분리

a32a6cb

refactor: Board 입력 검증 순서 변경 (빈칸 → 같은 좌표 → 턴)

43c9d67

refactor: Path → PiecesOnPath로 rename (경로 위 기물 상태 의도 명확화)

6ef0cf7

e9ua1 added 2 commits April 10, 2026 22:23

refactor: 점수 계산 결과를 ScoreResult로 응집화

2fa1c0e

- ScoreResult 도메인 객체 추가 (hanScore, choScore, 승패 판정) - Board.calculateScoreResult()로 두 팀 점수를 한번에 반환 - JanggiGame의 승패 판정 로직을 ScoreResult에 위임

fix: 리베이스 충돌 해결 및 PiecesOnPath 적용 복구

7d37b7c

e9ua1 force-pushed the step2 branch from e29580a to 7d37b7c Compare April 10, 2026 17:00

e9ua1 added 3 commits April 11, 2026 02:08

refactor: hasDistancePair → matchesDistance로 메서드명 개선

02fb6b4

refactor: Palace 내부 원시값을 Position으로 표현하고 of(Team) 추가

71c53a6

refactor: FakeGameRepository를 test 패키지로 이동

00e6f32

syoun602 approved these changes Apr 11, 2026

View reviewed changes

syoun602 merged commit 54cb05d into woowacourse:e9ua1 Apr 11, 2026

Conversation

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

Uh oh!

체크 리스트

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

1. 궁성(Palace) 도메인 객체 설계

2. DB 설계 — 트랜잭션 단위 및 업데이트 방식

3. Repository 패턴 — DAO 없이 구현체에서 직접 JDBC 처리

4. EmptyPiece의 팀 저장 방식

5. 점수 계산 — 덤(핸디캡) 적용 방식

6. 게임 이력 관리 — 현재 상태만 저장 vs 이동 기록 누적

7. FakeGameRepository와 실제 DB 테스트의 괴리

8. DB 연결 방식 — 매 트랜잭션마다 새 Connection 생성

Uh oh!

syoun602 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e9ua1 Apr 10, 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!

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!

e9ua1 Apr 10, 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!

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!

e9ua1 commented Apr 10, 2026

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

e9ua1 commented Apr 8, 2026 •

edited

Loading

e9ua1 Apr 10, 2026 •

edited

Loading

e9ua1 Apr 10, 2026 •

edited

Loading