[Lv1] 장기 미션 회고

장기 미션 회고

장기 도메인 설명

기물 종류

팀

• 한나라, 초나라가 존재한다.

승리 방법

• 먼저 왕을 잡으면 승리한다.
• 왕을 잡지 못하는 경우 점수로 판단한다.

 

리팩토링 단계

추상 클래스 ⮕ 합성 ⮕ enum

 

1. 추상 클래스

• Piece : 움직이는 방식 정의

 

추상 클래스를 이용해 구현한 이유

기물이 자신이 속한 팀 정보와 기물 종류를 알아야 하기 때문에 모든 기물에 대해 공통 필드가 생기게 되었다.따라서 중복을 제거하기 위해 추상 클래스로 상속했다.

public abstract class Piece {

    private final PieceType pieceType;
    private final Team team;

    public Piece(PieceType pieceType, Team team) {
        this.pieceType = pieceType;
        this.team = team;
    }

    public Path makePath(final Position currentPosition, final Position arrivalPosition,
                         final Map<Position, Piece> pieces) {

        int differenceForY = arrivalPosition.calculateDifferenceForY(currentPosition);
        int differenceForX = arrivalPosition.calculateDifferenceForX(currentPosition);

        validateMove(differenceForY, differenceForX);

        final List<Position> positions = new ArrayList<>();
        int currentY = currentPosition.getY();
        int currentX = currentPosition.getX();
        currentY = moveY(arrivalPosition, differenceForY, differenceForX, currentY, positions, currentX);
        moveX(arrivalPosition, differenceForY, differenceForX, currentX, positions, currentY);

        Path path = new Path(positions);
        validatePath(pieces, path);
        return path;
    }

    protected int calculateUnit(int difference) {
        if (difference == 0) {
            return difference;
        }
        return difference / Math.abs(difference);
    }

    protected boolean hasPieceInMiddle(final Path path, final Map<Position, Piece> pieces) {
        List<Position> positions = new ArrayList<>(path.getPositions());
        positions.removeLast();
        return positions.stream()
                .anyMatch(pieces::containsKey);
    }

    protected abstract void validateMove(int differenceForY, int differenceForX);

    protected abstract int moveY(Position arrivalPosition, int differenceForY, final int differenceForX, int currentY,
                                 List<Position> positions, int currentX);

    protected abstract int moveX(Position arrivalPosition, final int differenceForY, int differenceForX, int currentX,
                                 List<Position> positions, int currentY);

    protected abstract void validatePath(final Map<Position, Piece> pieces, final Path path);
}

public class Chariot extends Piece {

    public Chariot(Team team) {
        super(PieceType.CHARIOT, team);
    }

    @Override
    protected void validatePath(final Map<Position, Piece> pieces, final Path path) {
        if (hasPieceInMiddle(path, pieces)) {
            throw new IllegalArgumentException("[ERROR] 경로에 기물이 존재하여 이동할 수 없습니다.");
        }
    }

    @Override
    protected int moveY(Position arrivalPosition, int differenceForY, final int differenceForX, int currentY,
                        List<Position> positions,
                        int currentX) {
        int differenceUnitY = calculateUnit(differenceForY);
        while (currentY != arrivalPosition.getY()) {
            currentY += differenceUnitY;
            positions.add(Position.valueOf(currentY, currentX));
        }
        return currentY;
    }

    @Override
    protected int moveX(Position arrivalPosition, final int differenceForY, int differenceForX, int currentX,
                        List<Position> positions,
                        int currentY) {
        int differenceUnitX = calculateUnit(differenceForX);
        while (currentX != arrivalPosition.getX()) {
            currentX += differenceUnitX;
            positions.add(Position.valueOf(currentY, currentX));
        }
        return currentX;
    }

    @Override
    protected void validateMove(int differenceForY, int differenceForX) {
        if (canNotMove(differenceForY, differenceForX)) {
            throw new IllegalArgumentException("[ERROR] 차는 한 방향으로만 이동할 수 있습니다.");
        }
    }

    private boolean canNotMove(int differenceForY, int differenceForX) {
        return !((Math.abs(differenceForY) > 0 && Math.abs(differenceForX) == 0) ||
                (Math.abs(differenceForY) == 0 && Math.abs(differenceForX) > 0));
    }
}

 

추상 클래스의 한계

책임 과도화

다중 상속이 불가능하기 때문에, 한 클래스가 너무 많은 책임을 가질 가능성이 높다. 구현한Piece는 경로 생성 + 경로 검증 + 이동 검증의 책임을 가져 리팩토링이 필요하다.

 

큰 결합도

