코로나 바이러스가 점점 심해지고 있어서 재택 교육이 계속해서 연장되고 있다.
그래서 이번 주는 페어 프로그래밍을 온라인으로 진행했다.
구글의 행아웃을 통해 화상으로 대화하고 각자 push & pull하는 방식으로 진행했다.
이번 주는
- 미션을 진행하면서 몰랐던 Java API 몇 가지를 페어에게서 배울 수 있었다.
- CQS원칙에 대해 공부하고 정리할 수 있었다.
- 객체 지향적인 설계, 유지보수하기 좋은 구조에 대해 깊은 고민을 할 수 있었다.
이번 미션은 프리코스때 해봤던 블랙잭게임 이다.
2단계에 걸쳐서 총 2주동안 진행되는 미션이다.
페어에게서 배운 점
Map<MatchResult, Long> result = playerResults.stream()
.map(UserResult::getReverseResult)
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
for (MatchResult key : MatchResult.values()) {
result.putIfAbsent(key, INIT_MATCH_COUNT);
}playerResults가 가지고 있는 MatchResult enum의 인스턴스를 Map<MatchResult, Long>에 각각 인스턴스 개수를 Long으로 나타내려고 했다.
나는 단순히 Collectors.toMap을 사용해야겠다 생각했는데 페어는 Collectors.groupingBy와 Collectors.counting을 통해 굉장히 간단하게 해결하였다.
나는 Collectors.groupingBy와 Function.identity, Collectors.counting에 대해 모르고있었던 것이다. 앞으로 Stream을 계속 사용하려면 깊이있는 학습을 통해 제대로 알고 사용해야겠다는 다짐을 했다.
또한 페어는 playerResults에서 가지고있지 않은 MatchResult의 인스턴스를 Map<MatchResult, Long>에서 get할 때 null이 리턴되는 것을 방지하기 위해
Map인터페이스의 putIfAbsent 메소드를 사용하였다.
평소에 Map인터페이스를 자주 사용하지만 putIfAbsent라는 메소드가 있는 것을 모르고 있었다. 자바를 주 언어로 사용하고 있는데도 Map인터페이스의 메소드들을 제대로 모르고있다는 사실에 부끄러웠다.
String에서 equals를 사용해서 상수와 일치하는지 비교할 때도 페어가 전에 받았던 리뷰 내용을 배울 수 있었다.
private static fianl String YES = "y";
public boolean isContinue(String text) {
text.equals(YES);
}위와 같이 String.equals(상수)를 하면 NullPointerException을 방지하기 위해서 String null체크를 해줘야 한다.
private static fianl String YES = "y";
public boolean isContinue(String text) {
YES.equals(text);
}하지만 상수.equals(String)을 해준다면 null체크를 하지 않아도 된다!
리뷰어분에게 배운 점
이번 블랙잭 1단계 미션은 리뷰가 2개 밖에 달리지 않았다. 처음엔 내 코드가 나름 괜찮아서 그런 줄 알았는데 그건 아니였다.
2개지만 생각할 것이 굉장히 많은 리뷰였고 구조를 많이 바꿔야하는 리뷰여서 반영하는데 제일 오랜 시간이 걸렸었다.
블랙잭 게임에서 유저가 블랙잭인 조건은 카드를 2장 가지고 있고 카드의 합이 21인 경우이다. 버스트인 조건은 카드의 합이 21을 초과한 경우이다.
public class Cards {
private static final int BLACKJACK_SCORE = 21;
private static final int BLACKJACK_CARD_SIZE = 2;
private static final int BUST_MIN_SCORE = 22;
private final List<Card> cards;
public boolean isBlackjack() {
return cards.size() == BLACKJACK_CARD_SIZE
&& sumScore() == BLACKJACK_SCORE
}
public boolean isBust() {
return sumScore() >= BUST_MIN_SCORE
}
...
}public class User {
private final Cards cards;
public boolean isBlackjack() {
return cards.isBlackjack();
}
public boolean isBust() {
return cards.isBust();
}
...
}나는 Cards 클래스에서 블랙잭인지, 버스트인지를 판단하고 User 클래스에서 Cards의 메소드를 호출하는 식으로 코드를 짰었다.
하지만 리뷰어분의 말대로 Cards에서 블랙잭의 룰에 의한 판단을 하는 것은 Card를 관리하는 Cards 클래스의 책임에서 벗어난다. 그리고 룰이 변경된다면 Cards에서 버스트인지, 블랙잭인지 판단하는 메소드들은 수정돼야한다.
수정될 가능성이 있는 코드들은 분리를 하는게 룰 변경에 더 유연한 설계이므로 RuleChecker라는 클래스에서 판단하도록 구조를 변경했다.
public class RuleChecker {
private static final int BLACKJACK_SCORE = 21;
private static final int BLACKJACK_CARD_SIZE = 2;
private static final int BUST_MIN_SCORE = 22;
public boolean isBlackjack(User user) {
return user.getCardsSize() == BLACKJACK_CARD_SIZE
&& user.calculateScore() == BLACKJACK_SCORE;
}
public boolean isBust(User user) {
return user.calculateScore() >= BUST_MIN_SCORE;
}
}
고민을 참 많이했던 리뷰였다. 어떻게 수정해야할지 오랜 시간 감을 못잡아서 리뷰어님께 DM을 보내기도 했었다.
private static boolean isPlayerWin(User player, User dealer) {
return (player.isBlackjack() && !dealer.isBlackjack()) ||
(!player.isBust() && ((player.calculateScore()
> dealer.calculateScore()) || dealer.isBust()));
}
private static boolean isPlayerDraw(User player, User dealer) {
return dealer.isBlackjack() && player.isBlackjack();
}
private static boolean isPlayerLose(User player, User dealer) {
return (!player.isBlackjack() && dealer.isBlackjack()) ||
(player.isBust()) || (player.calculateScore() <= dealer.calculateScore());
}기존에 MatchResult라는 enum에서 위와 같은 메소드로 승리, 패배, 무승부 여부를 판단해서 MatchResult의 인스턴스를 리턴하도록 작성했었다.
한 눈에봐도 위의 메소드들은 가독성이 떨어진다.
리뷰어님의 리뷰를 정리하자면 다음과 같다.
- 플레이어와 딜러의 최종 점수를 관리해라.
- 점수를 추출하는 클래스를 분리해라.
- 점수를 추출한 클래스로 승패를 결정하는 클래스를 분리해라.
내가 고민했던 부분은 User 클래스에 RuleChecker 클래스를 통해 버스트인지, 블랙잭인지 판단하는 기능이 이미 있는데 점수를 추출하는 클래스를 분리해서 점수를 비교하는 기능 이외에 무슨 기능을 가져야할까 였다.
User에서 반드시 블랙잭인지, 버스트인지 판단하는 기능을 가져야한다고 생각했기 때문에 점수를 비교하는 기능 하나만을 가진다면 굳이 분리를 해야할까? 하는 생각에 사로잡혀있었다.
하지만 실제 블랙잭 게임을 할 때도 User한테 블랙잭인지 버스트인지 물어보지도 않고 꼼꼼히 살펴보며 생각해보니 게임 진행에도 문제 없을 것 같았다.
오랜 고민 끝에 최종 점수를 관리하는 FinalScore 클래스에서
- 블랙잭인지 판단하는 기능
- 버스트인지 판단하는 기능
- FinalScore를 파라미터로 받아서 점수를 비교하는 기능
세가지 기능을 가지기로 했다.
플레이어가 딜러를 점수로 이기는 기준과 딜러가 플레이어를 점수로 이기는 기준이 달랐기 때문에 FinalScore를 추상 클래스로 만들고 점수를 비교하는 메소드만 추상 메소드로 만들었다.
그리고 DealerFinalScore와 PlayerFinalScore 에서 Override하는 구조로 만들었다.
public abstract class FinalScore {
protected final int score;
private final boolean isBust;
private final boolean isBlackjack;
public FinalScore(User user) {
Objects.requireNonNull(user);
RuleChecker ruleChecker = new RuleChecker();
this.score = user.calculateScore();
this.isBust = ruleChecker.isBust(user);
this.isBlackjack = ruleChecker.isBlackjack(user);
}
public boolean isBust() {
return isBust;
}
public boolean isBlackjack() {
return isBlackjack;
}
public boolean isBigger(FinalScore finalScore) {
return Objects.requireNonNull(finalScore).isSmaller(score);
}
public abstract boolean isSmaller(int score);
}public class DealerFinalScore extends FinalScore {
public DealerFinalScore(User user) {
super(user);
}
@Override
public boolean isSmaller(int score) {
return this.score < score;
}
}public class PlayerFinalScore extends FinalScore {
public PlayerFinalScore(User user) {
super(user);
}
@Override
public boolean isSmaller(int score) {
return this.score <= score;
}
}FinalScore 클래스로 승패를 판단하는 클래스는 Referee라는 정적 메소드를 가진 클래스로 만들어서 기존에 사용하던 enum에서 판단 기준으로 사용하였다.
public enum MatchResult {
WIN("승", Referee::isPlayerWin),
DRAW("무", Referee::isPlayerDraw),
LOSE("패", Referee::isPlayerLose);
private static final String NO_SUCH_MESSAGE = "찾을 수 없는 경우입니다.";
private final String matchResult;
private final BiPredicate<PlayerFinalScore, DealerFinalScore> resultCondition;
MatchResult(String matchResult, BiPredicate<PlayerFinalScore, DealerFinalScore> resultCondition) {
this.matchResult = matchResult;
this.resultCondition = resultCondition;
}
public static MatchResult findMatchResult(PlayerFinalScore playerScore, DealerFinalScore dealerScore) {
return Arrays.stream(values())
.filter(result -> result.resultCondition.test(playerScore, dealerScore))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException(NO_SUCH_MESSAGE));
}
...
}기존에 승패를 판단하는 로직이 가독성이 많이 떨어졌기 때문에 메소드를 여러개로 분리해서 가독성을 높이고자 노력했다.
public class Referee {
public static boolean isPlayerWin(PlayerFinalScore playerScore, DealerFinalScore dealerScore) {
return isPlayerOnlyBlackjack(playerScore, dealerScore)
|| isDealerOnlyBust(playerScore, dealerScore)
|| isPlayerScoreWin(playerScore, dealerScore);
}
public static boolean isPlayerDraw(PlayerFinalScore playerScore, DealerFinalScore dealerScore) {
return playerScore.isBlackjack() && dealerScore.isBlackjack();
}
public static boolean isPlayerLose(PlayerFinalScore playerScore, DealerFinalScore dealerScore) {
return isDealerOnlyBlackjack(playerScore, dealerScore)
|| playerScore.isBust()
|| isDealerWinScore(playerScore, dealerScore);
}
}위에 피드백에서 처럼 룰에 따라 변경될 수 있는 로직을 분리함으로써
더 유연한 설계가 되었음을 느꼈다. 또한 책임과 역할을 분리함으로써 클래스들의 역할이 더 분명해졌다.
다음 주 부터는 우테코의 교육장에 가서 그리웠던 크루들과 대화도 많이하고 교육장의 넘치는 열정을 느끼며 더 열심히 공부 할 것이다.
미션 외의 학습시간 비중을 조금 늘려야겠다!