각종 후기/우아한테크코스

[우아한 테크코스 3기] LEVEL 1 회고 (46일차)

제이온 (Jayon) 2021. 3. 19.

안녕하세요? 제이온입니다.

 

오늘은 오전에 포비 수업을 듣고, 오후에는 아론과 만나서 체스 미션을 리팩토링하였습니다.

 

 

데일리 미팅

오늘 데일리 미팅 진행자는 삭정이었습니다. 삭정은 ZOOM 화면 공유 기능을 이용하여 확대된 사진을 보고 무엇인지 맞히는 게임을 진행해 주셨습니다. 인물과 동물 두 가지 카테고리로 문제가 나왔는데.. 저는 하나도 못 맞혔습니다. 눈만 보고 정답을 말하는 크루들을 보고 대단하다고 생각하였습니다.

 

결국 마지막까지 다 틀려서 다음 주 화요일 데일리 미팅 진행자가 되고 말았습니다. 다들 게임을 준비해 오는 추세인데 저는 뭘 해야할지 고민이네요.

 

 

엘레강트 오브젝트 수업

포비가 엘레강트 오브젝트 수업을 진행해 주셨습니다. 이 강의에서는 총 4가지 섹션을 다루었습니다. '-er로 끝나는 이름을 사용하지 마세요', '생성자 하나를 주 생성자로 만드세요', '생성자에 코드를 넣지 마세요', '메서드 이름을 신중하게 선택하세요' 입니다. 여기서, 생성자 하나를 주 생성자로 만들라는 주제 외에는 크루들의 의견이 상충되는 모습을 보였습니다.  (사실, 다른 주제도 나왔던 것 같은데 많은 내용을 다뤄서 다 기억이 나지는 않네요.)

 

'-er로 끝나는 이름을 사용하지 마세요.'는 객체의 이름을 지을 때, '무엇을 하는지'에 대해 생각하지 말고 이 객체는 '무엇인지'에 대해 고려하라고 말씀해 주셨습니다. 그런데, 저는 '무엇을 하는지'는 객체의 역할을 고려하여 판단한 것이므로 올바르게 이름을 정한 것이 아닌가 의문을 가졌습니다. 이에 대해 포비는 '무엇인지'가 '무엇을 하는지'를 포괄하는 개념이므로 그렇게 짓는 것이 더 바람직하다고 답변해 주셨습니다.

 

'생성자에 코드를 넣지 마세요.'는 생성자를 최대한 단순하게 설계하라는 것입니다. 값을 초기화만 해야지, 이것저것 코드의 조작을 일으켜서는 안 된다고 합니다. 그런데, 메소드를 묶어서 코드를 조작하는 대신 다른 객체를 new 키워드로 만들어서 인자를 넘기는 모습을 보였습니다. 예를 들어, 해당 객체의 필드 자료형이 String이라면 생성자의 인자가 int라고 해서 'Integer.parseInt()' 메소드를 사용하지 말라는 것입니다. 대신, 이 메소드를 사용하는 A라는 객체를 이용하여 'new A(num).toString()'과 같이 나타내라고 합니다.

 

여기서 저는 new 키워드를 이용하여 객체를 만들어서 메소드를 반환하는 것도 결국 코드를 넣는 것이 아닌가 생각은 들었습니다. 이 부분은 생성자가 단순해야 하는 것은 동의하지만, new 키워드를 통해 객체를 장풍마냥 쭉 생성하는 것이 가독성 면에서 좋지 않다고 생각하였습니다. 제가 제대로 이해를 못 한 걸 수도 있어서 다시 알아 봐야겠습니다.

 

다음으로, '메서드 이름을 신중하게 선택하세요.'입니다. 여기서 메서드 이름을 정하는 방법이 여러 가지 나와 있습니다. 가령, 반환값이 있는 메소드는 명사, void는 동사, boolen 반환값은 형용사 등으로 지으라는 것입니다. 특히, 우리가 사랑하는 getter 메소드는 특정 클래스의 필드가 무엇이 있는지 밝히는 것이므로 캡슐화를 깨뜨리는 것이라고 주장한 부분은 흥미로웠습니다. 시간이 되면 이번 체스 미션에 적용할 계획입니다.

 