상속은 상위 클래스와 하위 클래스의 결합도가 매우 크다. 필드(상태)와 메서드(행위)를 모두 상속받으므로 상위 클래스 변경시에 하위 클래스가 영향을 크게 받는다.

 

오버라이드를 통해 LSP 위반 위험

상속에서 하위 클래스가 부모 클래스의 메서드를 오버라이드할 경우, LSP를 위반할 수 있다.LSP를 지키기 위해서는 부모 클래스의 메서드를 호출하거나 (super), 부모 클래스의 행위를 보존하면서 확장하도록 구현해야한다.

 

만약 하위 클래스의 메서드가 LSP를 위반한다면, 상속 대신 합성(컴포지션)을 이용하도록 리팩토링하자.

만약, 하위 클래스에서 오버라이드를 방지하고 싶다면, final 키워드를 붙이자.

 

 

2. 합성 (컴포지션)

현재 Piece 클래스는 한 클래스가 너무 많은 책임(움직임을 관리하는 책임, 팀별 기물을 관리하는 책임 등)을 가지므로 단일 책임 원칙(SRP)을 위반한다. 코드를 읽을 때도 한번에 추상 클래스가 어떤 책임을 가지는지 알기 어렵다. 따라서 Piece에 존재하던 많은 책임들을 세분화하여 객체로 쪼개고 합성하였다.

 

1. 팀 책임 Players로 이동

Piece에서 팀별 기물을 관리하는 책임(기물이 잡히는 행위 등등) 을 Players로 이동하였다. 팀으로 가진 기물들을 나누면 (Map<Team, Board>) 기물이 잡히거나, 움직일 수 있는지 쉽게 검증할 수 있다. 예를 들어 기물이 최종적으로 움직인 위치가 자신의 팀 기물이면 움직일 수 없고, 다른 팀 기물이면 잡을 수 있다.

public class Players {

    private static final int TOTAL_KING_COUNT = 2;

    private final Map<Team, Board> players;

    public Players(final Map<Team, Board> players) {
        this.players = new HashMap<>(players);
    }

    public final PieceMove move(final Position currentPosition, final Position arrivalPosition,
                                final Team currentTeam) {
        validateSamePosition(currentPosition, arrivalPosition);

        final Board currrentTeamBoard = players.get(currentTeam);
        final Board opponentBoard = players.get(currentTeam.getOppositeTeam());

        final Board totalPieces = getTotalBoard();
        currrentTeamBoard.validatePath(currentPosition, arrivalPosition, totalPieces);
        final Optional<Piece> caughtPieceOptional = catchPiece(arrivalPosition, currrentTeamBoard, opponentBoard);
        currrentTeamBoard.updatePiece(currentPosition, arrivalPosition);
        final Piece currentPiece = currrentTeamBoard.findPieceByPosition(arrivalPosition);

        if (caughtPieceOptional.isPresent()) {
            final Piece caughtPiece = caughtPieceOptional.get();
            return new PieceMove(true, currentTeam, currentPiece.getPieceType(), caughtPiece.getPieceType(),
                    currentPosition, arrivalPosition, true);
        }
        return new PieceMove(true, currentTeam, currentPiece.getPieceType(), null, currentPosition, arrivalPosition,
                false);
    }

    public Team findWinningTeam() {
        if (calculateExistKing() == TOTAL_KING_COUNT) {
            return compareByScore();
        }
        return players.entrySet().stream()
                .filter(entry -> entry.getValue().hasKing())
                .map(Entry::getKey)
                .findFirst()
                .orElseThrow(() -> new IllegalStateException("[ERROR] 왕이 존재하지 않을 수 없습니다."));
    }

    private Optional<Piece> catchPiece(final Position arrivalPosition,
                                       final Board currentTeamBoard,
                                       final Board oppositeBoard) {
        validateNotCatchingCurrentTeamPiece(currentTeamBoard, arrivalPosition);
        if (oppositeBoard.hasPiece(arrivalPosition)) {
            final Piece targetPiece = oppositeBoard.findPieceByPosition(arrivalPosition);
            oppositeBoard.removePiece(arrivalPosition);
            return Optional.of(targetPiece);
        }
        return Optional.empty();
    }

}

 

2. 응집도, 객체의 자율성 고려

기존에는 Piece가 위치를 파라미터로 받아 움직임을 처리했다. 하지만 기물이 위치를 상태로 가져 직접 관리하면 자율성 및 응집도가 높아진다. 따라서 Board에서 Piece로 위치 관리 책임을 넘겼다.

 

AS IS

public abstract class Piece {

    private final PieceType pieceType;
    private final Team team;

    public Piece(final PieceType pieceType, final Team team) {
        this.pieceType = pieceType;
        this.team = team;
    }

