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

[우아한 테크코스 3기] LEVEL 1 회고 - 블랙잭 2단계 미션의 1차 피드백을 받아보다 (41일차)

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

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

 

어제 피드백을 받아서, 오늘 제 나름대로 피드백을 반영해 보았습니다.

 

 

1차 피드백

 

 

첫 번째 수정 사항은 동일한 인자를 갖는 메소드의 인자들을 클래스의 필드로 빼는 것입니다. 

 

 

    public Map<Name, Integer> makeParticipantResults(final Participant dealer,
        final List<Participant> players) {
        final Map<Name, Integer> participantResults = new LinkedHashMap<>();
        participantResults.put(dealer.getName(), -calculateTotalPlayersRate(dealer, players));

        for (final Participant player : players) {
            final Result result = player.decideWinner(dealer);
            participantResults.put(player.getName(), (int) result.calculateRate(player.getMoney()));
        }
        return participantResults;
    }

    public int calculateTotalPlayersRate(final Participant dealer,
        final List<Participant> players) {
        double total = 0;
        for (final Participant player : players) {
            final Result result = player.decideWinner(dealer);
            total += result.calculateRate(player.getMoney());
        }
        return (int) total;
    }

 

 

수정 전 BlackjackGame 내의 있던 메소드인데, 두 메소드의 인자로 동일한 값을 받는 것을 알 수 있습니다. 따라서, 리뷰어님은 이것들을 BlackjackGame의 필드로 빼라고 말씀해 주셨습니다.

 

 

 

 

두 번째 피드백은 특정 객체를 생성하는 책임을 컨트롤러가 아닌 도메인에 전가하라는 것입니다. 

 

 

    private Participants createParticipants() {
        final Names names = requestName();
        final List<Double> moneys = new ArrayList<>();
        for (final Name name : names.toList()) {
            final double value = InputView.requestMoney(name);
            moneys.add(value);
        }
        return new Participants(names, new ArrayList<>(moneys));
    }

 

 

컨트롤러의 위 메소드를 통해서 Participants 객체를 생성하여 반환을 합니다. 여기서, 이름과 배팅 금액들은 사용자에게 입력을 받되, Participants를 생성하는 책임은 BlackjackGame으로 넘겼습니다.

 

 

    public BlackjackGame(final Names names, final List<Double> moneys) {
        participants = new Participants(names, moneys);
    }

 

 

간단하게 이러한 방식으로 수정하였습니다.

 

 

 

 

세 번째 수정 사항은 Map<Name, Integer> 형태로 반환하기보다는 특정 도메인 객체로 반환하는 것입니다.

 

 

    public ParticipantResult makeParticipantResults() {
        final Map<Name, Integer> participantResults = new LinkedHashMap<>();
        participantResults.put(dealer.getName(), -calculateTotalPlayersRate());

        for (final Participant player : players) {
            final Result result = player.decideWinner(dealer);
            participantResults.put(player.getName(), (int) result.calculateRate(player.getMoney()));
        }
        return new ParticipantResult(participantResults);
    }

 

 

그래서 BlackjackGame 내의 makeParticipantResults의 반환값을 Map이 아닌 ParticipantResult 정도로 수정하였습니다. 하지만, 이렇게 코드를 짜니까 ParticipantResult는 단순히 Map<Name, Integer>인 변수를 필드로 갖고 이 변수를 반환하는 메소드밖에 존재하지 않았습니다. 따라서, BlackjackGame 내의 makeParitipantResults 메소드를 ParticipantResult로 옮기기로 하였습니다.

 

 

public class ParticipantResults {

    private final Participant dealer;
    private final List<Participant> players;
    private final Map<Name, Integer> results;

    public ParticipantResults(final Participant dealer, final List<Participant> players) {
        this.dealer = dealer;
        this.players = players;
        results = makeParticipantResults();
    }

    public Map<Name, Integer> makeParticipantResults() {
        final Map<Name, Integer> participantResults = new LinkedHashMap<>();
        participantResults.put(dealer.getName(), -calculateTotalPlayersRate());

        for (final Participant player : players) {
            final Result result = player.decideWinner(dealer);
            participantResults.put(player.getName(), (int) result.calculateRate(player.getMoney()));
        }
        return participantResults;
    }

    public int calculateTotalPlayersRate() {
        double total = 0;
        for (final Participant player : players) {
            final Result result = player.decideWinner(dealer);
            total += result.calculateRate(player.getMoney());
        }
        return (int) total;
    }

    public Map<Name, Integer> getResults() {
        return results;
    }

}

 

 

이렇게 BlackjackGame에 있던 로직을 옮겨왔고, BlackjackGame에게는 Participants를 생성하는 책임과 카드를 나눠주는 책임, 게임의 결과를 반환하는 책임을 부여하였습니다.

 

 

public class BlackjackGame {

    private final Participants participants;

    public BlackjackGame(final Names names, final List<Double> moneys) {
        participants = new Participants(names, moneys);
    }

    public BlackjackGame(final Participant dealer, final List<Participant> players) {
        participants = new Participants(dealer, players);
    }

    public void distributeCard() {
        participants.distributeCard();
    }

    public ParticipantResults makeParticipantResults() {
        return new ParticipantResults(participants.getDealer(), participants.getPlayers());
    }

    public Participants getParticipants() {
        return participants;
    }

}

 

 

 

 

네 번째 수정 사항은 "승/무/패" 결정을 Result에서 하는 것입니다. 하지만, Dealer와 Player에서 자기의 승무패를 정하지 않고 Result에서 정하려면 필요한 인자와 조건문이 많아질 것 같아서 수정 사항을 그대로 반영하지는 않았습니다.

 