전반적으로 의문점이 더 많이 남았던 수업이지만, OOP 설계에 대해 다른 시각으로 생각해 볼 수 있었다고 생각합니다.

 

 

페어 프로그래밍

오늘은 어제 기능을 모두 구현한 체스 미션을 리팩토링해 보았습니다. 전반적으로 한 쪽 객체에 책임이 몰린 부분을 분산하고, 메소드의 길이를 10줄로 맞추려고 노력하였습니다. 그리고 테스트를 하면서 버그가 생긴 부분을 모두 수정해 보았습니다.

 

먼저, 기존의 블랙팀과 화이트팀을 나누던 boolean 타입의 변수 대신 Team이라는 열거형 타입을 정의하였습니다.

 

 

public enum Team {
    BLACK("black"),
    WHITE("white"),
    NOTHING("");

    private final String teamName;

    Team(final String teamName) {
        this.teamName = teamName;
    }

    public String teamName() {
        return this.teamName;
    }

    public String oppositeTeamName() {
        return this.oppositeTeam().teamName;
    }

    public Team oppositeTeam() {
        if (this == BLACK) {
            return WHITE;
        }
        if (this == WHITE) {
            return BLACK;
        }
        return this;
    }
}

 

 

위와 같이 Team을 도입함으로써 코드의 가독성을 높일 수 있었습니다. 사실, 개발을 하면서도 true가 블랙팀인지 false가 블랙팀인지 계속 헷갈렸는데 이 문제를 해결한 것이죠.

 

또한, 저는 Piece에 상속을 받는 빈 공간 객체를 만들었는데, 이 객체의 팀을 NOTHING로 정해주어 버그를 막았습니다. boolean 타입은 2가지 값만 존재하므로 블랙과 화이트팀은 구분이 가능한데, 아무런 팀도 없는 빈 공간은 적절한 값을 주기 어려웠습니다. 그래서 프로그램을 실행하면서 빈 공간에 대한 에러가 발생하게 되었고, 열거형 타입을 통해 이를 해결하였습니다.

 

이외에 뷰에서 출력하기도 쉬웠습니다. 해당 열거형 상수가 출력할 이름도 갖고 있기 때문이죠.

 

 

다음으로, 컨트롤러를 리팩토링하였습니다

 

 

   private void proceedMain(final Board board, Team team) {
        try {
            proceed(board, team);
        } catch (IllegalStateException | IllegalArgumentException e) {
            OutputView.printErrorMessage(e.getMessage());
            proceedMain(board, team);
        }
    }

    private void proceed(final Board board, Team team) {
        OutputView.printCurrentBoard(board.unwrap());
        final List<String> runtimeCommands = InputView.inputRuntimeCommand();

        if (InputView.END_COMMAND.equals(runtimeCommands.get(0))) {
            showResult(board);
            return;
        }

        final Position start = getPositionByCommands(runtimeCommands.get(1).split(""));
        final Position end = getPositionByCommands(runtimeCommands.get(2).split(""));
        board.move(start, end, team);

        if (board.isKingDead()) {
            showResult(board);
            return;
        }
        team = team.oppositeTeam();
        proceed(board, team);
    }

.

 

위 두 가지 메소드는 게임 진행을 담당하는 역할을 합니다. 이 중에서 proceed() 메소드의 길이가 길다는 문제가 있었습니다. 이 중에서 board.move() 메소드와 함께 2줄의 Position을 다른 메소드로 추출할 수 있었습니다.

 

다만, 사용자가 "end"를 입력할 때와 블랙 또는 화이트팀의 킹이 잡혔을 때 모두 게임을 종료하지만 비슷한 구조의 조건문을 2번 사용할 수 밖에 없었습니다. 왜냐하면, 해당 메소드가 재귀 함수로 되어 있었고,  종료되는 시점이 조금 달랐기 때문이죠.

 

