피드백) 1-3(블랙잭) 코드리뷰

📜 제목으로 보기

PR
블랙잭 1단계 피드백
블랙잭 2단계(베팅) 피드백
- 구초리
페어간 새롭게 안 사실들

PR

블랙잭 1단계 피드백

domain package내에서도 쌓인 class들을 용도에 package를 구분해라.
큰규모가 아니면, Contoller를 쓰지말고 Game/Program/Machine을 run하자.
자식class에서 상속받은 메서드에 대해 @Override는 강제가아니지만 표기하자
확인할 때는, 실제로 계산로직과 별개로, 확인만 하는 메서드를 만들자
- 예를 들어, isBust()를 확인할 때, 카드를 직접 받아서 Bust를 만들어보고 rollback이 아니라 카드를 받아서 계산하는 실제 로직과 비슷하더라도 로직을 포함해 물어보자
getXXX는 getter가 아니면 다른 용어를 쓰자. ex> .receiveXXXX()
블랙잭-CardDeck 요소는 정해져 있지만 셔플로 랜덤이 생기는 경우도 전략패턴으로 외부주입 가능하게 해서 테스트
- 자동차경주: 랜덤한 조건식을 테스트를 위해 전략패턴으로 빼서 외부주입
- 로또: 랜덤로또생성을 테스트하기 위해 전략패턴으로 빼서 외부주입
- 블랙잭: 요소들은 정해져있지만 셔플된 CardDeck을 테스트하기 위해 전략패턴으로 빼서 외부주입
- 체스: (예정) 요소들은 정해져있지만 너무 규모가 커서 테스트하기 힘든 map을 작은 Map.of(,)으로 만들어서 테스트하기 위해 전략패턴으로 빼서 외부 주입
java에서는 Stack대신 Deque자료구조를 사용
Enum도 결과 계산로직 등 중요로직을 가지고 있으면 EnumTest하기
list와 단일요소를 합칠 때는 .stream() + Stream.of() 으로 stream으로 통일한 뒤, concat하여 묶음을 만들 수 도 있다. 다만 묶은 Stream을 forEach를 사용할 경우는 생각해봐야한다.

my) 이미 정해진 자료를 db가 아닌 프로그램에서 초기화하려면,

프로그램 시작부터 초기화는 상수static final변수에 -> static { }블럭에서 초기화시키면 된다.

public class RandomCardGenerator implements CardGenerator {
    static final List<PlayingCard> cardDeck;
    