대신, 승무패 로직에서 단순히 점수와 비교하는 로직은 Participant 메소드로 만들고 나머지 다른 부분은 Dealer와 Player가 각각 정하도록 설계하였습니다.

 

즉, Dealer와 Player는 블랙잭 조건이나 버스트 조건만 가지고 승패를 결정하고, 나머지 점수만 가지고 비교하는 로직은 상위 클래스인 Participant의 메소드를 활용하는 것입니다.

 

 

<Player>

    @Override
    public Result decideWinner(final Participant participant) {
        if (this.isBlackjack() && !participant.isBlackjack()) {
            return Result.BLACKJACK;
        }
        if (this.isBust()) {
            return Result.LOSE;
        }
        if (participant.isBust()) {
            return Result.WIN;
        }
        return decideWinnerWithScores(participant);
    }

 

 

<Dealer>

    @Override
    public Result decideWinner(final Participant player) {
        if (this.isBlackjack() && !player.isBlackjack()) {
            return Result.BLACKJACK;
        }
        if (player.isBust()) {
            return Result.WIN;
        }
        if (this.isBust()) {
            return Result.LOSE;
        }
        return decideWinnerWithScores(player);
    }

 

 

이렇게 블랙잭과 버스트 조건에 따라 승패 조건을 정의하고, 나머지는 decideWinnerWithScores() 메소드를 이용합니다.

 

 

    protected Result decideWinnerWithScores(final Participant participant) {
        final int score = this.calculate();
        final int opponentScore = participant.calculate();
        return Result.compareScore(score, opponentScore);
    }

 

 

인자를 통해 비교할 점수를 얻어오고, 이것은 Result로 책임을 넘겼습니다.

 

 

    public static Result compareScore(final int score, final int opponentScore) {
        if (score == opponentScore) {
            return Result.DRAW;
        }
        if (score > opponentScore) {
            return Result.WIN;
        }
        return Result.LOSE;
    }

 

 

다만, 위 방식대로 하더라도 테스트 코드 자체는 중복을 피할 수 없었습니다. 아무래도 딜러와 플레이어는 "버스트되는 순서"가 달라서 미묘한 차이는 있지만, 그 외에 나머지 코드는 동일하기 때문인 것 같습니다. 더 좋은 방법이 떠오른다면 추후에 수정을 해 보겠습니다.

 

 

 

 

다섯 번째 수정 사항은 독립적으로 실행되는 테스트 코드를 작성하는 것입니다. 기존의 제가 짠 CardDeck 코드부터 보여드리겠습니다.

 

 

public class CardDeck {

    private static final List<Card> cards = new ArrayList<>();

    static {
        for (final CardType type : CardType.values()) {
            Arrays.stream(CardNumber.values())
                .forEach(number -> cards.add(new Card(number, type)));
        }
        Collections.shuffle(cards);
    }

    private CardDeck() {
    }

    public static List<Card> getCards() {
        return Collections.unmodifiableList(cards);
    }

    public static Card distribute() {
        return cards.remove(0);
    }
}

 

 

위와 같이 card 리스트를 한 번만 초기화하고, 그 요소를 하나씩 빼 내는 방식으로 구현되어있습니다. 그리고 저는 'card 리스트의 사이즈가 52가 맞는지 확인', 'card를 한 장 뽑았을 때, card 리스트의 사이즈가 51인지 확인'과 같은 두 가지 테스트를 정의하였습니다.

 

이것은 하나씩 테스트를 실행하였을 때는 문제가 없었지만, 해당 CardDeckTest를 통째로 실행하니까 문제가 발생하였습니다. 실행된 순서를 보니까 '단순 사이즈 확인'이 아닌 '카드를 뽑은 후 사이즈 비교'가 먼저 실행되어서 '단순 사이즈 확인' 테스트를 할 때  52가 아니고 51이어서 테스트가 실패하였습니다.

 

즉, 테스트는 실행 환경에 따라 순서를 보장할 수 없기 때문에, 순서에 관계 없이 독립적으로 잘 작동하는 테스트를 구현하는 것이 중요합니다. 따라서, 아래와 같이 테스트 코드를 작성하는 것이 바람직합니다.

 

 

    @Test
    @DisplayName("카드덱이 정상적으로 카드를 분배하는지 확인")
    void distribute() {
        assertThat(CardDeck.getCards()).hasSize(52);
        assertThat(CardDeck.distribute()).isInstanceOf(Card.class);
        assertThat(CardDeck.getCards()).hasSize(51);
    }

 

 

굳이 카드 리스트를 처음 만들었을 때 size를 확인하지말고, 이 안에서 한 번에 수행하는 것이죠.

 

만약, 순서를 꼭 보장하고 싶다면 해당 테스트 클래스 위에 TestMethodOrder 어노테이션을 추가해 준 다음에 테스트 메소드 위에 Order 어노테이션을 추가하면 됩니다.

 

 

정리

이번 피드백을 통해서 특정 객체의 책임을 더욱 분산하는 경험을 하였습니다. 또한, 독립적인 테스트 코드의 중요성을 절실히 느낄 수 있었죠. 비록, 딜러와 플레이어가 승무패를 결정하는 로직이 미흡하지만 나머지 부분은 피드백을 잘 반영하였다고 생각합니다.

 

2차 피드백은 오는대로 수정해서 또 글을 올려보겠습니다.

댓글

추천 글