그래서 어떻게 해야하나 고민하고 있었는데, 아론이 두 가지 종료 조건에 대해 리턴을 하지 말고 throw로 에러를 던지자고 하였습니다. 저는 게임 종료를 에러로 볼 수 있는지 의문이 들었는데, 게임을 진행하는 메소드 입장에서는 "게임 종료" 자체를 에러로 판단할 수도 있겠다는 생각이 들었습니다. 이것이 바람직한 설계인지는 모르겠으나, 저로서는 새로운 시도였기에 굉장히 참신하다고 느껴졌습니다.

 

 

public class ChessController {
    public void runChess() {
        OutputView.printStartGameMessage();
        if (!InputView.inputInitialCommand()) {
            return;
        }

        final Board board = new Board();
        final Team team = Team.WHITE;
        proceedMain(board, team);
    }

    private void proceedMain(final Board board, final Team team) {
        try {
            proceed(board, team);
        } catch (IllegalStateException | IllegalArgumentException e) {
            OutputView.printErrorMessage(e.getMessage());
            proceedMain(board, team);
        } catch (GameOverException e) {
            showResult(board, e.getMessage());
        }
    }

    private void proceed(final Board board, final Team team) {
        OutputView.printCurrentBoard(board.unwrap());
        final List<String> runtimeCommands = InputView.inputRuntimeCommand();

        checkGameOver(InputView.END_COMMAND.equals(runtimeCommands.get(0)), "게임이 강제 종료되었습니다.");
        move(board, team, runtimeCommands);
        checkGameOver(board.isKingDead(), team.oppositeTeamName() + "의 king이 죽어서 게임이 종료되었습니다.");

        proceed(board, team.oppositeTeam());
    }

    private void checkGameOver(final boolean isGameOver, final String message) throws GameOverException {
        if (isGameOver) {
            throw new GameOverException(message);
        }
    }

    private void move(final Board board, final Team team, final List<String> runtimeCommands) {
        final Position start = getPositionByCommands(runtimeCommands.get(1).split(""));
        final Position end = getPositionByCommands(runtimeCommands.get(2).split(""));
        board.move(start, end, team);
    }

    private Position getPositionByCommands(final String[] commands) {
        return new Position(commands[0], commands[1]);
    }

    private void showResult(final Board board, final String message) {
        OutputView.printErrorMessage(message);
        OutputView.printGameResultNotice();
        if (InputView.isStatusInput()) {
            OutputView.printResult(board.getWinner(), board);
        }
    }
}

 

 

그래서 위와 같이 게임이 종료될 경우 throw를 하도록 수정하였습니다. 코드도 훨씬 간결해져서 만족스러웠습니다.

 

 

다만, 플레이어의 턴을 바꿔 주는 과정에서 버그가 발생하였습니다. proceed() 메소드 맨 마지막에서 턴을 바꾸는데, 에러가 발생한다면 턴이 화이트팀으로 고정되는 문제였죠. 왜냐하면, runChess() 메소드에서 team 변수를 화이트팀으로 초기화하고, proceedMain() 메소드의 인자로 넘겨주기 때문이었습니다. proceedMain() 메소드의 team은 화이트팀으로 고정되어있으므로 에러가 발생한다면 해당 proceedMain()의 team은 "항상" 화이트 팀이 되는 것입니다.

 

이를 해결하기 위하여 턴을 관리하는 Turn 객체를 정의하였습니다.

 

 

public class Turn {
    private Team team;

    public Turn() {
        team = Team.WHITE;
    }

    public Team now() {
        return team;
    }

    public void next() {
        team = team.oppositeTeam();
    }
}

 

 

컨트롤러 내에서 team 변수를 바꾸는 것이 아니라, Turn 객체 안에서 team 변수를 변경하도록 수정하였습니다.

 

 

다음으로, Board에 있던 점수 계산과 우승자 결정 로직을 ChessResult로 옮겼습니다.

 

 

public class ChessResult {
    private final Board board;

    public ChessResult(final Board board) {
        this.board = board;
    }

    public double calculateScore(final Team team) {
        double total = 0;
        for (final Horizontal column : Horizontal.values()) {
            total += getColumnTotalScore(team, column.getValue());
        }
        return total;
    }