    public final Path makePath(final Position currentPosition, final Position arrivalPosition,
                         final Map<Position, Piece> pieces) {
        final int differenceForY = arrivalPosition.calculateDifferenceForY(currentPosition);
        final int differenceForX = arrivalPosition.calculateDifferenceForX(currentPosition);

        final Movement movement = findMovement(pieceType, differenceForY, differenceForX);
        final Path path = Path.from(pieceType, movement, currentPosition, arrivalPosition);
        validatePath(pieces, path);
        return path;
    }

 

TO BE

public class Piece {

    private final PieceMoveRule pieceMoveRule;
    private Position position;

    public Piece(final PieceMoveRule pieceMoveRule, final Position position) {
        this.pieceMoveRule = pieceMoveRule;
        this.position = position;
    }

    public boolean isSamePosition(final Position givenPosition) {
        return position.equals(givenPosition);
    }

    public void updatePosition(final Position arrivalPosition) {
        position = arrivalPosition;
    }

    public void validateMovement(final Position currentPosition, final Position arrivalPosition,
                                 final Board board) {
        if (getPieceType().canNotMoveDiagonal()) {
            final Optional<Movements> optionalMovements = PalaceMovement.getMovements(currentPosition);
            optionalMovements.ifPresent(pieceMoveRule::addMovement);
            pieceMoveRule.validatePath(currentPosition, arrivalPosition, board);
            optionalMovements.ifPresent(pieceMoveRule::deleteMovement);
            return;
        }
        pieceMoveRule.validatePath(currentPosition, arrivalPosition, board);
    }

    public boolean isObstacleJumping() {
        return getPieceType() == PieceType.CANNON;
    }

    public boolean matchPieceMovement(final PieceType givenPieceType) {
        return getPieceType() == givenPieceType;
    }


    public Position getPosition() {
        return position;
    }

    public PieceType getPieceType() {
        return pieceMoveRule.getPieceType();
    }
}

 

3. Piece는 위치 관리 책임만 갖도록 수정

그런데Piece에서 위치 관리와, 움직임 관리 두 책임 모두를 가지는 것은 책임이 과다하다고 느껴졌다. 따라서 합성을 이용해 움직임에 대한 객체를 분리하여 해결했다.

• PieceMoveRule (구체 클래스) : 이동 검증, 장애물 이동 전략을 주입받는다.

public class Piece {

    private final PieceMoveRule pieceMoveRule;
    private Position position;

    public Piece(final PieceMoveRule pieceMoveRule, final Position position) {
        this.pieceMoveRule = pieceMoveRule;
        this.position = position;
    }

    public boolean isSamePosition(final Position givenPosition) {
        return position.equals(givenPosition);
    }

    public void updatePosition(final Position arrivalPosition) {
        position = arrivalPosition;
    }

    public void validateMovement(final Position currentPosition, final Position arrivalPosition,
                                 final Board board) {
        if (getPieceType().canNotMoveDiagonal()) {
            final Optional<Movements> optionalMovements = PalaceMovement.getMovements(currentPosition);
            optionalMovements.ifPresent(pieceMoveRule::addMovement);
            // 위임
            pieceMoveRule.validatePath(currentPosition, arrivalPosition, board);
            optionalMovements.ifPresent(pieceMoveRule::deleteMovement);
            return;
        }
        // 위임 
        pieceMoveRule.validatePath(currentPosition, arrivalPosition, board);
    }

    public boolean isObstacleJumping() {
        return getPieceType() == PieceType.CANNON;
    }

    public boolean matchPieceMovement(final PieceType givenPieceType) {
        return getPieceType() == givenPieceType;
    }


    public Position getPosition() {
        return position;
    }

    public PieceType getPieceType() {
        return pieceMoveRule.getPieceType();
    }
}

 

위임도 책임일까?

리팩토링 후, Piece가 pieceMoveRule에 이동 책임을 위임함에도 여전히 해당 책임을 가지는지 궁금하였다. 구체적인 구현은 분리되더라도 Piece의 행위(public 메서드, API)는 그대로이기 때문이다.

 

리뷰어는 Piece가 이동 검증에 대한 인터페이스를 제공하기 때문에 여전히 책임을 가진다고 말씀해주셨다. 즉, 합성을 통해 구체적인 구현에 대한 책임을 분리한 것이지 Piece에서 이동 책임을 완전히 분리한 것은 아니다.

 

따라서 여전히Piece가 이동 검증과 위치 관리 두 책임을 가지므로, 이동 검증에 대한 책임만 갖도록 리팩토링하였다.

위치 관리는 Board의 책임으로 옮겼다.

public class Piece {

