[1단계 - 블랙잭] 돌범(조재성) 미션 제출합니다. #295
Conversation
- 카드 받는 기준인 gambler의 burst아님(20이하)와 dealer의 (16이하)을 같은 메서드명 isNotFinished()로 추상화함.
- hasAce -> 갯수까지 파악한다음, 갯수 사라질때까지 보정 가능하도록 수정 - hasOne -> 삭제 - change ACE TO ONE / ONE TO ACE 메서드 삭제
- burst시 peekcard로 가상 확인 불필요
- burst확인시 card를 일단 뽑아서 주고, 21 넘으면 버스트로 Enum에서 처리
- isBurst()시 peek + !isBurst시 addCard했던 로직 간소화
- 결과Enum BiPredicate 추가 로직(burst) 대한 테스트 추가
- 카드 합 대소비교 전 마다 burst 여부 먼저 확인하도록
- dealer의 경우, 반복되는 isFinished가 다른 로직(16이하)
- 추가적으로 다 끝나고 burst를 확인하여 print해주는 기능 추가
- PlayDto에서 score필드 및 getScore필드 추가
- ace보정된 점수를 미리 계산하여 가지고 있어야 dto에 계산 로직이 추가 안된다.
- dealer의 결과 순서가 보존되도록, groupingBy로 생성하는 map 타입을 EnumMap으로 지정함.
|
피드백 해주신 것을 바탕으로 먼저 수정해보았습니다. 크게 달라진 점은 01 재귀로직을 while문에서 2번 물어서 처리
02 ACE를 ONE으로 써야할 경우, 객체를 교환하지 않고 ace갯수만큼 보정가능하게 확인만 함03 카드덱에 전략패턴 적용하여 수동 생성 가능04 카드덱에서 peek해서 가상의 결과 확인 -> pop으로만 진행하도록 함05 burst된 상태의 카드들을 그대로 들고 있다가 -> 결과객체응답 or 결과출력시 burst된 카드(21보다 큰지)를 들고 있는지 확인하여 처리하도록 함.06 PlayDto에 score필드를 추가하여, ace보정계산된 합계를 가지게 함.
07 & Q1 CardDeck을 싱글톤으로 생성해놓은 뒤, 전략메서드 호출시 stack을 copy만해서 stack을 반환하도록 함.
Q2. dealer의 burst를 확인하는 행위를 추가하고 싶습니다.
private void checkDealerBurst(final Player dealer) {
if (dealer.getSumOfCards() > BURST_CRITERIA) {
outputView.printBurst(PlayerDto.from(dealer));
}
}Q3. 딜러의 경우 16이하이라면 반복해서 무조건 받도록 했는데, burst적용후 패배 확률이 큰 것 같습니다. dealer도 입력을 받도록 해야할지 고민입니다! |
- 그에 대한 테스트도 추가
|
처음에 만들어 두고 같은 cardDeck을 재활용하고 싶으신것이였군요. dealer뿐만이 아니라 gambler도 burst인지 확인하는 것이 필요할텐데요. 기본적인 요구 사항이 16이하면 받도록 하는 것이기 때문에 요구사항을 지키는 것이 맞습니다. (실제 블랙잭 게임의 룰도 같습니다.) |
src/main/java/blackjack/domain/cardGenerator/RandomCardGenerator.java
Outdated
Show resolved
Hide resolved
| public enum Denomination { | ||
|
|
||
| ACE("A", 11), | ||
| ONE("A", 1), |
There was a problem hiding this comment.
ACE <-> ONE 객체 교환 로직을 없앴을 때, 같이 없앴어야하는데 놓쳤습니다!
혹시 사용되지 않는 enum에 대해서 알려주는 플러그인이나 체크방법이 따로 있는지도 궁금합니다!
코드 삭제시 사용된 객체들을 먼저 잘 확인하도록 하겠습니다!
There was a problem hiding this comment.
command+b와 로직을 보면서 사용하는 곳이 없어보이는거 같아서 말씀드렸습니다~
| } | ||
|
|
||
| public void spreadCards(List<Player> gamblers, Player dealer, CardDeck cardDeck) { | ||
| IntStream.range(DEFAULT_SPREAD_COUNT_START_INDEX, DEFAULT_SPREAD_COUNT_END_INDEX) |
There was a problem hiding this comment.
for문이 아닌 IntStream을 쓴 이유가 있을까요??
그리고 이 부분은 게임의 중요로직인데 첫 분배에서 2개가 나누어졌는지 확인하는 테스트가 필요하지 않을까요??
그리고 접근제어자가 public인데 이유가 있을까요??
There was a problem hiding this comment.
-
해당 구문은 for문으로 수정하였습니다. IntStream은 페어간에 stream으로 가독성 좋게 작성하려고 했었던 것 같습니다. 불필요하다고 생각해서 삭제했습니다.
-
또한 테스트는 바로 아래
printSpreadCards()로 출력을 해주는 상황이므로 작성하지 않았던 것 같습니다.! 그러나, 혼자 개발한다고 가정해보면 ouputView는 가장 나중에 개발할 것이고 테스트 과정이 필요할 것 같아 작성하였습니다!
Q. protected로 선언해놓고 확인해도 될까요??
로운은 private메서드를 어떻게 테스트 하시는지 궁금합니다!
- 그 동안 저는 생성자의 경우 public으로 열어두어 편하게 테스트
- 그외의 경우는 private이지만, 먼저 publiic 작성 테스트 -> 스스로 만족하면 private으로 변경
- 이런 방식으로 갔던 것 같습니다!
There was a problem hiding this comment.
테스트를 위해 protected로 선언하지는 않는것 같고요. 다른 곳에서 사용하지 않으면 항상 private로 막아두는 것 같아요.
private에 테스트가 필요한 경우에는 구조를 변경해서 테스트 가능한 구조가 없는지 생각해보는 것 같고 아니면 private을 사용하는 public 메서드를 테스트 함으로써 private메서드도 정상적으로 동작했다는 것을 검증하는 것 같아요.
위와 같이 view가 함께 있는 구조라면 테스트가 힘들 거에요. 처음에는 public으로 되어 있어서 말씀을 드렸고, 다른 방법이 없을까 고민해보시라고 코멘트 드렸었습니다~
이 경우에는 cardDeck의 drawTo를 테스트해서 기본적인 검증만 하는 것으로 마무리해도 될거 같아요.
- public -> protected로 변경후 해당 메서드에 대한 테스트 코드 추가
- isFinished(final CardDeck cardDeck) - 드합산후 더 진행할지에 대한 카드합산 기준(16초과 /21초과)만 다름 - 추상메서드에서 -> 일반 공통메서드(limitedCardSum 파라미터 추가)로 변경함. - isFinished(final CardDeck cardDeck, final int limitedCardSum)
- isFinish는 추상메서드화 -
- isFinish -> isNotFinish로 메서드명 변경 - isBurst(burst 확인후 출력)을 isNotFinished(카드 받을 수 있는지 확인)보다 먼저 확인함
controller에 List와 dealer가 따로 존재했으나 업캐스팅된 List로 묶어서 Players로 포장하였습니다.-> playGame시 로직이 다른 관계로 isinstance와 유사한 메서드(isDealer)를 정의하고 필요한 곳에서는 if분기 대신 내부에서 필터링한 getter를 사용할 수 밖에 없었습니다. 아직은 잘 모르겠습니다.ㅠ private void playGame(final Players players, final CardDeck cardDeck) {
playGameForGambler(players.getGamblers(), cardDeck);
playGameForDealer(players.getDealer(), cardDeck);
}isBurst와 isFinished 로직을 분리하고 Player에 정의하여 dealer, gambler 각각 사용할 수 있도록 하였습니다.
테스트가 필요한 메서드에 대해 접근제한자를 protected로 변경하였다.-> Q1. 테스트가 필요한 중요로직(ex> spreadCards()후 2장씩 가지고 있나?)는 proteced로 사용해도 될까요? protected void spreadCards(final Players players, final CardDeck cardDeck) {
for (int i = DEFAULT_SPREAD_COUNT_START_INDEX; i < DEFAULT_SPREAD_COUNT_END_INDEX; i++) {
spreadCard(players, cardDeck);
}
printSpreadCards(players);
}그외 페어의 피드백에 따라 자료구조 수정하였습니다.
CardDeck 자료구조 변경하였습니다.
|
|
https://tecoble.techcourse.co.kr/post/2021-04-26-instanceof/ 위에 답변 달았습니다~ 개인적인 질문인데요. |
하이! 로운! 저는 돌범이라고 합니다!
아직 전 단계 미션 로또를 이리저리 TDD해보고 쪼개보면서 공부하고 있는 와중에, 블랙잭 미션을 받아 후반부가 저에겐 어려웠습니다. 제한된 시간내에 구현하였지만 아직 java 숙달도가 낮아 다른 크루들에 비해 코드가 부족할 수 있습니다.!
페어시 최대한 작년 선배님들이나 작성된 코드를 안보고 해보기를 시도하다가 갈라졌는데, 막상 후반부로 가니 raw한 코딩에 테스트도 제대로 완성 못한 체 마감시간에 맞추어 완성했습니다.
주말을 이용해서 가다듬어 보겠습니다.
CardDeck이 뽑을때마다 사라지는 stack자료구조이면, 싱글톤으로 만들어놓지 못할까??
map자료구조로 게임 결과를 담는
BlackJackResult를 다시 List로 뿌리도록 DTO를 만들었는데, 이렇게 사용해도 될까?재귀를 이용한 코드
while hit면 카드를 받아가는데, if 받아가기 전 버스트에 걸리면 빠져나와라의 코드가 기본 depth가 2인데, 이것을 재귀말고는 구현하기가 힘들었습니다. 제한된 시간내에서 고민을 많이 했는데, 어떤 방법이 또 있을까요?? 주말간 고민해보겠습니다!추상메서드 1개로 [딜러-17이상이면 stop] == [겜블러-버스트로서 stop]를 같은 메서드명 isFinishfe()로 구현해도될까?
isFinish()를 각 딜러에는 17이상이면 아웃 / 겜블러들에게는 22이상 burst라서 아웃 를 각각 구현해서 사용했는데, 이러한 사용이 맞을까 궁금합니다!ACE처리
ace가 존재 && 그 합이 블랙잭을 넘어가버리면 one카드로 갈아치우자. 의 방식으로 일단 시도했습니다.gambler들이 burst일 경우, Denomination에 burst를 추가하여 0점 처리
이후, 작년 선배들 및 크루 코드를 살펴보면서 더 나은 방향으로 리팩토링 해보겠습니다~!