    static {
        cardDeck = Arrays.stream(Suit.values())
            .flatMap(RandomCardGenerator::getPlayingCardStream)
            .collect(Collectors.toList());
    }

초기화되어있는 컬렉션을 Collections.shuflle()만 해줘도 랜덤이다.
- 원본을 보호하고 싶다면, new ArrayDeque<> ( 컬랙션 )처럼 복사후 shuffle하고 return해준다?!

입력받은 값을 확인하는 메서드 내에서 바로 t/f return이 아닌, return전 또다시 물어본 대답에 따라 로직을 수행후 return t/f return하는 경우도 있다.** -> **isXXX Then YYY ()`

만약 카드받는 로직이 없이 물어보기만 했다면?

  private boolean isHit(final Player gambler) {
      final BlackJackCommand inputCommand = askHitOrStay(gambler);
      return inputCommand.isHit();
  }

물어본 대답에 따라 로직이 추가되는 경우 -> xxxThenyyy()

  private boolean isHitThenReceiveCard(final Player gambler, final CardDeck cardDeck) {
      final BlackJackCommand inputCommand = askHitOrStay(gambler);
      if (inputCommand.isHit()) {
          gambler.receiveCard(cardDeck); // isHit인 경우, 추가로직 수행후, return한다.
          return true;
      }
      return false;
  }
        

블랙잭 2단계(베팅) 피드백

같은 상태값을 공유하는 테스트들만 따로 @Nested를 적용해줘도 된다.
- 예를 들어, setUp에 버스트의 상황이 없는 경우에 한하여테스를 작성할 때
전략패턴으로 빼고 랜덤까지만 production의 domain 패키지에, 테스트를 위한 수동전략객체Class는 test코드> testutil> XXXXFixtureGenerator로 만들어준다.
- 전략객체라도 테스트를 위한 수동전략객체는 -> production에 생성하면안된다.
1. getter가 아니라면 -> .receiveXXX() or .createXXXX()
2. 조건이 여러개 있을 땐, 포함관계인지 확인하자

객체에 역할수행을 위해 메세지를 보낼 때, 역할수행에 필요한 재료들만를 보내야지, 역할을 가진 도메인을 보내지마라

역할을 가진 cardDeck을, 역할을 가진 players에게 보낸다?? -> (X) -> (O)

내부에서 cardDeck이 역할을 수행해야하는데, 다른 도메인내부에서 다른 도메인이 역할하게 하지말고 재료만 보내자.
만약, players에 있는 복수의 player에게 서로 다른 재료를 제공하려면, 복수의 재료를 제공해야하고, 매번 1개씩 재료를 제공해준다면, 재료를 1개씩 제공하는 역할의 도메인은 넘겨도 된다.
중요한 것은, 메세지를 받는 players, player를, 다시 내부에서 재료로 들어간 cardDeck 역할 수행시, 그 재료로 쓰면 안된다.

  private void spreadCards(final Players players, final CardDeck cardDeck) {
      // 변경 전:
      //players.receiveCard(cardDeck);
        
      // 중간 변경:
      // 컬렉션(복수)에 넘겨주는 재료는 <-- 1개만 주면 안된다.
      //players.receiveCard(cardDeck.pop());
        
      // 최종 변경:
      // 컬렉션(복수)에 메세지 던질 때는, 재료컬렉션(복수)나, 재료를 매번 제공해주는 역할의 도메인을 넘겨도 된다.
      // 복수 <-- 재료도 복수 or 내부에서 복수재료를 만드는 역할은 수행해도 된다.
      players.receiveCard(cardDeck);
  }

  public void receiveCard(final CardDeck cardDeck) {
      for (Player player : value) {
          //cardDeck.drawTo(player, DEFAULT_DRAW_COUNT);
        
          // player들을 돌면서, 매번 새로운 재료를 공급받는다.
          final PlayingCard firstCard = cardDeck.pop();
          player.receiveCard(firstCard);
        
          // 2장씩 제공받는다. -> 2번이라서 반복문 굳이 안써도 된다??
          final PlayingCard secondCard = cardDeck.pop();
          player.receiveCard(secondCard);
      }
  }

상한이 있는 경우, 점수보정은 작은 것으로 출발하는 것이 좋다.
- blackjack의 ace를 1or11 중 큰 점수를 유지하고 싶어서 default 11로 하고, -10점씩 보정시켜줬지만
- 애초에 상한 점수(21)점이 정해져있으면, ace 3장을 가져도 33으로의 보정은 아예 불가능한 상황이기 때문이다.
  - ace2장만 있다고 치더라도, 22점은 아예 불가능한 점수기 때문에, ace를 작은 점수 1을 default로 하고, 보정도 22를 못넘어도록 1번만 +10보정이 가능하므로 while문을 돌릴 필요도 없다

구초리

domain과 view(m<->v)분리할 수없다면, mvc패턴이 아니므로 컨트롤러를 쓰지마라
inputView에도
1. input메서드() 전에 출력로직이 있어야한다.
2. .scanXXX() 애매 -> 탐색/검색의 의미임. -> .inputXXX()로 가자,
3. view에서 형변환까지 완료해서 return 해보면 어떻까? 고민
view로직이 조금이라도 포함된다면? domain패키지에서 빼라
지역변수 추출을 활용해서 가독성을 높이자
scanAndGetBetMoney : controller에서 내부 2가지 하는 일(로직 + 출력)을 하도록 묶은 메서드에 대해
1. 메서드명에 내부 여러가지 일을 명시하여 노출하면 캡슐화 위배
2. controller(M+V가 섞인 class)에서 메인로직 + View출력을 1개 함수로 묶지마라
controller or M+V메인로직에서 반복문 등 로직을 수행하지마라 -> 로직을 담당하는 클래스에 메세지를 보내 책임을 넘겨라
- 메인로직 클래스가 일이 많아지거나 로직을 수행하게되면 객체에게 넘겨라
scanAndGetBetMoney ->
- 내부 .scanXXXX() -> 의미상 inputXXXX()으로 변경
- scanAnGetBetMoney -> createBetMoney로 변경
  - get -> getter아니면 receive(다른데서 받기) or create(생성)
A && B && C -> 드모르간으로 단순화시킬 수 있다.
- not A return false;
- not B return false;
- (자동 A && B) return C

스트림의 결과를 변수로 추출하라

 //변경 전
 public static BlackJackResult from(final Players players) {
     return new BlackJackResult(players.getGamblers()
                                .stream()
                                .collect(Collectors.toMap(
                                    Function.identity(),
                                    gambler -> calculateProfit(players, gambler),
                                    (x, y) -> y,
                                    LinkedHashMap::new)
                                        ));
        
 //변수로 추출
     public static BlackJackResult from(final Players players) {
         final Map<Player, Double> ProfitsPerPlayer = players.getGamblers()
             .stream()
             .collect(toMap(Function.identity(),
                            gambler -> calculateProfit(players, gambler),
                            (x, y) -> y,
                            LinkedHashMap::new));
    
         return new BlackJackResult(ProfitsPerPlayer);
     }

 //변경 전
 public double calculateDealerProfit() {
     return this.value.values()
         .stream()
         .mapToDouble(Double::valueOf)
         .sum() * DEALER_FLIP_UNIT;
 }
    
 //변수로 추출
 public double calculateDealerProfit() {
     final double dealerTotalProfit = this.value.values()
         .stream()
         .mapToDouble(Double::valueOf)
         .sum();
     return dealerTotalProfit * DEALER_FLIP_UNIT;
 }
    

dto -> 필요case : 시스템커졌을때, 레이어 나눠서 (도->서) 관리, 계층넘어갈때 문제

페어간 새롭게 안 사실들

추상클래스를 사용하는 많은 경우가 인페에서는 공통적으로 못쓰는 공통 상태값을 공유하기 위해서

여러 구상체(자식들) 존재시 변수는 무조건 추상체로 받아 -> 컬렉션<추상체>로 묶어서 처리가 가능

변수, 파라미터, 응답값 모두 추상체로 주기

  // given
  String expected = "pobi";
  final Player pobi = new Gambler(expected);
        
  final Player rich = new Dealer("rich");

만약 단수로 구현체로 받는 경우는 , 추상체로 묶어 사용할 것을 포기한 상태로, 구현체 전용 메서드를 사용하는 순간이다.

  @Test
  @DisplayName("딜러는 현재 합산 17이상인지 확인할 수 있다")
  void over_than_sixteen() {
      // given
      final Dealer rich = new Dealer("rich");
      rich.addCard(PlayingCard.of(Suit.HEARTS, Denomination.KING));
      rich.addCard(PlayingCard.of(Suit.SPADES, Denomination.KING));
        
      // when
      boolean actual = rich.isUnderSixteen();
        
      // then
      assertThat(actual).isFalse();
  }

값 비교시, null가능한 값은 메서드 파라미터로, 확실한 값만 좌항으로 + 소문자 가능하려면 equalsIgnoreCase
- 비교시 확실한 값을 왼쪽에 넣어서 null에러 안나게 + ignoreCase
싱글톤
- static + final으로 클래스변수로서 애초에 생성해놓고 초기화도해놓음
- 정펙매로 이미 생성된 것만 반환함 ( 객체 생성 생성자 제공 안함. getInstance로만 사용함.)
  - my) 프로그램 시작시부터 생성되어야할 놈들
    - static +final 상수 변수에 담아야함
    - static { } 블럭에 초기화 - > 프로그램 작동중에 다시 초기화할 때는?
    - 싱글톤 -> new 기본생성자 제공도 안함.?!
디버깅 찍고 싶은데, 아래 아무것도 없으면 sout만들어놓고, 그 위에 디버깅 표시
도메인1 vs 도메인2 비교계산 로직이 같은 객체/일급컬렉션이 아닐 때의 비교 -> 결과도메인이 1,2를 각각알고 서로는 모르게
1. my) 같은형의 일급컬렉션 or 객체면, 상관없이 그냥 메세지> 도메인1.메세지( 도메인2 )
2. 서로 전혀 다른 도메인인데, 각 도메인에서 최종결과값만 꺼내서 비교될 예정이라면? -> 제3의 도메인을 만들어서 비교
  - dealer vs players 점수비교하여 승패비교
    - dealer. compare ( players )?
    - players . compare ( dealer)?
    - Result.of ( dealer, players )
  - 로또 비교도 마찬가지..?
    - LottoResult.of( winningLotto, Lottos )