    private final PieceMoveRule pieceMoveRule;

    public Piece(final PieceMoveRule pieceMoveRule) {
        this.pieceMoveRule = pieceMoveRule;
    }

    public void validateMovement(final Position currentPosition, final Position arrivalPosition,
                                 final Board board) {
        pieceMoveRule.validatePath(currentPosition, arrivalPosition, board);
    }

    public boolean isObstacleJumping() {
        return pieceMoveRule.isObstacleJumping();
    }

    public boolean matchPieceMovement(final PieceType givenPieceType) {
        return getPieceType() == givenPieceType;
    }

    public PieceType getPieceType() {
        return pieceMoveRule.getPieceType();
    }
}

 

PieceMoveRule : 전략 패턴

이동이라는 책임 속에 이동 방식, 장애물에 대한 처리 방식만 다르기 때문에 전략 패턴을 사용했다.

• 움직임 전략(MoveStrategy): 보드 위의 간선으로만 움직일 수 있는지(EdgeMoveStrategy) 간선에 상관없이 움직이는지(RelativeMoveStrategy)로 나뉜다.
• 장애물 전략(ObstacleStrategy): 장애물이 있으면 움직일 수 없는지(ObstacleBlockStrategy)와 장애물이 있으면 뛰어넘을 수 있는지(ObstacleJumpingStrategy)로 나뉜다.

public class PieceMoveRule {

    private final PieceType pieceType;
    private final MoveStrategy moveStrategy;
    private final ObstacleStrategy obstacleStrategy;

    public PieceMoveRule(final PieceType pieceType, final MoveStrategy moveStrategy,
                         final ObstacleStrategy givenObstacleStrategy) {
        this.pieceType = pieceType;
        this.moveStrategy = moveStrategy;
        this.obstacleStrategy = givenObstacleStrategy;
    }

    public void validatePath(final Position currentPosition, final Position arrivalPosition, final Board board) {
        if (pieceType.doesLiveInPalace()) {
            currentPosition.validateIsInPalace(arrivalPosition);
        }
        final Movement movement = moveStrategy.move(currentPosition, arrivalPosition, pieceType);
        obstacleStrategy.checkObstacle(currentPosition, arrivalPosition, movement, board);
    }

    public boolean isObstacleJumping() {
        return obstacleStrategy.isObstacleJumping();
    }

    public PieceType getPieceType() {
        return pieceType;
    }
}

 

역할/ 책임 vs 전략

• 역할/책임 : 객체의 전체적인 성격과 책임, 본질적인 특성
• 전략 : 구체적인 구현(알고리즘), 동작이나 행위만 달라질 때

 

추가) 여러개의 전략을 가질 경우

현재 전략 2개를 PieceMoveRule에서 주입받는데, 경우의 수가 나뉠때마다 전략을 주입받는게 좋은 방법인지 궁금했다. 움직임 전략과 장애물 이동 전략을 하나의 전략으로 합치는 것도 생각해보았는데, 전혀 다른 성격이라 합치면 오히려 이상하다는 생각이 들었다. 

 

 

전략을 합치는 경우

• 두 전략이 항상 함께 변경되는 경우 (생명주기가 같을 때)
• 특정 조합만 유효하고 다른 조합은 의미가 없는 경우
• 두 전략 간에 강한 의존성이 있는 경우

현재는 전략이 서로 연관관계가 없고 독립적이므로 합칠 필요가 없다. :)

 

리팩토링 결과

• Piece : 이동 검증 (인터페이스 제공)
• PieceMoveRule : 기물 이동 검증 (구체 구현 제공)
• Players : 팀 관리
• Board : 기물 위치 관리

 

현재 설계의 한계

전략 패턴 : 잘못된 전략 주입 가능

기물별 움직이는 전략은 이미 고정이 되어있는데 전략을 선택하도록 하여 잘못된 조합의 전략이 주입될 수 있다.

 

잘못된 추상화

상위 클래스에서 하위 클래스 종류를 알아야하는 상황이 존재한다. ( isKing(), isJumpingPiece()) 예를 들어, 승패 결정을 위해 객체의 기물 종류가 왕인지 알아야한다. 상위 클래스에서 하위 클래스의 타입을 물어보는 것은 instanceof를 사용하는 것과 같다. 잘못된 추상화가 아닐까?

 

따라서 Piece라는 클래스가 아닌 PieceType이라는 enum이 상위 클래스가 되는 것이 더 올바른 추상화라는 결론에 이르렀다.

 

3. enum 사용 (최종)

1. 기물별 움직이는 전략을 enum으로 이동

public enum Piece {

