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

boyekim · 2026-04-08T12:24:06Z

체크 리스트

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

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

리뷰 요청이 늦어진 점 죄송합니다...

DB 적용

DB는 SQLite를 사용했습니다. 이유는 요구사항을 충족시키려면 파일 DB여야하고, H2는 이전에 예약어 때문에 귀찮았던 경험이 있어 반감이 있습니다. 그래서 한번도 안써본 SQLite를 사용해봤습니다.
DB에 어떤 정보를 저장할지 고민한 끝에, 보드 상태 전체가 아니라 이동 정보만 저장하도록 구성했습니다.
결론은 이전에 에코가 남겨주신 이동 정보만 남겨보았습니다! 이유는 어차피 애플리케이션 재시작 시 Turn 정보를 재구성하기 때문입니다. 이 방식은 보드 스냅샷을 직접 저장하는 것보다 저장 구조가 단순하고, 기존 doGame / move 흐름을 그대로 재사용할 수 있다는 장점이 있다고 판단했어요.

이전 코멘트 적용

Board를 인자로 받을 경우 Board <-> Piece 양방향 의존성이 생깁니다. Piece의 변경에 Board가 영향을 받고, Board 변경에 Piece가 영향을 받는데요, 이 부분 어떻게 해결할 수 있을까요?

해당 코멘트 적용 방식은 다음과 같습니다.
AS-IS

Piece가 Board를 직접 받아 장애물을 검증한다.
이 과정에서 Board와 Piece 사이에 양방향 의존성이 생긴다.

TO-BE

Board가 이동 경로의 위치 정보를 수집해 DTO(MoveRoute)로 만든다.
Piece는 Board 대신 MoveRoute를 받아 이동 가능 여부를 검증한다.

이 변경을 통해 Piece가 보드 전체 구조를 직접 알 필요가 없어졌고, Board와 Piece 사이의 결합도를 낮출 수 있었습니다.

echo724

안녕하세요 보예!
저도 리뷰가 늦어진 점 죄송합니다..
일단 공성과 DB 요구사항 깔끔하게 잘 처리하신 것 같아요! 기존 턴제 기반 구조에 맞게 디비에도 이동 정보 기반으로만 잘 담아서 표현하신 것 같습니다. 다른 크루들의 구조를 보면 트랜젝션을 이용해서 피스들을 전체 업데이트 해야하는 구조인데, 현재 보예 코드 기준으로는 트랜젝션이 필요하지도 않아서 좋은 설계가 아닌가 생각이 듭니다! 오히려 보드 전체를 담았다면 굳이 필요하지 않은 정보들까지 영속화가 되어 디스크에 부담을 주었을 것 같습니다. 리플레이가 된다면 굳이 보드의 스냅샷을 영속화 할 필요가 있을까요? 물론 턴이 수백턴이 넘어간다면 스냅샷을 중간중간 고려해볼 수 있겠지만 모든 턴에 보드 정보를 담는것은 어려웠을 것 같습니다. 그런 의믜에서 저는 현재 구조가 훨씬 낫다고 생각합니다.

그리고 이전 커멘트도 잘 반영해주신 것 같습니다! 굳이 피스에게 있어서 보드의 구현 내용을 알 필요는 없으니 좋은 방향으로 설계 해주신 것 같아요!

이번 리퀘스트 체인지에서는 1. Service, Game 객체들의 결합 고민 2. 서비스, 레파지토리 테스트를 추가로 해주셨으면 합니다. 1번의 경우 커멘트를 남겼고, 2번의 경우에는 디비의 경우에는 특히 배포 환경, 테스트 환경에 따라 다른 DB를 사용한다거나 하는 식으로 외부 영향을 받습니다. 1. 어떻게 하면 이 영속화 계층과 계층을 포함해서 테스트를 짤 수 있을까 2. 어떻게 하면 프로덕션 데이터베이스 없이 테스트할 수 있을지에 대해서 고민해보시고 직접 테스트 작성해보시면 좋을 것 같습니다! 혹시 애매한 부분은 커멘트 달아주시거나 DM 주세요 화이팅입니다!

echo724 · 2026-04-10T12:27:39Z

src/main/java/janggi/persistence/dto/MoveHistory.java

+package janggi.persistence.dto;
+
+public record MoveHistory(
+    int turnNumber,


턴 정보가 저장은 되는데 따로 쓰이는 곳은 없는 것 같네요. 혹시 넣은 이유가 있을까요?

포함해두면 유용할 것 같아서 추가했었습니다!
가장 최근 턴을 가져온다거나, 몇 번 만에 게임이 종료되었는지 등 기본 게임 정보에 해당한다고 생각해서 넣었었는데요, 코드 상에서는 사용이 안되니 제거하는게 나을까요?

echo724 · 2026-04-10T12:29:10Z

src/main/java/janggi/JanggiRunner.java

+    private boolean isCancelCommand(String input) {
+        return CANCEL.equalsIgnoreCase(input.trim());
    }


오 취소 커멘드 좋네요! 실제 실행해보면서 있으면 좋겠다 생각했었는데 잘 처리하셨네요!

다만 CANCEL이 JanggiRunner에 있어도 되는지에 대해서는 고민해보시면 좋을 것 같습니다. inputView에서 처리해주는게 맞지 않을까요?

제가 정의한 cancel은

이후 좌표 검증으로 들어가기 전에 취소해야함

장기 게임 규칙이라고 보기 어려움

콘솔과 사용자의 상호작용임

이런 특징을 가지고 있다고 생각했습니다.
따라서 게임과 사용자의 상호작용보다는 그 이전단계인 콘솔과의 상호작용이라고 생각해서 Runner에 위치시켰었습니다.
하지만 게임과 관련된 것이 아니니 아예 InputView가 처리해줘도 좋을 것 같네요. 오히려 Runner가 몰라도 되는 입력 규칙을 알고있게 되는 것 같아요.
inputView에 readCancelableLine라는 메서드를 추가해줘보았습니다.
어때보이나요?!

echo724 · 2026-04-10T13:33:29Z

src/main/java/janggi/JanggiRunner.java

+    private void play(JanggiGame janggiGame, Position startPosition, Optional<Position> endPosition) {
+        janggiGame.doGame(startPosition, endPosition.get());
+        janggiService.saveMove(janggiGame, startPosition, endPosition.get());
+    }


장기 게임과 장기 서비스 라이프 사이클이 비슷하기도 하고, 사실 janggiGame이 Repository를 필드로 갖고 있으면 Service가 딱히 필요해보이지 않을 것 같은데 어떻게 생각하시나요? 게임이 게임 실행 + 저장을 맡아서 하는 셈인거죠

해당 구조의 장점은 게임을 실행한 다음 저장하는 간극을 줄일 수 있다고 생각합니다. 예를 들어 장기말은 인메모리 상에서 성공적으로 움직여졌는데 모종의 이유로 Service 쪽에서 저장이 안될 경우도 있잖아요? 그러면 인메모리와 영속 상태를 어떻게 일치시킬 수 있을까에 대한 고민도 필요하다고 생각합니다.

영속에 대한 책임은 기존 게임 실행과 시점이 분리되어있다고 생각해서, 게임 실행 + 저장을 분리해주려 했었습니다.

하지만 간극을 줄이는 것이 중요할 것 같아서 생각을 전개해보았는데요,
Service에서 게임 실행과 저장을 들고 있는 것은 어떨까요?
Input,Output에 대한 사용자와의 상호작용을 Runner가 하고, 게임 진행 및 저장을 Service가 해준다면 간극을 줄일 수 있고 역할도 분리할 수 있을 것 같다고 생각했습니다.
e95429f 해당 커밋에서 반영해보았습니다!

echo724 · 2026-04-10T13:36:59Z

src/test/java/janggi/domain/JanggiGameTest.java

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


현재 서비스에 대한 테스트는 보이지 않네요? DB에 대한 의존성만 해결해주면 작성이 가능할 것 같은데 어떻게 해결할 수 있을까요?

Fake객체를 통해 서비스 테스트를 추가해보았습니다.
다음 두가지에서 고민했었는데요,

test용 파일 db를 판다.

fake 객체를 만든다.

하지만 관리에 대한 비용, 그리고 Service 로직만이 실패 원인이 아닐 수도 있게 될 것 같아서....우선 2번을 택해보았습니다.

echo724 · 2026-04-10T13:41:52Z

src/main/java/janggi/persistence/repository/JdbcJanggiGameRepository.java

+        } catch (SQLException e) {
+            throw new IllegalStateException("수 저장에 실패했습니다.", e);
+        }


수 저장 실패에 대한 처리가 없고 예외가 터지면 종료되는 구조네요. 항상 디비 관련은 상태 불일치에 대한 고민이 필요하다고 생각하는데 게임을 종료하는 것 대신 해볼 수 있는 사항이 없을까요? 재시도를 한다던지 하는식으로요. 그리고 executeUpdate 주석에는 SQLTimeoutException이 던져진다고도 하는데 타임 아웃에 대한 예외처리도 필요하지 않을까요? 현업에서는 특히 타임아웃 관련 예외가 많이 발생하는데 이 부분도 어떻게 처리할지 또 상태 불일치는 어떻게 해결할 수 있을지 고민해보면 좋을 것 같습니다.

echo724 · 2026-04-10T13:45:59Z

src/main/java/janggi/JanggiRunner.java

+            janggiGame.findWinner()
+                .orElseThrow(() -> new IllegalStateException("승자가 존재하지 않습니다.")),
+            winnerScore


이 부분 약간 어색한데, winnerScore는 구할 수 있는데 Winner를 못찾아오는 경우가 있나요? 어차피 Turn에게서 승자 정보를 가져오는 방식이라면 묶어서 가져올 수 있는방법이 있지 않을까요?

boyekim added 17 commits April 6, 2026 21:07

feat: 궁성 기능 추가

d92271f

feat: 궁 포획 시 종료 로직 추가

f7ef7c6

feat: 말 선택 취소 기능 추가

a94521d

refactor: 사용하지 않는 메서드 제거

87ef1ba

feat: 점수 계산 기능 추가

8165335

test: 점수 계산 테스트 추가

65be702

build: build.gradle 의존성 추가

0bf8a46

feat: 스키마 파일 추가

e972bc8

feat: 데이터 베이스 생성 로직 추가

679ccd1

feat: 레포지토리 코드 추가

973782d

feat: DB 저장 기능 추가

1d1b1f9

test: 궁성 테스트 추가

348f8c7

refactor: AppConfig 추가로 객체 생성 로직 분리

ef1561c

test: 궁성 테스트 추가

e2a542e

test: 게임 종료 테스트 추가

58c3ec8

refactor: Piece의 Board 의존성 제거

2416820

test: 테스트 수정

c2fc2fc

echo724 self-requested a review April 8, 2026 12:59

echo724 requested changes Apr 10, 2026

View reviewed changes

boyekim added 3 commits April 12, 2026 17:41

refactor: cancel 처리 책임 InputView로 이관

85ba374

refactor: 게임 실행과 저장 간극 감소

e95429f

test: Service 테스트 추가

d3c641a

Conversation

boyekim commented Apr 8, 2026

체크 리스트

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

DB 적용

이전 코멘트 적용

Uh oh!

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

boyekim Apr 12, 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!

boyekim Apr 12, 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!

boyekim Apr 12, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

boyekim Apr 12, 2026 •

edited

Loading

boyekim Apr 12, 2026 •

edited

Loading

boyekim Apr 12, 2026 •

edited

Loading