[코드 리뷰 정리] Level 1. 블랙잭
2021-03-15글
Controller에 있는 도메인 로직
게임 진행을 위해 Controller에 많은 역할을 구현했었다.
위 피드백을 받고 BlackjackGame이라는 객체를 만들어 게임의 진행을 하도록 구현했다.
각각의 플레이어들의 게임을 진행에 Input과 Output이 연결되어 있어서
처음 리팩토링에서는 BlackjackGame에서 플레이어들을 꺼내오고, 플레이어 하나 하나마다 게임을 진행하게 되었다.
BlackjackController 일부
private void askWantToHit(Player player, BlackjackGame blackjackGame) {
while (isAbleToAskHit(player, blackjackGame) &&
Answer.of(InputView.inputDrawAnswer()).isYes()) {
blackjackGame.hit(player);
OutputView.printPlayerCards(player);
}
}
private boolean isAbleToAskHit(Player player, BlackjackGame blackjackGame) {
if (blackjackGame.isNotGameOver(player)) {
OutputView.printAskOneMoreCard(player);
return true;
}
return false;
}
하지만 결국 BlackjackGame에서 플레이어를 get
해와 값을 조회하고,
해당 값에 대한 로직이 Controller에서 처리되고 있었다.
게이츠의 말씀대로 도메인에 있어야 할 로직이 외부인
Controller에서 처리되고 있는 것이다.
페어인 다니도 비슷한 리뷰를 받았는데, BlackjackGame 객체를 통해 게임 진행 과정을 캡슐화하라 하셨고,
이에 대해서는 출력을 제외하고 getter가 사용되는 부분이 하나도 남아있지 않아야
캡슐화가 잘 진행되었다고 볼 수 있다고 하였다.
위 피드백들로 게임을 진행하는 역할을 완전히 BlackjackGame으로 옮기기 위해
현재 게임이 끝나지 않은 플레이어들을 BlackjackGame에서 관리하도록 하고
(getCurrentPlayer()
), BlackjackGame에서도 Players에게 현재 게임을
진행해야 하는 Player를 얻어오도록 구현했다.
내가 적용한 부분은 다음과 같다.
테스트 코드의 가독성
리뷰어님께서 테스트 코드의 가독성을 위해 given, when, then으로
줄바꿈 할 것을 권해주셨는데, 찾아보니 테스트 코드를 작성할 때 주로 사용하는
Given-When-Then Pattern을 발견하였다.
- Given
테스트를 위해 준비를 하는 과정
테스트에 사용하는 변수, 입력 값 등을 정의하거나, Mock 객체를 정의하는 구문 포함 - When
실제로 액션을 하는 테스트를 실행하는 과정 - Then
마지막은 테스트를 검증하는 과정
예상한 값, 실제 실행을 통해서 나온 값을 검증
사실 지금껏 테스트 코드를 짜는 것에만 집중을 하고 가독성은 고려하지 못했던 것 같다.
올바르게 적용한 것인지 확신은 없으나,
앞으로 테스트 코드를 작성할 때 위 패턴을 지키려 노력해야겠다.
추상 메서드로 공통 기능을 선언하고 각각 구현하기
기능 요구사항에서 초기 카드를 보여주는 로직이 딜러와 플레이어마다 달랐다. 딜러는 처음에 한장만 보여주고, 플레이어는 두장을 보여주어야 한다. 이를 처음 구현했을 때는 User 추상 클래스에 public final Cards getCards()
를 두고 플레이어는 초기에 이 메서드를 통해 카드를 2장인 Cards를 (사실은 전부) 반환하고, 딜러는 딜러만의 Card showOneCard()
를 통해 Card 객체를 반환한다. 반환 타입도 달라서 딜러와 플레이어들의 카드를 출력하기 위해서는 매개변수가 다른 출력 메서드들도 필요했다. 때문에 BlackjackGame 객체에서도 출력만을 위한 비슷한 get()
들이 생겨났다. 게이츠의 말씀대로 이렇게 추상 클래스인 User에 Cards showInitialCard();
를 만들고 딜러와 플레이어 각각이getCardsByCount(int count)
에 각자 맞는 개수로 카드를 가져올 수 있도록 리팩토링 해보았다.
캐시 적용
처음 로직 구현 때는 Deck 객체 자체를 static으로 만들어서 캐싱하는 방법으로 구현하였다.
하지만 static은 메모리 주소를 하나만 가지기 때문에, 해당 게임을 여러 스레드에서 진행한다면 카드 배분에 문제가 생길 것이다.
때문에 Deck을 그냥 인스턴스로 생성하게 하였는데, Deck에 들어갈 카드를 캐싱하는 부분을 생각하지 못했었다.
처음에는 LottoNumber 때를 생각하고 of로 객체 하나하나를 반환해주어야 하나 고민을 했는데, 그냥 List<Card>
를 미리 생성하면 되었다.
UI와 관련된 로직
BlackjackGame 객체에서 카드를 더 받기 원하는지에 대한 대답을 받아야 했는데, 스스로는 BlackjackGame 객체가 게임 진행을 담당하고 있으니, yes / no 에 대한 정보는 yes인지 no인지에 따라 게임 진행 여부가 다르니, 게임 객체가 알아야 한다고 생각했다. 이에 대해 내 생각을 전달하고 의견을 물어보았는데, "BlackjackGame 객체가 블랙잭 게임 역할을 담당하고 있으니 yes no에 대한 정보를 가지고 있어도 괜찮지만 yes, no에 대한 정보는 UI 요구사항이라 추후에 예, 아니오로 변경됏을 때 도메인 객체까지 영향 범위가 갈 것 같다"라고 말씀하셨다. View의 요구사항이 변경되었을 때 도메인 객체까지 영향이 갈 것 같다는 부분은 생각하지 못했던 것 같다. 이 피드백에서 많은 인사이트를 얻었는데, 체스 게임 미션에서 사용자의 대답을 입력 받는 부분에 있어서 대답에 대한 검증을 어디에서 취해줄까를 계속 토론하다 게임 객체에서 해주었었는데, 이 답변을 받고 도메인 밖으로 대답 검증을 뺄 수 있었다.
💾 미션 정리
상태 패턴 적용기
0309 강의에서 이번 미션에 상태 패턴을 적용하여 구현하는 법에 대해 배웠다.
이번 미션을 진행하면서 스스로 제일 이슈라고 생각했던 부분은
딜러와 플레이어의 점수만을 가지고 결과를 계산하는 것이 아니라,
딜러와 플레이어의 상태별로 결과를 계산하는 분기처리였다.
이 부분에 대한 처리를 이번 미션동안 많이 고민해보았었고,
나의 삽질의 과정들을 함께 기록해보려 한다.
1. if 문을 통한 처리
맨 처음 미션을 제출할 때는 각각의 상태에 따른 결과 산출을 if문을 통해서 처리해주었다.
/* 변수로 다음과 같이 compareValue를 가지고 있음
* private final String result;
* private final int compareValue;
*/
public static Result decide(Dealer dealer, Player player) {
if (dealer.cards.isBust() && !player.cards.isBust()) {
return WIN;
}
if (dealer.cards.isBust() && player.cards.isBust()) {
return STAND_OFF;
}
if (!dealer.cards.isBust() && player.cards.isBust()) {
return LOSE;
}
return Arrays.stream(values())
.filter(value -> value.compareValue == player.cards.compareTo(dealer.cards))
.findFirst()
.orElseThrow(IllegalArgumentException::new);
}
하지만 이렇게 구한 결과에 스스로 다음과 같은 문제점을 느꼈다.
- 딜러와 플레이어의 상태를 비교하는 동일한 행위를 if문으로 반복해서 처리하여 길어진
decide()
- 해당 타입을 구하기 위한 책임이 분리되어 있음 (데이터와 로직 분리되어 있음)
- compareValue인
1, 0, -1
값이 Result의 의미를 명확히 드러내지 못함 - 만약 실수로
decide()
의 if문 한줄을 지웠다면 프로그램 오류
때문에 if문으로 처리했던 상태들을 어떻게 줄일 수 있을까 많은 고민을 했다.
2. 함수형 인터페이스를 통한 처리
해결책을 찾아보다, 위 글들을 참고하여 딜러와 플레이어의 상태마다 다른 처리를
함수형 인터페이스를 사용해 구현해보았다.
이에 스스로 함수형 인터페이스를 학습하며 정리하고
Dealer에 대한 Player의 결과를 구하는 로직을 BiPredicate<T>
로 처리했다.
함수형 인터페이스를 사용하여 딜러, 플레이어의 상태별 / 점수별 결과 산출 코드는 다음과 같다.
public enum Result {
WIN("승", (playerNotBust, dealerNotBust) -> playerNotBust && !dealerNotBust,
(playerScore, dealerScore) -> playerScore > dealerScore),
STAND_OFF("무", (playerNotBust, dealerNotBust) -> !playerNotBust && !dealerNotBust,
Integer::equals),
LOSE("패", (playerNotBust, dealerNotBust) -> !playerNotBust && dealerNotBust,
(playerScore, dealerScore) -> playerScore < dealerScore);
private final String result;
private final BiPredicate<Boolean, Boolean> statusPredicate;
private final BiPredicate<Integer, Integer> scorePredicate;
// ...
public static Result decide(User player, User dealer) {
return Arrays.stream(values())
.filter(value -> value.statusPredicate.test(player.isAbleToHit(), dealer.isAbleToHit()))
.findFirst()
.orElseGet(() -> decideByScore(player.score(), dealer.score()));
}
private static Result decideByScore(int playerScore, int dealerScore) {
return Arrays.stream(values())
.filter(value -> value.scorePredicate.test(playerScore, dealerScore))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("승패 결과 조건에 매치되지 않습니다."));
}
// ...
이렇게 Enum에 함수형 인터페이스를 사용하여 결과 산출을 했을 때,
스스로 다음과 같은 장점을 느꼈다.
- 동일한 기능(딜러와 플레이어의 상태를 비교)에 대해 각각 다른 연산을 가지고 있어
늘어졌던decide()
단순화 - (현 프로그램에서는 그럴일이 없겠지만) 새로운 Result 타입이 추가되어도,
메서드의 추가적 수정이 없다. - 타입이 해야 하는 기능에 대해서 가장 잘 알고 있을 수밖에 없는,
자신 안에 로직을 추가하면서, 로직에 대한 명확한 상수명을 가짐
이렇게 함수형 인터페이스를 사용하면서 이를 사용했을 경우의 장점을 스스로 생각하고,
리뷰어님께 질문을 남겼는데 추가적으로 의견을 달아주셨다 👀
상태 패턴
미션 2단계를 시작하면서, 베팅 금액을 입력 받고 상태에 따라 수익을 구하는 기능을 추가해야했다.
미션을 시작하기 앞서,
0309 블랙잭 피드백 강의에서 다룬 상태패턴을 적용해 여러 분기처리를 해결하고 싶었고,
이후 스스로 상태 패턴에 대해 더 찾아보고 이번 미션을 통해
상태패턴이 무엇이고, 이를 적용하면 어떠한 장점이 있는지 알아보자!를 목표로
상태패턴 적용길을 걸었다.
내가 적용한 상태 패턴
제이슨 코드를 미리 보고 상태패턴을 시도했기 때문에, 스스로 생각해보지 못한 코드 따라쟁이가 되어버릴까봐 스스로 계속해서 상태별로 다르게 구현해야할 기능들에 대하여 생각해보고 클래스 다이어그램에 나름대로의 명분을 정리해보았다.
또한 아래는 상태 패턴의 콘텍스트인 User 클래스이다.
public abstract class User {
protected final Name name;
protected State state;
protected Money bettingMoney;
public User(String name) {
this(new Name(name));
}
public User(Name name) {
this.name = name;
}
public final void initializeCards(Cards cards) {
state = StateFactory.generateStateByCards(cards);
}
public final boolean isAbleToHit() {
return !state.isFinish();
}
public final Score score() {
return state.cards().totalScore();
}
public final Cards cards() {
return state.cards();
}
public final String getName() {
return name.getName();
}
public State getState() {
return state;
}
private void changeState(State state) {
this.state = state;
}
public final void hit(Card card) {
changeState(state.draw(card));
}
public final void stay() {
changeState(state.stay());
}
// ...
콘텍스트의 필드로 상태(State)를 가지고 있으며,
상태에 관련된 기능들을 그 상태에게 메시지를 보내 처리하도록 하였다.
PS
위 말은 교육장에서 포비와 이야기를 나누다 인상 깊어 스스로 DM에 남겨놓은 말이다.
말씀대로 디자인 패턴이 무조건 좋은 것은 아니지만,
개인적으로는 이번 미션동안 제일 고민했던 부분인 상태에 따른 여러가지 분기처리를
어떻게하면 효과적으로 구현할 수 있을까? 에 대한 좋은 답이 상태패턴이라 생각하여
상태 패턴을 사용하는 방법과 이점을 느껴보고 싶었기에 스스로 좋은 시도였다고 생각한다 !
물론 다른 미션에서 상태패턴을 적용하라하면 잘 적용할 수 있을지는 모르겠으나,
이번 미션을 통해 이렇게 상태에 따른 분기처리를 줄일 수 있구나,
디자인 패턴은 이런 장점이 있구나를 느낄 수 있었다.