    CANNON(
            new Movements(
                    new Movement(UP),
                    new Movement(RIGHT),
                    new Movement(LEFT),
                    new Movement(DOWN)
            ),
            MovementType.PALACE_CONSIDERATE,
            PathObstacleRule.MUST_JUMP_EXACTLY_ONE_OBSTACLE,
            7
    ),
    CHARIOT(
            new Movements(
                    new Movement(UP),
                    new Movement(DOWN),
                    new Movement(RIGHT),
                    new Movement(LEFT)
            ),
            MovementType.PALACE_CONSIDERATE,
            PathObstacleRule.CANNOT_PASS_OBSTACLES,
            13
    ),

기물의 특성으로 전략을 정의하여, 기물 별 전략이 고정되도록 수정했다. 이를 통해 기물들을 생성하는 PieceFactory에서 기물과 전략을 따로따로 주입하지 않아도 되고, 잘못된 전략이 주입되는 버그를 막을 수 있어서 명쾌해졌다.

 

public class Board {

    private final Map<Position, Piece> pieces;

    public Board(final Map<Position, Piece> pieces) {
        this.pieces = new HashMap<>(pieces);
    }

    public void move(final Position from, final Position to, final Board totalBoard) {
        final Piece piece = findPieceByPosition(from);
        piece.validateMovement(from, to, totalBoard);
    }

    public Piece findPieceByPosition(final Position position) {
        if (pieces.containsKey(position)) {
            return pieces.get(position);
        }
        throw new IllegalArgumentException("[ERROR] 해당 좌표에 자신의 팀 기물이 존재하지 않습니다.");
    }

    public boolean hasPiece(final Position position) {
        return pieces.containsKey(position);
    }

    public boolean hasKing() {
        return pieces.values().stream()
                .anyMatch(piece -> piece == Piece.KING);
    }

    public void removePiece(final Position position) {
        pieces.remove(position);
    }

    public void updatePiece(final Position currentPosition, final Position arrivalPosition) {
        final Piece currentPiece = findPieceByPosition(currentPosition);
        pieces.remove(currentPosition);
        pieces.put(arrivalPosition, currentPiece);
    }

Board는 Piece enum을 직접 가지므로 상위 타입에서 어떤 타입인지 물어보는 문제도 사라졌다. (올바르게 추상화가 되었다.)

 

2. 전략 객체 싱글턴으로 수정

public enum PathObstacleRule {

    CANNOT_PASS_OBSTACLES {
        @Override
        public void validatePathObstacles(final Path path, final Board board) {
            if (containsObstacles(path, board)) {
                throw new IllegalArgumentException("[ERROR] 경로에 기물이 존재하여 이동할 수 없습니다.");
            }
        }
    },
    MUST_JUMP_EXACTLY_ONE_OBSTACLE {
        private static final int OBSTACLE_JUMPING_THRESHOLD = 1;

        @Override
        public void validatePathObstacles(final Path path, final Board board) {
            final int count = countObstacles(path, board);
            if (count != OBSTACLE_JUMPING_THRESHOLD) {
                throw new IllegalArgumentException("[ERROR] 오직 하나의 기물만 뛰어넘을 수 있습니다.");
            }
            if (wouldJumpSameTypePiece(path, board)) {
                throw new IllegalArgumentException("[ERROR] 같은 종류의 기물을 뛰어넘거나 잡을 수 없습니다.");
            }
        }
    };

여러 기물들이 같은 전략을 공유함에도 매번 전략들이 생성되는 문제가 있었다. 따라서 싱글톤으로 구현하여 객체가 전략별로 하나만 생성되도록 리팩토링했다.

 

enum 설계의 한계

리팩토링 후, 리뷰어가 이러한 설계가 가져올 수 있는 한계를 고민해보라고 하셨다. 현재는 기물이 두개의 전략만 존재하지만, 기물별로 가진 전략의 수가 달라질 수 있기 때문이다.

 

각 PieceType별 객체를 만든다면?

장점 : 캡슐화

기물의 필드로 움직임과 전략을 선언해둔다면 좀 더 객체지향적인 코드가 될 수 있다. 현재는 포의 움직임이 변경될 때, piece라는 enum 파일을 수정해야하지만, 각 클래스를 만들면 cannon 클래스만 수정하면 되기 때문이다. 전략 주입 구조가 변경될 경우(Cannon에 ObstacleRule이 필요없어질 때나 두 개 이상 전략을 가질 경우)에도 해당 객체만 수정하는 것이 enum의 구조를 변경하는 것보다 훨씬 유지보수 관점에서 좋다.

 

단점 : 데이터만 가진 클래스

움직임과 전략만 가진 클래스들만 생성된다. 단지 각 움직임과 전략을 제공하는 역할만 하는 것이 좋은 객체인가?

차라리 enum이 나을 것 같다.

 

결론

enum을 사용하는 것이 추상화나 구조 측면에서 명확하다. 하지만 만약 도메인의 규칙이 변경되어서 기물의 움직임 전략 여러개를 조합하게되거나, 기물별 상태를 관리해야된다면 기물 클래스 각각을 만들어 확장을 고려할 것 같다. (YAGNI)

 

리뷰어의 질문 덕분에 장기 게임에 대한 설계에 대해 더 깊게 고민할 수 있어 좋았다. 

 

DAO

Manager 객체 도입

도메인에서 dao를 의존하기보다, 도메인과 dao의 중간 연결다리 역할을 하는 Manager를 도입하였다. 이를 통해 데이터 접근 로직과 도메인(비즈니스 로직)을 분리할 수 있다.

public class PieceHistoryManager {

