-
Notifications
You must be signed in to change notification settings - Fork 388
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
[1단계 - 블랙잭] 연로그(권시연) 미션 제출합니다. #220
Conversation
private void playPlayerTurn(BlackJackGame blackJackGame, Participant player) { | ||
Command command = Command.HIT; | ||
|
||
while (command != Command.STAY && !player.isFinished()) { |
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.
- while 조건을 좀 더 단순하게 작성해봅시다.
- enum의 값을 꺼내서 같은 타입의 enum끼리 비교하지 말고 enum 내부에서 처리해보세요.
|
||
while (command != Command.STAY && !player.isFinished()) { | ||
command = inputCommand(player); | ||
blackJackGame.drawCardByCommand(player, command); |
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.
blackJackGame.drawCardByCommand(player, command); | |
blackJackGame.drawCard(player, command); |
파라미터 값으로 전달하는데 메서드명에 또 쓸 필요는 없겠네요.
@@ -0,0 +1,34 @@ | |||
package domain.card; | |||
|
|||
public enum Denomination { |
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.
Denomination이 뭔가요?
블랙잭에서 사용하는 용어가 맞나요?
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.
트럼프 카드의 숫자 또는 알파벳을 denomination이라고 표현하기 때문에 이름을 지었습니다!
다시 찾아봤는데 블랙잭에서는 카드의 숫자/알파벳보다는 배팅한 돈(chip)에서 더 자주 사용하는 단어 같네요
질문하셨던 부분 답변을 드리면 서로 다른 패키지에 존재하더라도 같은 객체를 필요로하는 테스트들이 많아요. '명시적이다'라는 것에 대해서는 제 개인적인 생각을 말씀드릴 수밖에 없는데요;; 😅 그러면 연로그가 생각하는 abstract class의 이점은 무엇이었는지 알 수 있을까요?? 새로운 객체로 감싸서 반환을 할 수도 있고 불변으로 반환을 할 수도 있었을텐데 getter에서 불변으로 반환한 이유가 무엇인가요?? |
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.
반가워요 연로그! 리뷰어 로운입니다! 😄
피드백 몇 가지 남겨봤으니 확인 부탁드려요.
궁금한 점이 생기면 언제든지 DM 혹은 댓글남주세요 ^-^
for (int i = 0; i < INIT_CARD_COUNT; i++) { | ||
cards.add(cardDistributor.distribute()); | ||
} | ||
return new Cards(cards); |
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.
기본로직이 참여자가 카드를 가져가는데 이곳에서만 카드를 2개 받아서 참여자에게 넘겨주는 이유가 있을까요?
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.
참여자가 생성한 후에 drawCard()를 호출하기보다는 참여자의 생성자 호출과 동시에 Cards를 초기화시키고 싶어서 사용했습니다!
로운의 의견을 듣고 보니 어떤 방법이 더 좋을지에 대한 고민이 되네요🤔
생성자 이후에 값 초기화를 위해 호출해야하는 메소드가 존재한다면 다른 개발자가 개발할 때 헷갈려하진 않을까요?
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.
제가 생각했던 방향을 말씀드리면,
참여자를 생성하는 것은 이름으로도 생성을 할 수 있고, 한장씩 draw하는 메서드도 있다면 참여자를 먼저 이름으로 생성하고,
drawInitialCards에서 participants.drawCard를 2번 호출하면 되지 않을까 생각을 했어요.
실제 흐름으로는 플레이어를 먼저 만들고 blackJack을 시작할 수도 있지 않을까요??
participants의 생성이 cards에 종속되는 듯한 느낌이 들어서 말씀드려 봤습니다~
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.
- BlackjackGame 생성자 - Name을 이용해 Player와 Dealer 생성
- BlackjackGame의 drawCard() 메서드 2번 호출
위 순서를 말씀하신게 맞으실까요?
실제 흐름으로 생각해보면 일단 참가자가 모집된 후, 그 후에 카드가 배분되니 로운의 말에 설득이 되네요
다만 drawCard를 2번 호출하는 부분에 대해서는 이견이 있는데요
저는 카드를 어떻게 초기화할지에 대한 로직(2장을 뽑는다)은 BlackjackGame의 책임이라고 생각했기 때문에 따로 메소드를 생성했습니다!
이 부분에 대해서는 어떻게 생각하시나요?
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.
네 2장을 뽑는다는 것은 게임에 대한 규칙이기때문에 BlackjackGame의 책임이 맞는데요.
그렇기 때문에 BlackJackGame에서 게임 흐름에 맞게 drawCard를 2번호출하면 되지 않나요??
따로 메서드를 생성했다는 것이
public void initFirstDraw(Participant participant) {
participant.drawCard(cardDistributor.distribute());
participant.drawCard(cardDistributor.distribute());
}
의 의미가 맞을까요??
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.
네 그 의미가 맞습니다!!
ConsoleGame에서 drawCard를 2번 호출하도록 하자고 제안하신걸로 착각했네요 😂
의 정도로 인식하고 있습니다! 제가 공부해본걸 정리한 것은 요 글입니다.
Abstract Class는 흔히 상위 클래스의 변동이 하위 클래스까지 영향을 끼칠 수 있기 때문에 꺼린다고 알고 있는데요.
View에 도메인 모델이 아닌 dto를 전달하면 뷰가 변경되어도 부담 없이 필드를 변경할 수 있고 뷰에서 이용하는 로직을 dto에게 책임을 위임하는 등의 역할을 할 수 있기 때문에 사용합니다! 전체적인 수정 사항
|
2,3 제 개인적인 의견입니다~ 제 개인적인 경험을 말씀드리면 추상클래스에 코드가 굉장히 많은 경우가 있었어요. dto에서 불변처리 로직은 저도 필요가 없다고 생각하고요. 단순히 필요한 데이터만 잘 넘겨주면 된다고 생각합니다! |
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.
고생하셨어요. 연로그! 이전 리뷰 내용을 잘 반영해주셨습니다 💯
몇가지 코멘트를 달아봤는데요. 코멘트 반영은 2단계에서 부탁드릴게요.
그리고 고민해보면 좋은 점들도 같이 적었으니 한번 생각해봐주세요~
추가적인 궁금증은 이 pr이 머지되어도 댓글 남겨주셔도 좋구 DM도 좋습니다.
} | ||
} | ||
|
||
private MatchResult playResult(Player player, Dealer dealer) { |
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.
결과를 도출하다
라는 의미이 메서드명으로 하는게 좀 더 명확할 거 같아요.
|
||
private static final int ACE_ADDITIONAL_VALUE = 10; | ||
protected static final int BLACKJACK_VALUE = 21; | ||
protected static final int BLACKJACK_COUNT = 2; |
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.
접근제어자가 왜 protected일까요??
이곳에서 카드 상태와 점수를 도출하고 BlackJack에 대한 기준도 관리하고 있는데요.
연로그는 Cards의 역할과 책임을 어떻게 정했을까요??
카드의 상태와 게임의 규칙과 관련된 것을 Status에서 관리한다면 status에 있는게 맞지 않을까요??
아니면 결과를 도출하기 위해 필요한 값들이니 GameResult에 있는게 맞을 수도 있지 않을까요??
ace가 있는 경우 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.
Status에서 사용하기 때문에 protected로 열었습니다!
Enum에는 상단에 pirvate static final의 선언이 불가능한데 21를 넣는건 매직넘버로 느껴져서 Cards로 빼서 사용했습니다
지금 다시 생각해보니 Enum에서 사용하기 위해 불필요한 상수를 넣다니 좋지 않은 생각인 것 같네요😂
카드의 상태에 관련해서는 Status로 옮기는게 맞을 것 같네요!
enum은 상수로 쓰자에 포인트를 두다보니 자꾸 로직을 밖에서 쓰는 경향이 있는 것 같습니다ㅠ.ㅠ
원래는 ACE가 있는 경우 10을 더할지 말지랑 별개로 '카드들의 합계'를 구하는거니 Cards에 있어도 괜찮다고 생각했습니다
하지만 로운의 말을 듣고보니까 게임의 규칙과 연관이 있는거니 GameResult에 있는게 맞을 것 같아요
다음 단계 때 반영하겠습니다 :D
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.
enum 상단에 private static final의 선언이 불가능하다고 했는데요.
public enum Yn {
Y, N;
private static final number = 1;
이렇게 사용하면 되지 않나요??
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.
public enum Command {
private static final String YES = "y";
private static final String NO = "n";
HIT(YES),
STAY(NO);
}
public enum Command {
HIT(YES),
STAY(NO);
private static final String YES = "y";
private static final String NO = "n";
}
이런 식의 사용이 불가능하다는 의미였습니다!
} | ||
|
||
public void drawCard(Participant participant, Card card) { | ||
if (participant.getClass() == Player.class) { |
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.
class를 확인하는 것은 isinstanceof를 사용하는 것과 의미가 같지 않을까요??
안녕하세요 로운!!
요번 주도 벌써 다 지나갔네요 고생 많으셨습니다!
이번 과제 잘 부탁드려요😊
이번 미션을 진행하면서 고민했던 점에 대해도 추가로 남겨둡니다!
공통으로 사용하는 테스트 메소드
편의를 위해 Cards를 생성해주는 getCards() 메소드를 생성했습니다.
요 메소드를 테스트 코드 여기저기서 사용해서 사용하는 곳마다 생성해주었습니다
원래는 public으로 만들었는데 혹시나 메소드 내부 로직이 변경되면 다른 테스트 코드에 영향을 줄 수 있다고 생각했습니다
서로 다른 패키지에 존재하는 경우 테스트 메소드를 public화 해서 사용하는건 어떻게 생각하시나요?
Abstract Class vs Interface
처음에는 수정의 자유도가 비교적 높은 Interface를 택했었는데
공통된 상태와 공통된 메소드 내용이 많아 Abstract Class로 변경했습니다.
java에서 어떤 식으로 활용하나 찾아보기 위해 Map, AbstractMap, HashMap으로 찾아보았는데요
HashMap은 Map을 implements하는 동시에 AbstractMap을 extends 하고 있었고 왜 그런가에 대해서 찾아보았습니다
그러다 stackoverflow 글을 보게 되었는데 '내부 구현 로직을 세세히 볼 필요 없이 interface를 통해 메소드 목록을 즉시 확인할 수 있어 명시적이다.' 라고 이해했습니다.
하지만 전 오버 프로그래밍이 아닌가 라는 의문이 드네요😂
명시적이다라는 의미를 잘 이해하지 못한 것 같은데 로운은 이 문제에 대해 어떻게 생각하시나요?
렌트카 미션에서 interface 사용
블랙잭 미션은 아니지만 렌트카에서 interface 사용하라는 조건이 있었습니다.
interface를 사용해서 구현해보았으나 abstract class보다 이점이라고 느껴지는 점이 없어서 롤백했습니다.
혹시 어떤 맥락에서 interface를 사용하려고 한 것인지 힌트를 주실 수 있을까요?ㅜ.ㅜ
dto의 사용
이전 미션에서는 생성자, getter에서 불변 처리를 해줘도 충분했는데 이번에는 구조가 복잡해지면서 불변 처리를 하는게 헷갈리더라고요
불변 처리 역할을 위임하기 위해 dto를 생성하는 것은 어떻게 생각하시나요?