[3, 4단계 - 체스] 커찬(이충안) 미션 제출합니다. #21

leegwichan · 2024-04-01T11:32:07Z

프로그램 실행 방법

database 폴더로 이동한다.
docker-compose.yml를 통하여 docker를 실행한다.
init.sql의 명령어를 docker 내부에서 차례로 실행한다.
IDE를 통해 ChessApplication를 실행한다.

-> Docker(Mysql)가 실행되지 않거나 초기 테이블 설정이 되어있지 않다면, 프로그램이 정상적으로 작동하지 않습니다.

구현 시 고려한 점

요구 사항 만족

"돌아가는 쓰레기를 만들자"와 "주어진 프로그래밍 요구 사항을 지킬 수 있는 만큼 지키자"를 목표로 프로그램을 만들었습니다. 메서드 네이밍이 직관적이지 않거나 구조 측면이 과하게 복잡할 수 있습니다. 보이시는 만큼 리뷰 부탁드립니다. ( _ _ )

자동 저장 기능

말이 한 번 움직일 때마다 저장합니다. 입력 받는 중에 예기치 못하게 중단되거나 오류가 발생한다 하더라도 이전의 상황을 그대로 사용할 수 있습니다.

고려하지 못한 점

트랜젝션

따로 트랜젝션을 사용하지 않아서, 동작 중에 오류가 발생하면 DB가 정상적으로 저장되지 않을 수 있습니다.

reddevilmidzy

레벨 1 동안 수고 많았어!!
방학동안 푹쉬고 레벨2도 같이 힘내자💪

reddevilmidzy · 2024-04-02T12:49:13Z

database/init.sql