    private final PieceDao pieceDao;

    public PieceHistoryManager(final PieceDao pieceDao) {
        this.pieceDao = pieceDao;
    }

    public void initialize(final Players players) {
        pieceDao.deleteAll();
        initializeChoPieces(players.getChoPieces());
        initializeHanPieces(players.getHanPieces());
    }

    public void updatePiece(final PieceMove pieceMove) {
        if (pieceMove.isCaught()) {
            pieceDao.delete(pieceMove);
        }
        pieceDao.update(pieceMove);
    }

    private void initializeChoPieces(final Board choPieces) {
        for (final Piece piece : choPieces.getPieces()) {
            final Position position = piece.getPosition();
            pieceDao.insert(new PieceDto(Team.CHO, piece.getPieceType(), position.getY(), position.getX()));
        }
    }

 

테스트

1. DAO 쿼리 테스트

쿼리가 잘 동작하는지 테스트하기 위해 도커로 Test DB를 띄워 테스트하였다.

public class PieceDaoTest {

    private final PieceDao pieceDao = TestPieceDaoImpl.getPieceDao();

    @BeforeEach
    void setUp() {
        pieceDao.deleteAll();

        final PieceDto hanKingPiece = TestFixture.makeHanKingPiece();
        pieceDao.insert(hanKingPiece);
        final PieceDto choKingPiece = TestFixture.makeChoKingPiece();
        pieceDao.insert(choKingPiece);
    }

    @Test
    void 조회_기능_테스트() {
        // Given

        // When
        final List<PieceDto> choPieces = pieceDao.select(Team.HAN);

        // Then
        assertThat(choPieces).containsOnly(TestFixture.makeHanKingPiece());
    }

    @Test
    void 전체_조회_기능_테스트() {
        // Given

        // When
        final List<PieceDto> pieceDtos = pieceDao.selectAll();

        // Then
        assertThat(pieceDtos).containsAll(List.of(TestFixture.makeChoKingPiece(), TestFixture.makeHanKingPiece()));
    }
    // 등등

 

2. Manager는 FakeDao로 테스트

DB 연결 실패와 상관없이 Manager 로직을 안정적으로 테스트하기 위해 Fake객체를 생성하여 주입하였다.

public interface PieceDao {  

    List<PieceDto> select(Team team);  

    List<PieceDto> selectAll();  

    void insert(PieceDto pieceDto);  

    void update(PieceMove pieceMove);  

    void delete(PieceMove pieceMove);  

    void deleteAll();  
}

public class FakePieceDao implements PieceDao {

    private final List<PieceDto> dtos = new ArrayList<>();  

    @Override  
    public List<PieceDto> select(final Team team) {  
        return dtos.stream()  
                .filter(dto -> dto.team().equals(team))  
                .toList();  
    }  

    @Override  
    public List<PieceDto> selectAll() {  
        return Collections.unmodifiableList(dtos);  
    }  

    @Override  
    public void insert(final PieceDto pieceDto) {  
        dtos.add(pieceDto);  
    }  

    @Override  
    public void update(final PieceMove pieceMove) {  
        final Position currentPosition = pieceMove.from();  
        final PieceDto pieceDto = new PieceDto(pieceMove.team(), pieceMove.piece(), currentPosition.getY(),  
                currentPosition.getX());  
        dtos.remove(pieceDto);  
        final Position arrivalPosition = pieceMove.to();  
        final PieceDto updatedDto = new PieceDto(pieceMove.team(), pieceMove.piece(), arrivalPosition.getY(),  
                arrivalPosition.getX());  
        dtos.add(updatedDto);  
    }  

