[최현민] 연료 주입, 블랙잭 - Step1 #39
Conversation
- 기능 요구 사항 작성
- 팩토리 매세드 적용
- addCar 기능 추가
- generateReport 기능 추가
| private static final double DISTANCE_PER_LITER = 15; | ||
| private double distance; |
|
|
||
| private static final String NAME = "K5"; | ||
| private static final double DISTANCE_PER_LITER = 13; | ||
| private double distance; |
There was a problem hiding this comment.
Car의 모든 하위 구현체들이 distance를 상태로 가지고 getTripDistacne() 또한 동일한 결과 값을 반환하는 것 같아요. 🤔
추상 클래스인 Car에게 이 인스턴스 변수를 할당하고 메서드도 정의하면 반복되는 코드를 줄일 수 있을 것 같아요. 😃
| for(Car car : cars){ | ||
| report.append(car.getName()+" : "+(int)car.getChargeQuantity()+ LITER + NEW_LINE); | ||
| } |
There was a problem hiding this comment.
IDE의 코드 자동 정렬 기능(mac 기준 ⌘ + ⌥ + L)을 습관적으로 실행시켜주세요! 🙏
| for(Car car : cars){ | |
| report.append(car.getName()+" : "+(int)car.getChargeQuantity()+ LITER + NEW_LINE); | |
| } | |
| for (Car car : cars) { | |
| report.append(car.getName() + " : " + (int) car.getChargeQuantity() + LITER + NEW_LINE); | |
| } |
| private static List<Card> makeBlackJackCards() { | ||
| return Arrays.stream(CardNumber.values()) | ||
| .flatMap(number -> | ||
| Arrays.stream(CardType.values()) | ||
| .map(type -> new Card(number, type))) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
Deck으로 쓰이는 카드들은 카드의 형태와 갯수들이 정해져있고 재사용할 확률이 높다고 생각해요. 😃
미리 만들어뒀다가 필요한 시점에 재사용해보면 어떨까요? 🤗
|
|
||
| public Card cardDraw() { | ||
| if (cards.isEmpty()) { | ||
| throw new RuntimeException(INVALID_DRAW); |
There was a problem hiding this comment.
RuntimeException의 하위 (예외) 클래스는 100개가 넘는다고 해요. 👀
지금처럼 예외 처리로 RuntimeException을 던지게 되면, 이 예외를 catch하는 경우 해당 예외뿐만이 아니라 예상치 못했던 에러까지 잡히는 문제가 발생해요.🙂
즉, 예상치 못한 에러가 예외 처리돼버려서 애플리케이션에 결함을 발견하지 못할 수도 있어요. 🥲
구체적인 예외 클래스를 사용해서 예외 처리하도록 변경해주세요! 🙏
| if (playerDeck.getTotalScore() == dealerDeck.getTotalScore()) { | ||
| return DRAW; | ||
| } | ||
|
|
||
| if (playerDeck.getTotalScore() > dealerDeck.getTotalScore()) { | ||
| return WIN; |
There was a problem hiding this comment.
총점수를 계산하는 불필요한 연산이 반복되고 있어요! 🥲
playerDeck과 dealerDeck의 getTotalScore 메서드를 매번 호출하지 말고 필요한 시점에 한 번 호출할 때, 변수로 추출해둔 이후에 재사용하도록 변경해주세요! 🙏
| return LOSE; | ||
| } | ||
|
|
||
| public List<String> getDealerGameResult() { |
There was a problem hiding this comment.
get, set으로 시작하는 메서드 네이밍은 지양하는 것이 좋아요. 😃
자바 진영에서는 관습적으로 객체의 상태를 외부로 꺼내는 메서드를 getter, 객체의 상태를 변경하는 메서드를 setter로 정의하기 때문에 일반적인 getter 메서드와 혼동될 수 있어요. 🙂
| .entrySet() | ||
| .stream() | ||
| .sorted(Map.Entry.comparingByKey()) | ||
| .map(m -> m.getValue() + m.getKey().getName()) |
| } | ||
|
|
||
| public int getTotalScore() { | ||
| int total = ownCards.stream().mapToInt(Card::getCardNumber).sum(); |
There was a problem hiding this comment.
stream은 한줄에 하나의 메서드만 호출하도록 해서 가독성도 높이고 인텔리제이의 편의 기능도 이용하도록 변경해주세요! 🙏
| if (total <= BLACKJACK - ACE_NUMBER_TEN && ownCards.stream().anyMatch(Card::isAceCard)) { | ||
| return total + ACE_NUMBER_TEN - ACE_NUMBER_ONE; | ||
| } |
There was a problem hiding this comment.
아래와 같이 변경해보면 어떨까요? 🤗
| if (total <= BLACKJACK - ACE_NUMBER_TEN && ownCards.stream().anyMatch(Card::isAceCard)) { | |
| return total + ACE_NUMBER_TEN - ACE_NUMBER_ONE; | |
| } | |
| boolean hasAceCard = ownCards.stream().anyMatch(Card::isAceCard); | |
| if (total + ACE_NUMBER_TEN <= BLACKJACK && hasAceCard) { | |
| return total + ACE_NUMBER_TEN - ACE_NUMBER_ONE; | |
| } |
이현호 님과 페어프로그래밍을 통해 미션을 진행했습니다.
추상 클래스를 많이 사용해보지 않아서 배울 수 있었던 기회였던 것 같습니다.
블랙잭 미션은 너무 어려워서 코드가 상당히 난잡해졌는데
피드백 주시면 빠르게 고쳐나가도록 하겠습니다.
감사합니다