-
Notifications
You must be signed in to change notification settings - Fork 75
[박지현] 연료 주입, 블랙잭 (Step1) #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: jihyunhillpark
Are you sure you want to change the base?
Conversation
- 생성자 호출시 인자를 통한 여행거리 초기화 - 연비 구하는 기능 getDistancePerLiter() 상수를 사용하여 return Constant - 여행 거리를 구하는 기능 getTripDistance() 멤버변수를 반환
- 자동차 구현클래스들은 RentCar 추상 클래스를 상속받고, RentCar는 Car 인터페이스를 구현한다.
- 초기 카드덱 생성시 Collections.shuffle() 메소드를 통해 카드를 섞는다.
- 카드를 추가하는 addCard() 메서드 추가
- 게임 시작 시, 두 장의 카드를 배분하는 deal() 메서드 생성
- checkAceOneOrEleven() 메서드 추가하고, 11을 더해도 버스트가 되지 않으면 에이스를 11로 취급하도록 수정
- getCardNames(): 참여자가 가진 카드 리스트 반환 - getAllCards(): 참여자가 가진 모든 카드의 이름을 문자열로 join해 반환
- 대신, getName() 메서드 추가
- Gmae 내 결과 산출 기능 메소드 분리 - dealer와 players를 전달하여 GameResult 클래스에서 최종결과 산출
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지현님 안녕하세요!
블랙잭 미션 잘 구현해주셨네요!! :)
몇가지 코멘트를 남겼으니 확인 부탁드리겠습니다!!
# java-blackjack | ||
# 연료 주입 | ||
## 기능 요구 사항 | ||
우리 회사는 렌터카를 운영하고 있다 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기능 요구 사항 작성 👍
@@ -0,0 +1,5 @@ | |||
package rentcar; | |||
|
|||
public class RentCarMain { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메인 메서드가 없는 것 같아요..?!
} | ||
|
||
@Override | ||
public String toString() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
View(외부의 출력)을 위해 toString을 재정의 하는건 지양해야 해요!
이렇게 하면 View의 출력방식이 바뀌면 toString을 또 변경해야하기도 하고, 2개 이상의 출력방식에 대해서는 지원이 불가능한 구조가 되어버립니다! 요부분 이해가 잘 안가면 디엠 주세요!
@@ -0,0 +1,19 @@ | |||
package rentcar.util; | |||
|
|||
public class Constant { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 클래스를 없애고, 각각의 상수를 사용하고 있는 클래스의 안으로 넣어주면 어떨까요?
예를들어 SONATA_DISTANCE_PER_LITER
나 SONATA_NAME
은 Sonata 클래스에 정의해주는 것이죠!! 이렇게했을 경우에는 어떤 이점이 있을까요?
public List<Card> initDeck() { | ||
List<Card> deck = new ArrayList<>(); | ||
for (Suit suit : Suit.values()) { | ||
for (Denomination denomination : Denomination.values()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
depth가 2를 넘어가는 것 같아요! 아래의 요구사항을 지켜보면 어떨까요?
indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다.
예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.
힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메서드)를 분리하면 된다.
break; | ||
} | ||
|
||
player.hit(deck); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Game
이라는 객체에서 dealer와 player, Deck을 모두 가지고 있는데, 최초 2장을 뽑는 것 이외의 모든 작업은 Controller에서 하는게 어색해보이는데요!
Game객체를 조금더 활용해보면 어떨까요?
} | ||
|
||
public int getScore() { | ||
int total = cards.stream().mapToInt(card -> card.getDenomination().getScore()).sum(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
card.getScore()
라는 메서드를 만들어보면 어떨까요?
int total = cards.stream().mapToInt(card -> card.getDenomination().getScore()).sum(); | |
int total = cards.stream().mapToInt(card -> card.getScore()).sum(); |
if (total + 10 <= 21) { | ||
return total + 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매직넘버가 많아보여요~!
} | ||
|
||
@Override | ||
public String toString() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 toString재정의에 대해 다시한번 고려해주시면 좋을 것 같아요!
return cards.getCardNames(); | ||
} | ||
|
||
public String getAllCards() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 역할은 Domain객체의 역할이 아닌 것 같아보이는데요! 어떻게 생각하시나요~?
안녕하세요! 리뷰어님!!
이번 blackjack 미션은 세라님@Xxell-8 과 함께 진행했습니다!
저에게 이번 블랙잭 미션은 다소 어려웠던 것 같습니다! ㅠㅠㅠㅠ
(객체의 책임 정의하는 것과 메소드로 메시지를 전달하는 것에 많은 고민이 많았지만 결국 머리가 펑 터졌습니다!🤯)
리팩토링할 것도, 부족한 것도 많지만 잘 부탁드리겠습니다!!