    @Override  
    public void delete(final PieceMove pieceMove) {  
        if (pieceMove.isNotCaptured()) {  
            return;  
        }  
        final Position arrivalPosition = pieceMove.to();  
        final PieceDto pieceDto = new PieceDto(pieceMove.team().getOppositeTeam(), pieceMove.caughtPiece().get(),  
                arrivalPosition.getY(), arrivalPosition.getX());  
        dtos.remove(pieceDto);  
        System.out.println(pieceDto);  
    }  

    @Override  
    public void deleteAll() {  
        dtos.clear();  
    }  
}

class PieceHistoryManagerTest {  

    private final FakePieceDao fakePieceDao = new FakePieceDao();  
    private final PieceHistoryManager pieceHistoryManager = new PieceHistoryManager(fakePieceDao);  

    @Test  
    void 초기화한다() {  
        // Given  
        final PiecesFactory piecesFactory = new PiecesFactory();  
        final Board choBoard = piecesFactory.initializeChoPieces(BoardOrder.ELEPHANT_HORSE_ELEPHANT_HORSE);  
        final Board hanBoard = piecesFactory.initializeHanPieces(BoardOrder.ELEPHANT_HORSE_ELEPHANT_HORSE);  
        final Players players = new Players(Map.of(Team.CHO, choBoard, Team.HAN, hanBoard));  

        // When  
        pieceHistoryManager.initialize(players);  

        // Then  
        assertThat(fakePieceDao.selectAll()).hasSize(32);  
    }  

    @Test  
    void 기물이_없을_경우_초기화해야한다() {  
        assertThat(pieceHistoryManager.mustBeInitialize()).isTrue();  
    }  

    @Test  
    void 두_팀_중_하나의_왕이라도_죽었을_경우_초기화해야한다() {  
        // Given  
        fakePieceDao.insert(new PieceDto(Team.CHO, Piece.KING, 2, 5));  

        // When & Then  
        assertThat(pieceHistoryManager.mustBeInitialize()).isTrue();  
    }

 

3. Dao 중복 코드 제거

Dao 내부에서 커넥션을 얻고 쿼리를 실행하는 코드 사이에 중복되는 코드가 많이 존재했다. 피드백 주신 내용 중에,JdbcTemplate 처럼 템플릿을 이용하여 중복 코드를 줄일 수 있다는 내용을 반영해서 리팩토링했다.

 

템플릿 콜백 패턴

   public abstract class BaseDao {

protected final <T> List<T> findAll(final String query, final StatementSetter setter, final ResultMapper<T> resultMapper) {
        final List<T> results = new ArrayList<>();
        try (final Connection connection = databaseConnectionProvider.getConnection();
             final PreparedStatement preparedStatement = connection.prepareStatement(query)) {

            if (setter != null) {
                setter.setParameters(preparedStatement, connection);
            }

            final ResultSet resultSet = preparedStatement.executeQuery();
            while (resultSet.next()) {
                results.add(resultMapper.mapRow(resultSet, connection));
            }
            return results;

        } catch (final SQLException e) {
            throw new RuntimeException(e);
        }
    }

    protected final <T> List<T> findAll(final String query, final ResultMapper<T> resultMapper) {  
        return findAll(query, null, resultMapper);  
    }  

    protected final <T> T findOne(final String query, final StatementSetter setter,  
                                  final ResultMapper<T> resultMapper) {  
        try (final Connection connection = databaseConnectionProvider.getConnection();  
             final PreparedStatement preparedStatement = connection.prepareStatement(query)) {  

            if (setter != null) {  
                setter.setParameters(preparedStatement, connection);  
            }  

            final ResultSet resultSet = preparedStatement.executeQuery();  
            if (resultSet.next()) {  
                return resultMapper.mapRow(resultSet, connection);  
            }  
        } catch (final SQLException e) {  
            throw new RuntimeException(e);  
        }  
        throw new IllegalStateException("[ERROR] 해당 데이터를 찾을 수 없습니다.");  
    }

    @FunctionalInterface  
    protected interface StatementSetter {  
        void setParameters(PreparedStatement preparedStatement, Connection connection) throws SQLException;  
    }  

    @FunctionalInterface  
    protected interface ResultMapper<T> {  
        T mapRow(ResultSet resultSet, Connection connection) throws SQLException;  
    }

public class TurnDaoImpl extends BaseDao implements TurnDao {
    @Override
    public Turn selectCurrentTeam() {
        final var query = "SELECT current_team FROM turn LIMIT 1";
        // 사용 부분
        return findOne(query, (resultSet, connection) -> {
            final String teamName = getTeamNameById(connection, resultSet.getInt("current_team"));
            return Turn.initialize(Team.from(teamName));
        });
    }

아쉬운 점은 BaseDao를 상속받는 방식보다 합성을 사용하거나 util로 만들었다면 더 재사용성이 높았을 것 같다.

 

참고) 함수형 인터페이스