    private double getColumnTotalScore(final Team team, final int column) {
        final Map<Position, Piece> chessBoard = new TreeMap<>(board.unwrap());
        final List<Piece> pieces = chessBoard.keySet().stream()
                .filter(position -> position.getHorizontal().getValue() == column)
                .map(chessBoard::get)
                .filter(piece -> piece.isSameTeam(team))
                .collect(Collectors.toList());

        return pieces.stream()
                .mapToDouble(Piece::getScore)
                .reduce(0, Double::sum) - getPawnDiscountScore(pieces);
    }

    private double getPawnDiscountScore(final List<Piece> pieces) {
        long count = pieces.stream()
                .filter(piece -> piece instanceof Pawn)
                .count();

        if (count >= 2) {
            return count * Pawn.EXTRA_SCORE;
        }
        return 0;
    }

    public Team getWinner() {
        final Map<Position, Piece> chessBoard = new TreeMap<>(board.unwrap());
        if (board.isKingDead()) {
            return kingSlayerTeam(chessBoard);
        }

        return scoreWinner();
    }

    private Team kingSlayerTeam(Map<Position, Piece> chessBoard) {
        return chessBoard.values().stream()
                .filter(piece -> piece instanceof King)
                .map(Piece::team)
                .findFirst()
                .orElseThrow(IllegalArgumentException::new);
    }

    private Team scoreWinner() {
        if (calculateScore(Team.BLACK) > calculateScore(Team.WHITE)) {
            return Team.BLACK;
        }

        if (calculateScore(Team.BLACK) < calculateScore(Team.WHITE)) {
            return Team.WHITE;
        }
        return Team.NOTHING;
    }

}

 

 

별다른 건 없고 단순히 로직을 이동시켰습니다.

 

 

마지막으로, Blank 객체를 싱글톤 패턴으로 수정하였습니다.

 

 

public class Blank extends Piece {
    private static final Piece blank = new Blank();
    private static final String INITIAL_NAME = ".";

    private Blank() {
        super(Team.NOTHING, INITIAL_NAME);
    }

    public static Piece getInstance() {
        return blank;
    }

    @Override
    public boolean canMove(final Position source, final Position target, final Piece piece) {
        throw new IllegalStateException("비어 있는 칸입니다.");
    }

    @Override
    public double getScore() {
        return 0;
    }
}

 

 

이것을 한 이유는 간단합니다. 

 

 

    public void move(final Position source, final Position target, final Team team) {
        validateRightTurn(source, team);
        if (checkPath(source, target)) {
            chessBoard.put(target, chessBoard.get(source));
            chessBoard.put(source, new Blank());
            return;
        }
        throw new IllegalArgumentException("해당 위치로 이동할 수 없습니다.");
    }

 

 

위 메소드는 체스말을 이동시키는 것인데, 시작점에 있던 말을 빈 공간으로 만들어 주기 위하여 매번 Blank 객체를 새로 할당하는 것을 알 수 있습니다. Blank는 단순히 비어 있는 공간이므로 Blank 객체 내에서 메모리를 한 번만 할당하고, 그것을 돌려 쓰기로 하였습니다.

 

그 외에 자질구레한 버그는 간단하게 해결할 수 있었습니다.

 

 

정리

굵직굵직한 리팩토링은 모두 시간 안에 마쳐서 매우 만족스러운 날이었습니다. 4일 동안 함께 아론과 체스 미션을 완수해서 재미있었고, 다음에도 협업하는 기회가 있었습니다. 특히, 한 쪽이 20 ~ 30% 부족한 로직을 내놓으면, 반대 쪽에서 부족한 점을 보완하는 로직을 제안하면서 큰 어려움 없이 개발을 진행했던 것 같습니다. 가장 어려웠던 미션을 잘 맞는 크루와 끝까지 잘 마무리해서 운이 좋았다고 생각합니다. 아무쪼록 주말에 간단한 수정 작업만 거쳐서 pr을 날리려고 합니다.

댓글

추천 글