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

armd482 · 2026-04-03T04:58:43Z

체크 리스트

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

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

안녕하세요 초코칩.
이전 사이클에서 남겨주신 리뷰들 반영해서 pr올립니다!
db관련 어디서 데이터를 wrapping할 지에 대한 기준을 잘 몰라서, 일단 컨트롤러에서 dto를 생성하는 식으로 구현했습니다.
계속 혼자 고민하는 것보다 일단 pr올리고 코멘트 받은 걸로 고민하는 게 더 좋을 것 같아서 부족하지만 pr올립니다!

계층과 역할
데이터베이스 사용이 처음이라 계층에 대한 고민을 많이 한 것 같습니다.
dto라는 게 사용하는 게 맞는지, 사용한다면 어디서 생성하여 어디까지 전달해야 하는지, 이 부분이 잘 모르겠습니다.
runner -> 서비스 -> dao에서, 서비스라는 계층에서 dto를 받는지, vo를 받아야 하는 지, dao에는 원시값으로 전달해야 하는지, 아님 dto로 전달해야 하는지
반대로 dao -> service ->runner에서 서비스는 값 자체를 받아서 vo로 전달할 지, dto로 생성해야 할지 기준을 모르겠습니다.
테스트 단위
서비스, TransactionManger,SQL manager, dao에 대해서 어디까지 테스트 코드를 작성하는 게 맞을 지 모르겠습니다.
테스트를 작성한다면, 어느 부분을 테스트 해야할 지에 대한 고민이 있습니다

Chocochip101

안녕하세요, 우디! ✋🏻 리뷰어 초코칩입니다 🍪
사이클 1에 이어 사이클 2도 잘 구현해주셨네요.
리뷰 남겼으니 확인해보시고, 재요청주세요 😄

Chocochip101 · 2026-04-03T06:21:23Z

src/main/java/janggi/Runner.java

+
+        Side winnerSide = game.getWinnerSide();
+
+        if(winnerSide == null) {


null이 될 수 있나요? Side.Empty를 반환하지 않나요?

null로 다루는게 맞나요?

해당 로직을 game에서 다룰 수 없나요? (game.hasXXX()?같은)

해당 로직은 테스트로 커버 되었나요?

Chocochip101 · 2026-04-03T06:21:54Z

README.md

 - 시작 시 초/한 진영의 기본 기물 배치와 마상 배치를 반영해 초기화한다.
 - 장기판은 현재 위치의 기물 조회, 빈 칸 여부, 아군/적군 여부를 판단한다.
 - 장기판은 기물 이동 요청을 받아 실제 상태를 변경한다.
+- 장기판은 상대 기물을 잡았을 때 해당 진영의 점수를 실시간으로 차감한다.


기능목록 💯

Chocochip101 · 2026-04-03T06:30:51Z

src/main/java/janggi/domain/turn/HanTurn.java

+            return new TurnState(new FinishTurn(board, Side.HAN, turn), turnAttribute, pieceAttribute);
+        }
+
+        TurnAttribute turnAttribute = new TurnAttribute(Side.HAN, turn + 1);


HAN 다음에 CHO를 반환해야하지 않나요?

Suggested change

TurnAttribute turnAttribute = new TurnAttribute(Side.HAN, turn + 1);

TurnAttribute turnAttribute = new TurnAttribute(Side.CHO, turn + 1);

테스트로 커버가 되었을까요?

Chocochip101 · 2026-04-03T06:37:02Z

src/main/java/janggi/domain/turn/HanTurn.java