 @FunctionalInterface
  protected interface StatementSetter {
      void setParameters(PreparedStatement preparedStatement, Connection connection) throws SQLException;
  }

함수형 인터페이스로 선언하기 위해서는 단 하나의 추상 메서드만 가져야한다. 함수형 인터페이스는 함수를 일급 객체로 취급하여 파라미터로 넘길 수 있다.

 

  executeUpdate(query, (preparedStatement, connection) -> {
            preparedStatement.setInt(1, getTeamIdByName(connection, team.name()));
        });

메서드를 호출하는 사람이 PreparedStatement를 받아 파라미터를 설정할 수 있다(콜백). 이를 통해 익명 클래스를 사용하는 것보다 간결하게 코드를 작성할 수 있다.

 

DB

DB 설계

DB 설계시 코드와 같은 구조를 가질지 고민해보았지만, DB 테이블과 객체지향의 객체는 특성이 다른 것 같아 고려하지 않았다. DB에 저장할 필요가 있는 정보를 생각해보았을 때, 필요한 정보는 각 팀별 기물들의 위치와 현재 턴 뿐이었다. 따라서 Team, PieceType, Piece, Turn 테이블을 생성하였다.

 

 

enum

CREATE TABLE team
(
    team_id int                 NOT NULL AUTO_INCREMENT,
    name    ENUM ('HAN', 'CHO') NOT NULL,
    PRIMARY KEY (team_id)
);

리뷰어가 이미 정해진 값들이고 변경 가능성이 낮다면 ENUM 타입을 사용하는 것을 제안해주셨다. 알아보니 enum 타입을 사용하면 컬럼 범위를 제약할 수 있어 의도치 않은 값이 추가되는 것을 막을 수 있는 장점이 있다.

 

DTO

상속 구조로 설계

DTO에 데이터베이스에 업데이트할 파라미터와 흐름 처리를 위한 파라미터가 함께 존재하는 문제가 있었다. 따라서 MoveResult라는 응답에 대한 상위 객체를 생성해서, 데이터 전달과 흐름 처리 책임을 분리하였다.

 

AS IS

public record PieceMove(Team team, Piece piece, Position from, Position to, Optional<Piece> caughtPiece) {

    public PieceMove(final Team team, final Piece piece, final Position currentPosition, final Position arrivalPosition) {
        this(team, piece, currentPosition, arrivalPosition, Optional.empty());
    }

    public static PieceMove capture(final Team team, final Piece piece, final Position from, final Position to, final Piece capturedPiece) {
        return new PieceMove(team, piece, from, to, Optional.ofNullable(capturedPiece));
    }

    public boolean isNotCaptured() {
        return caughtPiece.isEmpty();
    }
}

 

TO BE

public record MoveResult(MoveStatus moveStatus, PieceMove pieceMove) {  

    public static MoveResult exit() {  
        return new MoveResult(MoveStatus.EXIT, null);  
    }  

    public static MoveResult moveCompleted(final PieceMove pieceMove) {  
        return new MoveResult(MoveStatus.MOVE_COMPLETED, pieceMove);  
    }  

    public boolean isExit() {  
        return moveStatus == MoveStatus.EXIT;  
    }  
}

public record PieceMove(Team team, Piece piece, Position from, Position to, Optional<Piece> caughtPiece) {  

    public PieceMove(final Team team, final Piece piece, final Position currentPosition,  
                     final Position arrivalPosition) {  
        this(team, piece, currentPosition, arrivalPosition, Optional.empty());  
    }  

    public static PieceMove capture(final Team team, final Piece piece, final Position from, final Position to,  
                                    final Piece capturedPiece) {  
        return new PieceMove(team, piece, from, to, Optional.ofNullable(capturedPiece));  
    }  

    public boolean isNotCaptured() {  
        return caughtPiece.isEmpty();  
    }  
}

 

PR

https://github.com/woowacourse/java-janggi/pull/30

[1단계 - 장기] 밍트(김명지) 미션 제출합니다. by Starlight258 · Pull Request #30 · woowacourse/java-janggi

 

https://github.com/woowacourse/java-janggi/pull/134

[2단계 - 장기] 밍트(김명지) 미션 제출합니다. by Starlight258 · Pull Request #134 · woowacourse/java-janggi

 

https://github.com/woowacourse/java-janggi/pull/173

[2.5단계 - 장기 리팩토링] 밍트(김명지) 미션 제출합니다. by Starlight258 · Pull Request #173 · woowacourse

 

레벨 1이 끝이 났다 !