+    turn VARCHAR(10) CHECK (turn IN ('BLACK', 'WHITE')) not null,
+    PRIMARY KEY (turn)
+);
+
+CREATE TABLE pieces (
+    board_file VARCHAR(2) CHECK (board_file IN ('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H')) not null,
+    board_rank VARCHAR(2) CHECK (board_rank IN ('1', '2', '3', '4', '5', '6', '7', '8')) not null,
+    type VARCHAR(10) CHECK (type IN ('KING', 'QUEEN', 'ROOK', 'BISHOP', 'KNIGHT', 'PAWN', 'EMPTY')) not null,
+    team VARCHAR(10) CHECK (team IN ('BLACK', 'WHITE', 'EMPTY')) not null,
+    PRIMARY KEY (board_rank, board_file)


테이블 명이나 컬럼명은 소문자로 하고 예약어는 대문자 컨벤션을 유지한거 같은데 not null은 소문자인 이유 궁금하군요!👀

어 그러게요 ㅎ;
대문자로 통일하는게 좋겠네요 ㅎㅎ

reddevilmidzy · 2024-04-02T12:50:10Z

src/main/java/chess/ChessApplication.java

+    private static final InputView INPUT_VIEW = new InputView();
+    private static final OutputView OUTPUT_VIEW = new OutputView();
+
    public static void main(String[] args) {


view 주입을 방식을 변경하게 된 이유가 궁금해요

메서드 줄 수 줄일려고 했는데, 통일성이 없어서 수정했습니다.

public static void main(String[] args) { ConnectionManager connectionManager = new ConnectionManager(); ChessService chessService = createChessService(connectionManager); InputView inputView = new InputView(); OutputView outputView = new OutputView(); ChessController chessController = new ChessController(chessService, inputView, outputView); try { chessController.run(); } finally { connectionManager.closeConnection(); } } private static ChessService createChessService(ConnectionManager connectionManager) { ChessGameRepository chessGameRepository = new MysqlChessGameRepository(connectionManager); PieceRepository pieceRepository = new MysqlPieceRepository(connectionManager); ChessDao chessDao = new ChessDao(chessGameRepository, pieceRepository); return new ChessService(chessDao); }

reddevilmidzy · 2024-04-02T12:52:02Z

src/main/java/chess/ChessGame.java

-        if (command.isEnd()) {
-            System.exit(0);


System.exit(0) 를 사용하지 않은 건 좋군요!👍

reddevilmidzy · 2024-04-02T12:54:30Z

src/main/java/chess/ChessService.java

+        if (board != null) {
+            throw new IllegalStateException("보드가 이미 초기화 되었습니다.");
+        }


board != null 대신 제일 초기에 빈 board를 넣어준다음 board.블라블라 메서드를 만들어 객체에게 물어 보는 방법도 좋을 거 같아요!

그냥 아마도 Board를 그냥 처음부터 만들어버리는 것도 방법이 될 것 같아요.
정말 init() 메서드를 외부에서 실행 해주어야 하나 고민이네요;;

그렇게 되면 빈 보드의 역할이 너무 모호해질 것 같아요. 다른 메서드는 다 되지 않고, 특정 메서드만 가능한 보드가 될 것 같아서 걱정이 되네요.

reddevilmidzy · 2024-04-02T12:55:18Z

src/main/java/chess/dao/ConnectionManager.java

+    private static final String OPTION = "?useSSL=false&allowPublicKeyRetrieval=true&serverTimezone=UTC";
+
+    private static final String URL = "jdbc:mysql://" + SERVER + "/" + DATABASE + OPTION;


사소하긴 하지만 여기의 공백은 필요한 공백이었을까요?

사용해도 되고, 사용하지 않아도 된다고 생각해요.
다른 변수를 구성하기 위한 변수, 실질적으로 메서드에서 사용되는 변수를 나누어 놓은 것도 가시성이 올라가지 않을까요?

reddevilmidzy · 2024-04-02T12:58:19Z

src/main/java/chess/domain/Point.java

+
+import java.util.Objects;
+
+public class Point {


Point 를 record가 아닌 class로 구현한 이유가 궁금해요

도메인 객체니까? 타칸이 테코톡으로 말했던 Record를 사용하는 때를 적용해본 것 같아.
기능 상으로는 문제는 없을 것 같은데, 조금 더 도메인 객체라는 의도를 주고 싶었던 것 같아.
(물론 도메인 패키지 안에 있으니까 상관 없겠지만)

reddevilmidzy · 2024-04-02T13:00:07Z

src/main/java/chess/domain/piece/Pawn.java

@@ -16,6 +17,9 @@ public final class Pawn extends Piece {
            new BlackPawnFirstMovement(), new BlackPawnDefaultMovement(), new BlackPawnDiagonalMovement());
    private static final List<MovementRule> WHITE_MOVEMENT_RULES = List.of(
            new WhitePawnFirstMovement(), new WhitePawnDefaultMovement(), new WhitePawnDiagonalMovement());
+    private static final Point PIECE_BASIC_POINT = new Point(1.0);
+    private static final Point PIECE_OVERLAPPED_POINT = new Point(0.5); // TODO 변수명 변경


todo 주석이 남아있군요👀

??? 이거 왜 있음??? ㄳㄳ

reddevilmidzy · 2024-04-02T13:01:59Z

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

+    public final boolean isKing() {
+        return this.getClass() == King.class;
+    }
+
+    public final boolean isPawn() {
+        return this.getClass() == Pawn.class;
+    }


Piece는 추상 클래스 즉 부모클래스인데 부모클래스가 자식 클래스를 알고 있어도 괜찮을까요?
괜찮은 의존 방향인가요

요런 관점에서 보면 sealed 클래스가 왜 만들어졌는지도 의문이군요😅

만약에 isPawn()과 같은 메서드를 하위 클래스 전부에서 구현했다면, sealed를 쓰지 않았을 것 같아.
구체 클래스가 무엇인지에 따라 정해지다보니, sealed를 사용했어.

어짜피 실제 인스턴스 클래스에 따라 구분되는 메서드이고, 추가적으로 계속적인 확장이 필수불가결하지 않다면 위와 같은 방법도 괜찮다고 생각해. 하지만 위와 같은 방법은 항상 쓸 수 있는 것은 아니고, 특정 상황에서만 쓸 수 있다고 생각해.

reddevilmidzy · 2024-04-02T13:38:54Z

src/main/java/chess/dto/ProgressStatus.java

+    END_GAME(false),
+    ;
+
+    private final boolean isContinue;


요 boolean 없이 아래에 isContinue 메서드에서

public boolean isContinue() { return this == PROGRESS; }

이렇게 구현하는건 어떤가요?

reddevilmidzy · 2024-04-03T01:43:04Z

src/main/java/chess/dao/MysqlPieceRepository.java

+import java.util.List;
+import java.util.Map;
+
+public class MysqlPieceRepository implements PieceRepository {


나였다면! MySql과 Mysql을 고민하다가 MySql을 사용했을 거 같아!

일반적으로 JSON을 클래스 이름으로 쓸 때, JSONParser보다는 JsonParser가 더 가독성이 좋다고 하더라고.
그래서 하나의 프로그램 응용프로그램인 MySql은 MysqlPieceRepository라고 쓰는게 가독성이 더 좋다고 생각했어

leegwichan

리비! 일찍 리뷰 와있어서 먼저 답변 달아봤어!
내 생각을 적어봤는데, 궁금한 점 있다면 코멘트 달거나 직접 와서 물어봐 줘!

leegwichan · 2024-04-03T05:35:05Z

database/init.sql

+    turn VARCHAR(10) CHECK (turn IN ('BLACK', 'WHITE')) not null,
+    PRIMARY KEY (turn)
+);
+
+CREATE TABLE pieces (
+    board_file VARCHAR(2) CHECK (board_file IN ('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H')) not null,
+    board_rank VARCHAR(2) CHECK (board_rank IN ('1', '2', '3', '4', '5', '6', '7', '8')) not null,
+    type VARCHAR(10) CHECK (type IN ('KING', 'QUEEN', 'ROOK', 'BISHOP', 'KNIGHT', 'PAWN', 'EMPTY')) not null,
+    team VARCHAR(10) CHECK (team IN ('BLACK', 'WHITE', 'EMPTY')) not null,
+    PRIMARY KEY (board_rank, board_file)


어 그러게요 ㅎ;
대문자로 통일하는게 좋겠네요 ㅎㅎ

leegwichan · 2024-04-03T05:37:17Z

src/main/java/chess/ChessApplication.java

+    private static final InputView INPUT_VIEW = new InputView();
+    private static final OutputView OUTPUT_VIEW = new OutputView();
+
    public static void main(String[] args) {


메서드 줄 수 줄일려고 했는데, 통일성이 없어서 수정했습니다.

public static void main(String[] args) { ConnectionManager connectionManager = new ConnectionManager(); ChessService chessService = createChessService(connectionManager); InputView inputView = new InputView(); OutputView outputView = new OutputView(); ChessController chessController = new ChessController(chessService, inputView, outputView); try { chessController.run(); } finally { connectionManager.closeConnection(); } } private static ChessService createChessService(ConnectionManager connectionManager) { ChessGameRepository chessGameRepository = new MysqlChessGameRepository(connectionManager); PieceRepository pieceRepository = new MysqlPieceRepository(connectionManager); ChessDao chessDao = new ChessDao(chessGameRepository, pieceRepository); return new ChessService(chessDao); }

leegwichan · 2024-04-03T05:40:40Z

src/main/java/chess/ChessService.java

+        if (board != null) {
+            throw new IllegalStateException("보드가 이미 초기화 되었습니다.");
+        }


그냥 아마도 Board를 그냥 처음부터 만들어버리는 것도 방법이 될 것 같아요.
정말 init() 메서드를 외부에서 실행 해주어야 하나 고민이네요;;

그렇게 되면 빈 보드의 역할이 너무 모호해질 것 같아요. 다른 메서드는 다 되지 않고, 특정 메서드만 가능한 보드가 될 것 같아서 걱정이 되네요.

leegwichan · 2024-04-03T05:42:11Z

src/main/java/chess/dao/ConnectionManager.java

+    private static final String OPTION = "?useSSL=false&allowPublicKeyRetrieval=true&serverTimezone=UTC";
+
+    private static final String URL = "jdbc:mysql://" + SERVER + "/" + DATABASE + OPTION;


사용해도 되고, 사용하지 않아도 된다고 생각해요.
다른 변수를 구성하기 위한 변수, 실질적으로 메서드에서 사용되는 변수를 나누어 놓은 것도 가시성이 올라가지 않을까요?

leegwichan · 2024-04-03T05:45:03Z

src/main/java/chess/dao/MysqlPieceRepository.java

+import java.util.List;
+import java.util.Map;
+
+public class MysqlPieceRepository implements PieceRepository {


일반적으로 JSON을 클래스 이름으로 쓸 때, JSONParser보다는 JsonParser가 더 가독성이 좋다고 하더라고.
그래서 하나의 프로그램 응용프로그램인 MySql은 MysqlPieceRepository라고 쓰는게 가독성이 더 좋다고 생각했어

leegwichan · 2024-04-03T05:47:59Z

src/main/java/chess/domain/Point.java

+
+import java.util.Objects;
+
+public class Point {


도메인 객체니까? 타칸이 테코톡으로 말했던 Record를 사용하는 때를 적용해본 것 같아.
기능 상으로는 문제는 없을 것 같은데, 조금 더 도메인 객체라는 의도를 주고 싶었던 것 같아.
(물론 도메인 패키지 안에 있으니까 상관 없겠지만)

leegwichan · 2024-04-03T05:48:37Z

src/main/java/chess/domain/piece/Pawn.java

@@ -16,6 +17,9 @@ public final class Pawn extends Piece {
            new BlackPawnFirstMovement(), new BlackPawnDefaultMovement(), new BlackPawnDiagonalMovement());
    private static final List<MovementRule> WHITE_MOVEMENT_RULES = List.of(
            new WhitePawnFirstMovement(), new WhitePawnDefaultMovement(), new WhitePawnDiagonalMovement());
+    private static final Point PIECE_BASIC_POINT = new Point(1.0);
+    private static final Point PIECE_OVERLAPPED_POINT = new Point(0.5); // TODO 변수명 변경


??? 이거 왜 있음??? ㄳㄳ

leegwichan · 2024-04-03T05:53:11Z

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

+    public final boolean isKing() {
+        return this.getClass() == King.class;
+    }
+
+    public final boolean isPawn() {
+        return this.getClass() == Pawn.class;
+    }


만약에 isPawn()과 같은 메서드를 하위 클래스 전부에서 구현했다면, sealed를 쓰지 않았을 것 같아.
구체 클래스가 무엇인지에 따라 정해지다보니, sealed를 사용했어.

어짜피 실제 인스턴스 클래스에 따라 구분되는 메서드이고, 추가적으로 계속적인 확장이 필수불가결하지 않다면 위와 같은 방법도 괜찮다고 생각해. 하지만 위와 같은 방법은 항상 쓸 수 있는 것은 아니고, 특정 상황에서만 쓸 수 있다고 생각해.

3Juhwan

커찬, 안녕하세요! 망쵸입니다~
코드 잘 읽었어요. 항상 드는 생각이지만 커찬의 코드는 깔끔하고 담백하네요.
3, 4단계에 추가된 코드를 위주로 리뷰를 진행했어요. 평소와 같게 감탄하며 코멘트 달 부분이 많지 않았네요.
레벨1 고생하셨어요!! 레벨2도 파이팅 하죠 ✊✊

3Juhwan · 2024-04-03T05:28:40Z

src/main/java/chess/dao/ChessDao.java

+public class ChessDao {
+
+    private final ChessGameRepository chessGameRepository;
+    private final PieceRepository pieceRepository;


dao와 repository의 네이밍의 의도가 궁금해요 💭
repository는 DB에 직접 접근하는 주체, dao는 비즈니스 로직을 품고 있다고 파악되네요 😀

일단 다른 크루들에게 물어봤을 때는 Repository, Dao는 크게 구분없이 통용되어 사용되고 있다고 이해했어. 그런데도 구분해서 사용한 이유는 Spring에서 Repository를 사용할 때 보통 한 테이블을 기준으로 사용하는 점을 차용했어. 그리고 Dao는 하나의 트랙젝션의 단위로 묶고 싶어서 이를 분리했어. (지금은 ChessGame에서 필드를 2개만 사용하기 위해서 ChessGameRepository, PieceRepository를 묶어주었다고 생각해도 무방할 것 같아.)

이렇게 DAO를 구성하게 되면 비즈니스 로직을 일부 품고 있게 되는 것은 어쩔 수 없다고 생각해. 지금은 저장하는 로직과 도메인 로직이 완전히 분리되어 관리해주어야 하다 보니... 그래도 "DB에 정상적으로 저장하는 작업 단위"로 묶어주었다는 것에 의의를 둘 수 있지 않을까?

3Juhwan · 2024-04-03T05:30:34Z

src/main/java/chess/dao/ConnectionManager.java

+
+    private static final String SERVER = "localhost:13306";
+    private static final String DATABASE = "chess";
+    private static final String OPTION = "?useSSL=false&allowPublicKeyRetrieval=true&serverTimezone=UTC";


DB 서버와 시간 형식을 맞춰보면 좋을 것 같아요! docker 설정 파일에는 ASIA/SEOUL로 되어 있더군요.

이걸 캐치하다니 ㅎㄷㄷ;;
Docker 설정에 대해서도 알아봐야겠네;;

3Juhwan · 2024-04-03T05:34:06Z

src/main/java/chess/dao/MysqlPieceRepository.java

+        try (PreparedStatement preparedStatement = connection.prepareStatement(query)) {
+            addBatch(pieces, preparedStatement);
+            preparedStatement.executeBatch();


배치 👍
저도 유사한 방식으로 구현했는데, 확실히 배치하고 싶더라고요!

JDBC에 대해서는 잘 모르는 편이라 '구글신'께 물어봐서 만든 코드야;; 다른 크루 리뷰를 찾아봤더니 Batch를 사용하는 편이 성능이 좋다고 하더라고.

3Juhwan · 2024-04-03T05:59:55Z

src/main/java/chess/dao/PieceEntity.java

+import chess.dto.PieceType;
+import chess.dto.TeamType;
+
+public class PieceEntity {


커찬이 작성한 entity, dto, domain의 차이가 궁금하네요!

entity : DB에 저장하기 위한 객체 -> DB로 정보를 저장할 때, DB의 정보를 불러올 때 사용하는 객체
dto : 정보 전달을 위한 객체 -> Domain의 일부 정보를 Domain 밖으로 전달하기 위한 객체
domain : 프로그램의 요구사항들을 담고 있는 객체 -> Domain은 최대한 Domain 객체끼리 의존해야 한다.

이런 식으로 정리해서 생각하고 있어. 추가적으로 domain <-> dto, domain <-> entity의 매핑 하는 방법들을 최대한 dto, entity에게 할당하는 형식으로 구성하는 편이야.

3Juhwan · 2024-04-03T06:07:31Z

src/main/java/chess/ChessGame.java

+    private ProgressStatus processTurn() {
        GameCommand command = inputView.readCommand();
        if (command.isStart()) {
            throw new IllegalArgumentException("이미 게임을 시작했습니다.");
        }
-        if (command.isEnd()) {
-            System.exit(0);
+        if (command.isMove()) {
+            return executeMove();
+        }
+        if (command.isStatus()) {
+            return executeStatus();
        }
-        executeMove(board);


if문으로 분기하니, 의도가 명확하게 보여서 좋은 것 같아요 👍
하지만 더 많은 커맨드가 추가된다면 어떨까요? 유지보수 관점에서 어떻게 생각하는지 궁금해요 💭

커멘드가 5~6개로 늘어난다면, 다른 대안을 생각해볼 것 같아.

지금과 같이 생각한 이유는 '특정 커멘드에 따라 행동하는 것은 하나의 파일로 같이 있어야 하지 않을까?'라는 생각이 있었어. 그런데 커멘드가 5~6개로 늘어난다면 지금과 같이 한 파일로 보는 것이 더 힘들것 같네...

3Juhwan · 2024-04-03T06:09:07Z

src/main/java/chess/dao/MysqlChessGameRepository.java

+    private static final Map<TurnType, String> TURN_TO_STING = Map.of(
+            TurnType.BLACK, "BLACK", TurnType.WHITE, "WHITE");
+    private static final Map<String, TurnType> STRING_TO_TURN = Map.of(
+            "BLACK", TurnType.BLACK, "WHITE", TurnType.WHITE);


이런 부분을 보면, 커찬 코드는 담백하다는 생각이 들어요.
저는 enum 같은 클래스로 분리할 생각을 하는데 이게 더 명확해 보이네요 👍

많은 사람들이 이 부분을 Enum으로 분리해서 관리하는데, 그것에 대해 의문점이 들었어.
DB에 어떤 텍스트로 저장되는지가 최대한 Repository에 붙어 있어야 하지 않을까 생각했어. 그래서 DB에 저장되는 텍스트가 변경되었을 때, 쉽게 변화에 대응할 수 있으니까.

3Juhwan · 2024-04-03T06:10:22Z

src/main/java/chess/dao/MysqlPieceRepository.java

+    private static final Map<File, String> FILE_TO_STRING = Map.of(
+            File.A, "A", File.B, "B", File.C, "C", File.D, "D",
+            File.E, "E", File.F, "F", File.G, "G", File.H, "H");
+    private static final Map<String, File> STRING_TO_FILE = Map.of(
+            "A", File.A, "B", File.B, "C", File.C, "D", File.D,
+            "E", File.E, "F", File.F, "G", File.G, "H", File.H);
+    private static final Map<Rank, String> RANK_TO_STRING = Map.of(
+            Rank.ONE, "1", Rank.TWO, "2", Rank.THREE, "3", Rank.FOUR, "4",
+            Rank.FIVE, "5", Rank.SIX, "6", Rank.SEVEN, "7", Rank.EIGHT, "8");
+    private static final Map<String, Rank> STRING_TO_RANK = Map.of(
+            "1", Rank.ONE, "2", Rank.TWO, "3", Rank.THREE, "4", Rank.FOUR,
+            "5", Rank.FIVE, "6", Rank.SIX, "7", Rank.SEVEN, "8", Rank.EIGHT);
+    private static final Map<PieceType, String> PIECE_TYPE_TO_STRING = Map.of(
+            PieceType.KING, "KING", PieceType.QUEEN, "QUEEN", PieceType.ROOK, "ROOK", PieceType.BISHOP, "BISHOP",
+            PieceType.KNIGHT, "KNIGHT", PieceType.PAWN, "PAWN", PieceType.EMPTY, "EMPTY");
+    private static final Map<String, PieceType> STRING_TO_PIECE_TYPE = Map.of(
+            "KING", PieceType.KING, "QUEEN", PieceType.QUEEN, "ROOK", PieceType.ROOK, "BISHOP", PieceType.BISHOP,
+            "KNIGHT", PieceType.KNIGHT, "PAWN", PieceType.PAWN, "EMPTY", PieceType.EMPTY);
+    private static final Map<TeamType, String> TEAM_TYPE_TO_STRING = Map.of(
+            TeamType.WHITE, "WHITE", TeamType.BLACK, "BLACK", TeamType.EMPTY, "EMPTY");
+    private static final Map<String, TeamType> STRING_TO_TEAM_TYPE = Map.of(
+            "WHITE", TeamType.WHITE, "BLACK", TeamType.BLACK, "EMPTY", TeamType.EMPTY);


이렇게 길어진다면 분리를 고민해 볼 것 같은데, 커찬은 괜찮다고 판단해서 그대로 두셨겠죠?

우선적으로는 앞의 ChessGameRepository와 같은 이유로 DB에 저장하는 텍스트를 한 파일에 몰아두었어. 근데 많이 길어지는 건 어쩔 수 없더라고;; 지금까지는 턱걸이로 허용되는 정도라고 생각하고, 이것보다 조금 만 더 많아지면, 이 텍스트를 분리할 필요가 있을 것 같아.

지금의 PieceType, TeamType 등이 OutputView에서도 같이 사용하고 있어서 Enum으로 텍스트를 옮길 수 없었어. 아마 다음에는 View에서 사용하는 Enum과 Entity에서 사용하는 Enum을 분리한다면 해당 텍스트를 옮길 수 있을 것 같아.

leegwichan

망쵸! 커찬이야~ 준 리뷰에는 최대한 코멘트를 다는 편인데, 미션하느라 너무 바빠서 방학 기간에 남기게 되었어. 혹시나 궁금한 점 있으면 추가 커멘트 남겨줘도 되!
꼼꼼히 리뷰해줘서 고마워!

leegwichan · 2024-04-09T10:49:20Z

src/main/java/chess/ChessGame.java

+    private ProgressStatus processTurn() {
        GameCommand command = inputView.readCommand();
        if (command.isStart()) {
            throw new IllegalArgumentException("이미 게임을 시작했습니다.");
        }
-        if (command.isEnd()) {
-            System.exit(0);
+        if (command.isMove()) {
+            return executeMove();
+        }
+        if (command.isStatus()) {
+            return executeStatus();
        }
-        executeMove(board);


커멘드가 5~6개로 늘어난다면, 다른 대안을 생각해볼 것 같아.

지금과 같이 생각한 이유는 '특정 커멘드에 따라 행동하는 것은 하나의 파일로 같이 있어야 하지 않을까?'라는 생각이 있었어. 그런데 커멘드가 5~6개로 늘어난다면 지금과 같이 한 파일로 보는 것이 더 힘들것 같네...

leegwichan · 2024-04-09T10:57:47Z

src/main/java/chess/dao/ChessDao.java

+public class ChessDao {
+
+    private final ChessGameRepository chessGameRepository;
+    private final PieceRepository pieceRepository;


일단 다른 크루들에게 물어봤을 때는 Repository, Dao는 크게 구분없이 통용되어 사용되고 있다고 이해했어. 그런데도 구분해서 사용한 이유는 Spring에서 Repository를 사용할 때 보통 한 테이블을 기준으로 사용하는 점을 차용했어. 그리고 Dao는 하나의 트랙젝션의 단위로 묶고 싶어서 이를 분리했어. (지금은 ChessGame에서 필드를 2개만 사용하기 위해서 ChessGameRepository, PieceRepository를 묶어주었다고 생각해도 무방할 것 같아.)

이렇게 DAO를 구성하게 되면 비즈니스 로직을 일부 품고 있게 되는 것은 어쩔 수 없다고 생각해. 지금은 저장하는 로직과 도메인 로직이 완전히 분리되어 관리해주어야 하다 보니... 그래도 "DB에 정상적으로 저장하는 작업 단위"로 묶어주었다는 것에 의의를 둘 수 있지 않을까?

leegwichan · 2024-04-09T10:58:24Z

src/main/java/chess/dao/ConnectionManager.java

+
+    private static final String SERVER = "localhost:13306";
+    private static final String DATABASE = "chess";
+    private static final String OPTION = "?useSSL=false&allowPublicKeyRetrieval=true&serverTimezone=UTC";


이걸 캐치하다니 ㅎㄷㄷ;;
Docker 설정에 대해서도 알아봐야겠네;;

leegwichan · 2024-04-09T11:00:37Z

src/main/java/chess/dao/MysqlChessGameRepository.java

+    private static final Map<TurnType, String> TURN_TO_STING = Map.of(
+            TurnType.BLACK, "BLACK", TurnType.WHITE, "WHITE");
+    private static final Map<String, TurnType> STRING_TO_TURN = Map.of(
+            "BLACK", TurnType.BLACK, "WHITE", TurnType.WHITE);


많은 사람들이 이 부분을 Enum으로 분리해서 관리하는데, 그것에 대해 의문점이 들었어.
DB에 어떤 텍스트로 저장되는지가 최대한 Repository에 붙어 있어야 하지 않을까 생각했어. 그래서 DB에 저장되는 텍스트가 변경되었을 때, 쉽게 변화에 대응할 수 있으니까.

leegwichan · 2024-04-09T11:04:57Z

src/main/java/chess/dao/MysqlPieceRepository.java

+    private static final Map<File, String> FILE_TO_STRING = Map.of(
+            File.A, "A", File.B, "B", File.C, "C", File.D, "D",
+            File.E, "E", File.F, "F", File.G, "G", File.H, "H");
+    private static final Map<String, File> STRING_TO_FILE = Map.of(
+            "A", File.A, "B", File.B, "C", File.C, "D", File.D,
+            "E", File.E, "F", File.F, "G", File.G, "H", File.H);
+    private static final Map<Rank, String> RANK_TO_STRING = Map.of(
+            Rank.ONE, "1", Rank.TWO, "2", Rank.THREE, "3", Rank.FOUR, "4",
+            Rank.FIVE, "5", Rank.SIX, "6", Rank.SEVEN, "7", Rank.EIGHT, "8");
+    private static final Map<String, Rank> STRING_TO_RANK = Map.of(
+            "1", Rank.ONE, "2", Rank.TWO, "3", Rank.THREE, "4", Rank.FOUR,
+            "5", Rank.FIVE, "6", Rank.SIX, "7", Rank.SEVEN, "8", Rank.EIGHT);
+    private static final Map<PieceType, String> PIECE_TYPE_TO_STRING = Map.of(
+            PieceType.KING, "KING", PieceType.QUEEN, "QUEEN", PieceType.ROOK, "ROOK", PieceType.BISHOP, "BISHOP",
+            PieceType.KNIGHT, "KNIGHT", PieceType.PAWN, "PAWN", PieceType.EMPTY, "EMPTY");
+    private static final Map<String, PieceType> STRING_TO_PIECE_TYPE = Map.of(
+            "KING", PieceType.KING, "QUEEN", PieceType.QUEEN, "ROOK", PieceType.ROOK, "BISHOP", PieceType.BISHOP,
+            "KNIGHT", PieceType.KNIGHT, "PAWN", PieceType.PAWN, "EMPTY", PieceType.EMPTY);
+    private static final Map<TeamType, String> TEAM_TYPE_TO_STRING = Map.of(
+            TeamType.WHITE, "WHITE", TeamType.BLACK, "BLACK", TeamType.EMPTY, "EMPTY");
+    private static final Map<String, TeamType> STRING_TO_TEAM_TYPE = Map.of(
+            "WHITE", TeamType.WHITE, "BLACK", TeamType.BLACK, "EMPTY", TeamType.EMPTY);


우선적으로는 앞의 ChessGameRepository와 같은 이유로 DB에 저장하는 텍스트를 한 파일에 몰아두었어. 근데 많이 길어지는 건 어쩔 수 없더라고;; 지금까지는 턱걸이로 허용되는 정도라고 생각하고, 이것보다 조금 만 더 많아지면, 이 텍스트를 분리할 필요가 있을 것 같아.

지금의 PieceType, TeamType 등이 OutputView에서도 같이 사용하고 있어서 Enum으로 텍스트를 옮길 수 없었어. 아마 다음에는 View에서 사용하는 Enum과 Entity에서 사용하는 Enum을 분리한다면 해당 텍스트를 옮길 수 있을 것 같아.

leegwichan · 2024-04-09T11:06:33Z

src/main/java/chess/dao/MysqlPieceRepository.java

+        try (PreparedStatement preparedStatement = connection.prepareStatement(query)) {
+            addBatch(pieces, preparedStatement);
+            preparedStatement.executeBatch();


JDBC에 대해서는 잘 모르는 편이라 '구글신'께 물어봐서 만든 코드야;; 다른 크루 리뷰를 찾아봤더니 Batch를 사용하는 편이 성능이 좋다고 하더라고.

leegwichan · 2024-04-09T11:09:58Z

src/main/java/chess/dao/PieceEntity.java

+import chess.dto.PieceType;
+import chess.dto.TeamType;
+
+public class PieceEntity {


entity : DB에 저장하기 위한 객체 -> DB로 정보를 저장할 때, DB의 정보를 불러올 때 사용하는 객체
dto : 정보 전달을 위한 객체 -> Domain의 일부 정보를 Domain 밖으로 전달하기 위한 객체
domain : 프로그램의 요구사항들을 담고 있는 객체 -> Domain은 최대한 Domain 객체끼리 의존해야 한다.

이런 식으로 정리해서 생각하고 있어. 추가적으로 domain <-> dto, domain <-> entity의 매핑 하는 방법들을 최대한 dto, entity에게 할당하는 형식으로 구성하는 편이야.

leegwichan added 30 commits March 26, 2024 11:38

refactor(Team): 수정이 용이하도록 메서드 재사용

91a3e25

refactor(PawnMovement) : 매직 넘버에 의미 부여 및 가시성 개선

607649f

feat(Piece): 왕 여부 판단

8ac528f

feat(Board): 해당 게임의 계속 진행할 수 있는지 판단

9abcbf8

feat(ChessGame): 왕이 잡히면 게임이 종료됨

edd54fa

feat(Point): 기물 점수 더하기

e05d029

feat(Piece): 자신의 기물 점수 판단

3625575

refactor(Point): 패키지 이동

eff640d

test(Piece): 각 기물별 점수 테스트 추가

e20e78c

feat(Board): 팀별 기물 점수 계산

2057f00

feat(ChessGame): 현재 팀별 말 점수 출력 기능

e7f9c78

refactor: 메서드 길이를 줄이기 위해 리팩터링 실시

23716a6

test(BoardTest): Optional assert 구문 수정

cbc4b37

docs: 4단계 추가 요구사항 맟 db 설정 정리

a0db668

refactor(OutputView): 가독성을 위해 메서드 분리

2d395f8

feat(Type): 데이터베이스 관리의 편의성을 위해 null 객체 추가

e955a9e

feat(ChessDao): 데이터베이스 저장을 위한 Dao 계층 생성

ba2012b

feat(TurnType) : TeamType괴의 구분을 위해 구현

710bf81

test(BoardTest): entry 순서에 영향을 받지 않도록 수정

3cf4dc0

feat(ChessGameRepository): 체크 게임의 턴 상황 CRUD

8c7e15b

feat(MysqlPieceRepository): 기물 상황 CRUD

8fd21fe

feat(BoardService): 보드 초기화 구현

56f5451

feat(ChessService) : 서비스 레이어 구성

4e47860

feat(ChessGame): ChessService와 연결되도록 구현

e5e006c

refactor(OutputView): printf 기능을 사용하도록 리팩터링

da3bbf1

feat(ChessGame): 시작 시에 현재 누구의 차례인지 안내하는 문구 출력

db1b6c3

fix(OutputView): 추가 기능 출력

f7cf3ff

refactor (ChessService) : null 객체를 이용하여 출력하도록 수정

e50ea69

refactor(ChessService): Optional의 편의 메서드를 이용하여 if문 제거

2df6c31

style(ChessService): 현재 턴을 찾는다는 내용을 강조하기 위해 메서드 이름 변경

bcf5d26

leegwichan added 8 commits April 1, 2024 10:07

refactor(ChessService): 가독성을 위해 PieceEntity의 메서드 구현

2317cc3

refactor(PawnMovement): 폰 움직임 추상화

4564d64

refactor(MysqlPieceRepository): 인덴트 조건을 지키기 위해 메서드 분리

7d6b083

style(sql): 테이블 이름 복수형으로 변경

bffe03f

refactor(Repository): 숫자가 아닌 필드의 이름으로 조회하도록 변경

c3bbcf1

refactor(Piece) : getter로 Team을 제공하기 때문에, isBlack() 메서드 제거

b241845

docs(README): 실행 방법 서술

00e8d61

style(Piece): 조건을 명확하게 하기 위해 변수명 변경

a2f0d0c

reddevilmidzy reviewed Apr 3, 2024

View reviewed changes

leegwichan commented Apr 3, 2024

View reviewed changes

3Juhwan reviewed Apr 3, 2024

View reviewed changes

leegwichan commented Apr 9, 2024

View reviewed changes

		private static final String OPTION = "?useSSL=false&allowPublicKeyRetrieval=true&serverTimezone=UTC";

		private static final String URL = "jdbc:mysql://" + SERVER + "/" + DATABASE + OPTION;

[3, 4단계 - 체스] 커찬(이충안) 미션 제출합니다. #21

Are you sure you want to change the base?

[3, 4단계 - 체스] 커찬(이충안) 미션 제출합니다. #21

Conversation

leegwichan commented Apr 1, 2024

프로그램 실행 방법

구현 시 고려한 점

요구 사항 만족

자동 저장 기능

고려하지 못한 점

트랜젝션

reddevilmidzy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leegwichan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3Juhwan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leegwichan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment