[2단계 - 블랙잭 베팅] 가이온(한승연) 미션 제출합니다 #872
Conversation
| @Deprecated | ||
| public Map<GameResult, Integer> calculateDealerGameResult() { |
There was a problem hiding this comment.
더이상 사용하지 않으면 지워도 되지 않을까요? @Deprecated 를 붙이고 메서드를 남겨놓은 이유가 있을까요?
There was a problem hiding this comment.
더 이상 사용하지 않아도 남겨두고 싶어서 Deprecated 어노테이션을 사용하였는데, 사용하지 않으면 지워버리는 것이 적절한가요?
| import java.util.stream.Collectors; | ||
|
|
||
| public class BlackJackBetCalculator { | ||
| private Map<String, Bet> playersBet = new HashMap<>(); |
There was a problem hiding this comment.
| private Map<String, Bet> playersBet = new HashMap<>(); | |
| private Map<String, Bet> playersBet |
생성자에서 초기화 해주니 필드 할당은 불필요하네요
There was a problem hiding this comment.
헉... 생성자에서 초기화하므로 필드 할당을 제거하겠습니다.
src/main/java/domain/bet/Bet.java
Outdated
| private static final int BLACKJACK_PRIZE_RATE = 15; | ||
| private static final int DRAW_BET_PRIZE = 0; | ||
|
|
||
| int bettingCount; |
There was a problem hiding this comment.
접근 제어자, final 키워드가 Bet 클래스에서만 안붙어있는데, 이유가 있나요?
There was a problem hiding this comment.
private를 붙이지 않은 것은 실수입니다..! 아직 final을 사용하는 습관이 자리 잡히지 않았는데, 항상 final을 붙이고 꼭 필요한 경우에만 final을 제거하는 방식으로 하면 될까요?
src/main/java/view/InputView.java
Outdated
| throw new IllegalArgumentException("[ERROR] 배팅 금액이 올바르지 않습니다."); | ||
| } | ||
| } | ||
|
|
||
| private String readInput() { | ||
| final Scanner scanner = new Scanner(System.in); |
There was a problem hiding this comment.
Scanner 를 매번 생성하는 건 비효율적으로 보여요. 하나만 생성해놓고 재사용해볼까요?
| return LoopTemplate.tryCatchLoop(() -> | ||
| LoopTemplate.tryCatchLoop(inputView::readPlayers)); |
There was a problem hiding this comment.
| return LoopTemplate.tryCatchLoop(() -> | |
| LoopTemplate.tryCatchLoop(inputView::readPlayers)); | |
| return LoopTemplate.tryCatchLoop(inputView::readPlayers); |
tryCatchLoop 을 한번 더 감싸줄 필요가 있나요?
There was a problem hiding this comment.
한번만 해도 돠네요..!
public static <T> T tryCatchLoop(Supplier<T> callback) {
try {
return callback.get();
} catch (IllegalArgumentException | IllegalStateException e) {
System.out.println(e.getMessage());
return tryCatchLoop(callback);
}
} public static <T> T tryCatchLoop(Supplier<T> callback) {
while(true){
try{
return callback.get();
}
catch (IllegalArgumentException | IllegalStateException e){
System.out.println(e.getMessage());
}
}
}indent 1을 유지하기 위해 첫 번째 코드처럼 재귀를 사용했지만, 재귀 호출마다 불필요한 메모리 할당이 발생할 수 있습니다. 따라서 indent 1을 어기더라도 두 번째 코드처럼 반복문을 사용하는 것이 더 적절하다고 생각됩니다. 꼭 indent 1을 지켜야 하는지 궁금합니다.
| gamer.receiveCard(card); | ||
| } | ||
|
|
||
| public boolean isBlackJack() { |
There was a problem hiding this comment.
딜러가 블랙잭인 경우 1.5배로 배팅한 금액을 빼앗아가는 것으로 착각하여 메서드를 만들어두었다가 사용하지 않은것으로 보입니다...
src/main/java/domain/bet/Bet.java
Outdated
| int bettingCount; | ||
|
|
||
| public Bet(int bettingCount) { | ||
| this.bettingCount = bettingCount; |
There was a problem hiding this comment.
count 라는 변수명을 보고는 베팅한 금액, 즉 돈이라는 걸 한번에 알아보기 어려울 것 같아요
There was a problem hiding this comment.
네 bettingCount는 배팅한 금액인 것을 알아차리기 힘든 것 같습니다. bettingAmount로 수정하였습니다.
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class Bet { |
There was a problem hiding this comment.
베팅한 금액의 범위와 같은 유효성 검증이 도메인인 Bet에 들어있으면 좋을 것 같아요 :)
There was a problem hiding this comment.
넵 수정하였습니다.
private void validateBetAmount(String betAmountInput){
try{
Integer.parseInt(betAmountInput);
}catch (NumberFormatException e){
throw new IllegalArgumentException("[ERROR] 배팅 금액은 숫자만 가능합니다.");
}
}숫자인지 판단하는 부분도 Bet에서 해야하는 역활인지 InputView에서 하는것이 적절한 부분인지 궁금합니다. 단순히 숫자인지 판단하는 예외이므로 저는 View에서 처리하는게 맞다 생각하는데 어떤가요?
| calculator = new BlackJackBetCalculator(betMap); | ||
| } | ||
|
|
||
| @DisplayName("플레이어가 딜러를 상대로 승리한 경우 배팅한 금액만큼 얻는다") |
There was a problem hiding this comment.
1차에서 세부적인 로직말고 추상적인 레벨로 테스트를 작성하라는 피드백이 있었는데, 2단계에서 테스트 코드가 추상적으로 잘 작성되었는지 궁금합니다.
아래에 추가적으로 세부사항에 대한 테스트케이스를 작성해주셨지만, 구체적인 금액 등이 언급되지 않은 추상적인 방향의 테스트 케이스를 작성해주셔서 잘 작성되었다고 느꼈습니다ㅎㅎ
There was a problem hiding this comment.
@DisplayName("플레이어가 1000원과 2000원을 베팅하면 딜러는 3000원을 얻는다")
@DisplayName("플레이어가 1000원과 2000원을 베팅하고 딜러가 승리하면 딜러는 3000원을 얻는다")이렇게 구체적인 금액 보다는
@DisplayName("플레이어들이 패배하는 경우 딜러는 배팅한 금액을 얻는다.")
@DisplayName("플래이이가 승리하는 경우 딜러는 플레이어가 배팅한 만큼 잃는다.")구체적인 부분이 드러나지 않아야 한다는 거군요!
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
객체 책임분리를 고려하여 객체를 작게 유지하도록 하였습니다. 디미터의 법칙과 책임분리가 잘 이루어 졌다고 생각합니다.
어떤 부분에 집중하여 리뷰해야 할까요?
1차에서 세부적인 로직말고 추상적인 레벨로 테스트를 작성하라는 피드백이 있었는데, 2단계에서 테스트 코드가 추상적으로 잘 작성되었는지 궁금합니다.