+        PieceAttribute pieceAttribute = board.move(start, end, Side.HAN);
+
+        if(turn == MAX_TURN) {
+            TurnAttribute turnAttribute = new TurnAttribute(Side.EMPTY, turn + 1);


ChoTurn과 넘기는 인자가 다른데, 어떤게 맞나요?

Chocochip101 · 2026-04-03T06:38:18Z

src/main/java/janggi/Runner.java

+        SideScore score = game.getCurrentSideScore();
+        if(score.cho() > score.han()) {


getter로 꺼내와서 비교해야하나요?

Chocochip101 · 2026-04-03T07:25:16Z

src/main/java/janggi/Runner.java

+        OutputView.printLine();
+
+        MoveResult moveResult = game.move(startPosition, endPosition);
+        TurnDto turnDto = new TurnDto(moveResult.turnAttribute().side().getName(), moveResult.turnAttribute().turn());


디미터 법칙 위반이네요. 어떻게 개선해볼 수 있을까요?

프로그래밍 요구사항: 한 줄에 점을 하나만 찍는다.

Chocochip101 · 2026-04-03T07:28:27Z

src/main/java/janggi/Runner.java

-        game = new Game(choArrangement, hanArrangement);
-    }
+        List<PieceDto> pieceDtos = game.init(choArrangement, hanArrangement).stream()
+                .map(pieceInitInfo -> new PieceDto(pieceInitInfo.position().getX(), pieceInitInfo.position().getY(), pieceInitInfo.pieceType().getName(), pieceInitInfo.side().getName()))


PieceDto에서 정적 메서드를 만들어도 좋을 것 같은데, 어떻게 생각하시나요?

Suggested change

.map(pieceInitInfo -> new PieceDto(pieceInitInfo.position().getX(), pieceInitInfo.position().getY(), pieceInitInfo.pieceType().getName(), pieceInitInfo.side().getName()))

.map(PieceDto::from)

Chocochip101 · 2026-04-03T07:29:36Z

src/main/java/janggi/Runner.java

+    }
+
+    private Optional<Integer> getPreviousBoard() {
+        GameInfos gameInfos = new GameInfos(janggiService.getEntireGame().stream()


여기도 비슷하게 적용할 수 있겠네요.

https://github.com/woowacourse/java-janggi/pull/274/changes#r3031804883

Chocochip101 · 2026-04-03T07:30:04Z

src/main/java/janggi/Runner.java

+
+        GameInfo selectedGame = askUntilValid(() -> getSelectedGame(gameInfos));
+
+        List<PieceInitInfo> pieceInitInfos = janggiService.getPieceInitInfos(selectedGame.id()).stream().map(pieceDto -> new PieceInitInfo(new Position(pieceDto.x(), pieceDto.y()), Side.from(pieceDto.side()),


여기두요?

https://github.com/woowacourse/java-janggi/pull/274/changes#r3031804883

Chocochip101 · 2026-04-03T08:13:54Z

README.md

@@ -23,20 +25,27 @@
 - 좌표는 입력값을 검증하여 생성한다.


계층과 역할

당연히 어떠한 정답은 있는게 아니기에 저의 기준을 얘기할게요. 저는 필요에 의해 쓰는 것 같아요. 아무래도 view의 변경이 많다고 생각하기 때문에 controller에서 view로 넘어갈 때는 DTO를 사용하는데, DAO와 서비스 사이에는 주로 쓰지 않아요. 생각보다 DAO의 변경은 신중을 가하기 때문에 서비스의 변경으로 인해 DAO가 변하는 경우가 view-controller에 비해 적기 때문이에요. 넘겨야 되는 인자가 많으면 필요하다고 느껴서 쓰기도 하고요.

추가로, 레이어 간 의존 방향을 지키기 위해 DTO를 쓰는 경우도 있어요. 도메인 객체를 DAO까지 그대로 전달하면 DAO가 도메인 모델 구조를 알아야 하고, DB 스키마가 바뀌었을 때 도메인 객체까지 영향이 전파될 수 있거든요. DTO가 두 레이어 사이의 경계를 명시적으로 만들어서 서로 독립적으로 변경할 수 있게 해줄 수도 있어요.

테스트 단위

어디까지 테스트 코드를 작성하는 게 맞을 지 모르겠습니다: 작성한 코드의 모든 부분을 테스트할 수 없나요? 불가능하거나 필요없는 부분이 있을까요?

어느 부분을 테스트 해야할 지에 대한 고민이 있습니다: 사용자의 Use case부터 접근해보는건 어떤가요? 애플리케이션 시작 시, 게임을 생성하면 저장이 되니까 '게임이 저장된다'부터 시작해볼 수 있겠네요.

전체적으로 테스트가 부족한 것 같습니다! 모든 케이스에 대해 테스트가 커버되었는지 확인 부탁드릴게요!

armd482 added 29 commits March 31, 2026 23:23

refactor: Game객체 null에 대한 방어적 코드 변경(생성자로 변경 및 주입)

ea8a875

refactor: 미사용 상수 제거

0392115

rename: Turn 추상 클래스 및 자식 turn 클래명 변경

bce6ed8

refactor: PieceManifest 이름 변경

bc7846a

feat: dx,dy에 대한 정적 팩토리 메서드 구현

0bc4f56

refactor: Linear Piece 경로 계산 방식 Movement로 변경

20564a3

refactor: 공통 로직 상위 단으로 이동

0314ed8

feat: 궁성 기능 추가

89b3f7a

feat: 점수 계산 로직 추가

5bebc61

test: board, materialScore 테스트 추가

8e63e17

feat: 종료 기능 추가

23514d6

docs: readme 업데이트

200784c

feat: 점수 높은 진영에 대한 승리 출력 구현

b99a5f2

test: 직선 및 궁성 대각선 대한 방향, 길이 테스트 코드 추가

aa8337e

feat: max turn 기능 구현

b0b67d9

refactor: 직선 판별 Linear로 이동

54d5e64

chore: 미사용 import 제거

e38c5ec

feat: 게임 방 가져오는 dao 구현

fc60b03

feat: 게임 방 출력 및 선택 입력 추가

cb946b8

feat: 게임 기물 데이터 저장, 가져오기 기능 구현

e47ab05

feat: gameRoom turn, side 연결 및 move 시 데이터 업데이트 추가

e56bb2e

feat: main문 마지막에 connection close 추가

b8c0b00

fix: 진영 저장 오류 수정

346c19b

refactor: 트랜잭션 분리

ef592d6

refactor: runner 분리 및 잘못 입력 시 반복 실행 추가

00ed4b0

refactor: 보드 가져오기 index 예외 처리 방식 변경

2ae0b21

docs: readme 기능 업데이트

4fdd10b

refactor: 빈 객체 제외하고 전달

b42a62c

refactor: 시작 턴 0으로 변경

95941a1

Chocochip101 requested changes Apr 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 우디 미션 제출합니다.#274

[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 우디 미션 제출합니다.#274
armd482 wants to merge 29 commits intowoowacourse:armd482from
armd482:step2

armd482 commented Apr 3, 2026 •

edited

Loading

Uh oh!

Chocochip101 left a comment

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Chocochip101 Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		Side winnerSide = game.getWinnerSide();

		if(winnerSide == null) {

	TurnAttribute turnAttribute = new TurnAttribute(Side.HAN, turn + 1);
	TurnAttribute turnAttribute = new TurnAttribute(Side.CHO, turn + 1);

		SideScore score = game.getCurrentSideScore();
		if(score.cho() > score.han()) {

	.map(pieceInitInfo -> new PieceDto(pieceInitInfo.position().getX(), pieceInitInfo.position().getY(), pieceInitInfo.pieceType().getName(), pieceInitInfo.side().getName()))
	.map(PieceDto::from)


		GameInfo selectedGame = askUntilValid(() -> getSelectedGame(gameInfos));

		List<PieceInitInfo> pieceInitInfos = janggiService.getPieceInitInfos(selectedGame.id()).stream().map(pieceDto -> new PieceInitInfo(new Position(pieceDto.x(), pieceDto.y()), Side.from(pieceDto.side()),

		@@ -23,20 +25,27 @@
		- 좌표는 입력값을 검증하여 생성한다.

Conversation

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

Uh oh!

체크 리스트

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

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

armd482 commented Apr 3, 2026 •

edited

Loading