Skip to content

Conversation

hyoenju
Copy link

@hyoenju hyoenju commented Feb 17, 2022

안녕하세요

연료 주입과 블랙잭 미션 구현 완료하여 과제 제출합니다.

감사합니다 :)

hyoenju and others added 30 commits February 11, 2022 16:02
- CarTest.java
- RentCarTest.java
hyoenju and others added 29 commits February 16, 2022 01:47
- Player 와 Dealer에게 카드 2장을 배분 프로세스 구현
- Dealer의 이름은 "딜러"로 고정
- Dealer의 카드를 출력을 OutputView로 묶음
- 매직넘버 & 리터럴 추가
Copy link

@hyunniiii hyunniiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요~ 현주님
몇가지 코멘트 남겼습니다. ″̮
확인부탁드립니다!

Comment on lines +48 to +62
public void receive(Player player) {
Gameable gameable = player.getCards();
String yesOrNo = "";
do {
yesOrNo = InputView.inputYesOrNo(player.getName());
if (yesOrNo.equals("y")) {
gameable.addCard(CardDeck.pop());
OutputView.printCurrentCardsState(player.getName(), player.getCards());
gameable = gameable.judge();
}
if (yesOrNo.equals("n")) {
gameable = new State(gameable.cards(), false);
}
} while (gameable.isEnd());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수(또는 메서드)의 길이가 10라인을 넘어가지 않도록 구현한다.
요구사항에 맞도록 구현해보면 어떨까요?

private static final String DEALER_NAME = "딜러";
private final static int DEALER_THRESHOLD = 16;

private final List<Player> players;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

players를 일급컬렉션으로 분리해주는건 어떨까요?
책임을 조금 더 분리할 수 있을 것 같습니다.

}

public boolean giveCardToDealer() {
if (dealer.getCards().cards().sumScore() <= DEALER_THRESHOLD) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디미터 법칙을 위반하고 있네요.
dealer.getCards().cards().sumScore() <= DEALER_THRESHOLD 이 부분에 대한 책임을 넘겨주고 결과만 받아서 사용하도록 하면 어떨까요?

public class Winner {

private final int BLACKJACK = 21;
private final Game game;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Winner 라는 클래스가 상태값으로 Game을 가지고 있네요.
Winner우승자들을 상태값으로 가지고 있는게 맞지 않을까요? ″̮

아니면 조금 더 적절한 이름으로 변경해주는건 어떨까요?
한번 고민해보면 좋을 것 같습니다.


public class Dealer extends Person {

private final Gameable cards;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cardsPerson에 구현해서 중복을 줄여보는건 어떨까요?

import java.util.List;
import java.util.stream.Collectors;

public class OutputView {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디미터 법칙을 위반하는 곳이 많네요.
디미터 법칙을 위반하지 않도록 수정해주세요 : )
https://mangkyu.tistory.com/147

}

public static void printGameWinOrLose(Person person, List<Integer> gameResult) {
System.out.printf("%s : %d승 %d패%n", person.getName(), gameResult.get(0), gameResult.get(1));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스크린샷 2022-02-20 오후 12 03 48

a,b 가 2패가 아니라 각각 1패가 되야하지 않을까요? 확인 부탁드립니다.


private static final String CAR_NAME = "Avante";
private static final double DISTANCE_PER_LITER = 15;
private final double distance;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상수와 변수는 개행을 통해 구분해주세요 : )

@@ -0,0 +1,27 @@
package rentcar.domain;

public abstract class Car {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

각 자동차들의 distance들도 추상클래스에 구현해주면 어떨까요?
중복을 줄일 수 있을 것 같습니다.


private static final String CAR_NAME = "K5";
private static final double DISTANCE_PER_LITER = 13;
private final double distance;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 상수와 변수는 개행을 통해 구분해주세요 : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants