[1단계 - 블랙잭 게임 실행] 이든(최승준) 미션 제출합니다. #654
Conversation
PgmJun
changed the title
src/main/java/blackjack/domain/card/UnShuffledDeckGenerator.java
Outdated
Show resolved
Hide resolved
Set: O(1) List: O(N)
distinct는 중복 제거를 위해 Stream의 모든 요소와 비교하지만 Set은 O(1) 시간복잡도로 조회,삭제를 수행하기 때문에 성능상 이점을 위해 Set을 사용하도록 변경
ArrayList -> List
유지보수성 증가를 위해 한 곳에서 관리하고, 존재하는 Command인지 검증하는 행위를 객체가 지니게 하기 위해 Enum 적용
해싱처리를 하는 HashMap과 달리, 지정된 Enum클래스의 상수 순서를 인덱스를 사용하기 때문에 Key가 Enum 타입인 경우 사용하게 되면 더욱 빠른 성능을 가짐
|
|
||
| private void validateCommand(String command) { | ||
| if (!List.of(COMMAND_YES, COMMAND_NO).contains(command)) { | ||
| throw new IllegalArgumentException(String.format("%s 또는 %s만 입력할 수 있습니다.", COMMAND_YES, COMMAND_NO)); |
There was a problem hiding this comment.
Command는 도메인 아닌가요? ㅎㅎ
view에서 도메인이 있게되네요
| List<Card> copyDeck = new ArrayList<>(cards); | ||
| validate(copyDeck); | ||
| this.cards = copyDeck; |
There was a problem hiding this comment.
| List<Card> copyDeck = new ArrayList<>(cards); | |
| validate(copyDeck); | |
| this.cards = copyDeck; | |
| validate(cards); | |
| this.cards = new ArrayList<>(cards); |
불필요한 변수 생성일것 같네요!
There was a problem hiding this comment.
이 부분은 이번에 테코톡을 준비하는 과정에서 배운 TOCTOU(Time of check, Time of use라는 개념을 반영해본 내용인데요!
만약 멀티스레딩 환경이라고 가정했을 때
Deck 객체의 생성자 인자로 받은, 참조 객체 List<Card> cards가
validate(check) 로직을 수행하고 this.cards = deck(use) 로직을 통해 인스턴스 변수에 초기화 되는 그 찰나의 시점에
외부에서 다른 스레드에 의해 상태 변경이 발생한다면,
Deck 객체 내부에 저장되는 값은 검증에서 성공했던 값이 아닌 다른 값으로 변경된 값이기 때문에
검증의 신뢰도가 떨어지게 됩니다.
이러한 상황을 고려해서 먼저 방어적 복사를 수행한 뒤에 검증을 수행하라는 내용이 이펙티브 자바 Item50에 나와있는데요!
사실 영이 말씀대로 지금은 멀티스레딩 환경이 아니기 때문에 필요없는 것이 맞지만,
정말 좋은 내용이라고 생각되었고 TOCTOU를 고려하는 좋은 습관을 들여보고자 이번 미션 코드에 적용을 해보게 되었습니다!
혹시 영이는 이렇게 멀티스레딩 환경이 아니더라도 안전성을 고려하는 습관을 들이기 위해
공부한 것을 조금씩 적용해보는 것이 과한 엔지니어링이라고 생각하시는 지 궁금합니다 🥲
There was a problem hiding this comment.
좋습니다👍
| List<Card> copyDeck = new ArrayList<>(cards); | |
| validate(copyDeck); | |
| this.cards = copyDeck; | |
| this.cards = new ArrayList<>(cards); | |
| validate(this.cards); |
item50 의 예제를 보니까
순서를 이렇게 가져가면 불필요한 변수생성은 없앨수 있지 않을까요?
(자세히 읽어보지는 않았습니다...)
There was a problem hiding this comment.
제대로 읽으신 게 맞는 것 같아요! :)
하지만 제가 책과는 조금 다르게 지금과 같은 코드를 작성한 이유는
코드의 가독성을 위해서였습니다.
검증된 값을 초기화하는 것이 아니라
값을 필드에 초기화한 뒤에 검증한다는 흐름이 읽는 사람에게 약간의 혼란을 줄 수도 있을 것 같다고 생각이 들었거든요..!
때문에 코드 한 라인을 희생(?)해서 변수 생성작업을 거쳐
코드의 가독성을 올릴 수 있다면
이렇게 사용해보는 것도 괜찮을 것 같다고 생각했습니다!
영이의 생각은 어떠신가요?
| validateDuplicate(cards); | ||
| validateSize(cards); |
There was a problem hiding this comment.
굳이 메서드로 한번 더 만들어서 처리하기보다는
validate안에서 모두 조건을 체크하면 어떨까요?
핵심 비즈니스로직은 아니기도 하고 검증이 더 필요하다면 클래스가 복잡해질 수 있을것 같아요
There was a problem hiding this comment.
굳이 메서드로 한번 더 만들어서 처리하기보다는 validate안에서 모두 조건을 체크하면 어떨까요? 핵심 비즈니스로직은 아니기도 하고 검증이 더 필요하다면 클래스가 복잡해질 수 있을것 같아요
private void validate(final List<Card> cards) {
int distinctSize = new HashSet<>(cards).size();
if (cards.size() != distinctSize) {
throw new IllegalArgumentException("카드는 중복될 수 없습니다.");
}
if (cards.size() != INIT_SIZE) {
throw new IllegalArgumentException(String.format("덱의 사이즈가 %d장이 아닙니다.", INIT_SIZE));
}
}영이의 의견은 요렇게 작성하는 것이 더 괜찮아 보인다는 말씀이 맞으실까요??
제가 이해를 잘 한 건지 모르겠어서 일단 이 코드를 바탕으로 답변을 드리겠습니다..!
(아니셨다면 죄송합니다..!)
저도 처음에는 이렇게 코드를 작성하고 리팩토링 한 것인데요!
물론 메서드 분리를 하지 않고 하나의 메서드에서 처리하면 메서드가 늘어나지 않아서 복잡도가 줄어들지만
저는 가독성이 많이 아쉬웠다고 느꼈습니다.
또 여기서 validate 메서드 내부에 검증 로직이 늘어나게 되면 로직의 가독성이 더욱 저하되고,
코드 라인이 요구 사항인 10라인보다 더 늘어나게 될 것 같다고 느껴
각각의 검증을 메서드로 분리하여 가독성을 더 고려해보았습니다.
private void validate(final List<Card> cards) {
validateDuplicate(cards);
validateSize(cards);
}때문에 어떤 검증 로직인지 이름을 붙여서 가독성을 높여 보고자,
이렇게 변경해보았습니다!
결론적으로 지금 상황은 가독성을 고려해서 메서드를 분리하느냐 VS 클래스 복잡도를 줄이기 위해 하나의 메서드에서 관리하느냐 이 두 가지 사이의 트레이드 오프 싸움인 것 같습니다
저는 지금 필요한 검증 로직은 2개 밖에 존재하지 않으니 메서드로 분리하더라도
클래스 복잡도에 큰 영향이 없을 것이라고 생각하는데 영이의 생각은 어떠신지 궁금합니다!
There was a problem hiding this comment.
저는 우선 가독성의 면에서도 메서드를 분리하지 않는것이 좋다고 생각합니다
검증로직이 하나만 추가되어도 검증로직을 이해하기 위해 3번을 왔다갔다 해야할것 같아요!
이든이 생각하시기에 분리하는게 좋다고 생각하시면 지금처럼 진행해도 괜찮을것 같습니다🙏
어디까지나 제 주관적인 생각이니까요!
There was a problem hiding this comment.
저는 우선 가독성의 면에서도 메서드를 분리하지 않는것이 좋다고 생각합니다 검증로직이 하나만 추가되어도 검증로직을 이해하기 위해 3번을 왔다갔다 해야할것 같아요! 이든이 생각하시기에 분리하는게 좋다고 생각하시면 지금처럼 진행해도 괜찮을것 같습니다🙏 어디까지나 제 주관적인 생각이니까요!
저도 영이가 검증로직을 찾기위해 스크롤을 여러번 왔다갔다 해야한다고 하신 부분에 공감이 되네요!
그러나 검증 로직을 메서드로 관리하게 되면 어떤 검증을 수행하는 지 이름을 통해 먼저 유추할 수 있고,
그 덕분에 이름 없는 검증 코드가 여러 개 섞인 validate 메서드를 유심히 살펴보며
각각의 코드가 어떤 검증의 책임을 가졌는 지 이해하는 것보다 더 좋은 가독성을 제공한다고 생각했어요!
또한 validate 메서드 아래에 validateDuplicate와 validateSize를 호출 순서대로 함께 배치시켜놓으려고 신경써주고 있기 때문에 지금 상태로 유지해도 괜찮을 것 같다고 생각이 됩니다:)
메서드 분리에 대해 다시 한번 생각해볼 기회를 만들어주셔서 감사해요 🙂
| public static Dealer newInstance(final Deck deck) { | ||
| return new Dealer(deck, CardHand.createEmpty()); | ||
| } |
There was a problem hiding this comment.
newInstance는 제가 개발 생활하면서 자주 보지 못했는데 어떤 장점이 있나요??
그냥 생성자나 from, of 의 정적팩토리 메서드는 정말 자주 사용하는데
newInstance는 어떤 장점이 있어서 사용하시는지 궁금합니다
(지적 이런게 아니라 정말 궁금해서 물어보는겁니다!!)
There was a problem hiding this comment.
newInstance는 제가 개발 생활하면서 자주 보지 못했는데 어떤 장점이 있나요?? 그냥 생성자나 from, of 의 정적팩토리 메서드는 정말 자주 사용하는데 newInstance는 어떤 장점이 있어서 사용하시는지 궁금합니다 (지적 이런게 아니라 정말 궁금해서 물어보는겁니다!!)
저는 newInstacne라는 정적 팩토리 메서드의 이름을 테코블 에서 보고 적용해보았습니다.
기존 코드
public Dealer(final Deck deck, final List<Card> card) {
super(new CardHand(new ArrayList<>()));
this.deck = deck;
}new Dealer(deck, new ArrayList<>());정적 팩토리 메서드를 적용하기 전 자바 코드는 이렇게 되어 있었는데요.
매번 Dealer를 생성해줄 때마다 외부에서 빈 ArrayList를 생성하여 인자에 입력해주는 부분이 굉장히 불필요하고 어색하게 느껴졌습니다.
그래서 0장의 List<Card>를 가진 초기 상태의 Dealer를 생성해준다는 의미를 가진 정적 팩토리 메서드 도입을 고려하게 되었는데요.
이때 from을 사용하지 않은 이유는 다음과 같았습니다.
보통 from이라는 네이밍은 어떠한 한 가지 값을 인자로 받아 객체를 생성함을 드러낼 때 사용하는 네이밍으로 알고 있는데요!
저는 Dealer를 생성할 때 외부로부터 입력받은 Deck 객체와
내부에서 생성한 아무런 값도 가지지 않은 new ArrayList<>() 를 통해
객체의 상태를 초기화해주는 로직을 사용하고 있기 때문에fromDeck 이라는 이름을 사용하는 것이 약간은 어색했습니다.
이때 from이 아닌 블로그에서 소개하는 newInstance라는 이름을 적용하고 추측해보았을 때,
"게임에서 새로 생성된 유저는 당연히 아무것도 가지고 있지 않다"라는 생각의 흐름으로 이어졌고
이러한 생각의 흐름이 자연스럽다고 느껴져서 newInstance 라고 네이밍하게 되었습니다.
There was a problem hiding this comment.
public Dealer(final Deck deck) {
super(CardHand.createEmpty());
this.deck = deck;
}충분히 생성자로 표현할 수 있을것 같아요!
newInstance를 사용하셔도 상관없지만 자주 사용하는 방법은 아닌것 같아요! 참고만 해주세요
There was a problem hiding this comment.
충분히 생성자로 표현할 수 있을것 같아요! newInstance를 사용하셔도 상관없지만 자주 사용하는 방법은 아닌것 같아요! 참고만 해주세요
제가 습관적으로 정적팩토리 메서드를 적용하고 있었는데요.
리뷰를 보고 "정말 여기서 정적 팩토리 메서드가 필요할까?" 에 대해 조금 더 생각해보았습니다.
하지만 영이의 의견처럼 newInstance 라는 네이밍은 new Dealer 가 의미하는 바와 똑같아서 충분히 생성자로 표현해도 괜찮을 것 같아요!
이 부분에 대해서는 차라리 생성자 체이닝 패턴을 적용해서 변경해보겠습니다:)
public Dealer() {
this(Cards.createEmpty());
}
public Dealer(final List<Card> cards) {
this(new Cards(cards));
}
public Dealer(final Cards cards) {
super(cards);
}| import blackjack.domain.card.CardHand; | ||
| import blackjack.domain.card.Deck; | ||
|
|
||
| public class Dealer extends Gamer { |
There was a problem hiding this comment.
구현체를 상속하기보다 인터페이스를 상속하는게 어떨까요?
현재는 Dealer와 Player에서 Gamer에 있는 로직이 완전히 공통적으로 적용되고 있고,
Gamer에 있는 CardHand라는 인스턴스 필드 또한 공통으로 가지고 있습니다.
또한 Dealer, Player가 Gamer 와 is-a 관계가 성립되기 때문에 상속을 선택하였는데요!
찾아보니 상속은 부모 객체의 변경이 모든 자식 객체에 영향을 미치는 등
결합도가 많이 높아서, 이후에 부모 객체가 변경되었을 때 자식 객체까지 함께 변경되기 때문에
문제 발생의 여지가 있어 사용을 지양하는 편이 좋다는 얘기가 많았습니다.
또한 자바는 다중 상속이 불가능하기 때문에 여러 기능을 조합하려면 정말 많은 클래스가 생겨나는 클래스 폭발 문제가 발생한다고도 하네요!
그래서 이 부분에 대한 변경을 고민하고 있었는데요!
Gamer를 인터페이스로 만드는 것은 Gamer가 CardHand라는 필드를 가지고 있기도 하고,
내부 로직이 전부 public static으로 변경되기 때문에 캡슐화가 되지 않아 적용하지 않았고
Composite 패턴을 적용해서
Player,Dealer가 Gamer를 가지도록 has-a 관계로 바꿔서 해결책을 제시해 볼 수도 있을 것 같다는 생각이 들어서
한번 공부하면서 적용해보겠습니다!
혹시 다른 의견이 있으시거나, 제가 잘못 알고 있는 부분이 있다면 답변 부탁드리겠습니다!
There was a problem hiding this comment.
- 처음 적용하다보니 적용하는 데에 시간이 오래걸려서 우선 리뷰 요청을 보내고 개발 이어서 해보겠습니다..!
| public static final int INIT_CARD_COUNT = 2; | ||
| public static final int MAX_HIT_SCORE = 16; | ||
|
|
||
| private final Deck deck; |
There was a problem hiding this comment.
delaer가 deck을 가지고 있는게 어색한것 같아요!
지금의 구조라면 오히려 상속을 사용해서 얻는 이점이 크게 없는것 같기도 하고요!
blackJackGame 혹은 Game 등의 클래스를 만들어서 game에서 deck, dealer, players를 처리하는게 어떤가요?
There was a problem hiding this comment.
우선 설계 시에 Dealer가 Deck을 가지고 있게 했던 이유는 아래와 같았습니다.
Blackjack 게임에 대한 도메인 지식을 바탕으로 각 객체 의 책임을 설계하는 과정에서
카드를 받을 때 플레이어가 직접 덱에서 카드를 꺼내가는 것이 아니라
딜러가 덱에서 카드를 꺼내어 플레이어들에게 나누어주는 행위를 보고
"덱에서 카드를 꺼내 나누어주는 책임은 딜러에게 있다" 라고 판단하였습니다.
또한 카드를 나눠주는 책임을 가진 딜러만 덱에 접근할 수 있어야하니,
딜러에게 덱을 가지고 있는 책임을 부여하여 덱을 캡슐화하고자 하였습니다.
이러한 흐름에 의해 카드를 나눠주는 책임을 가진 딜러에게 Deck을 가지고 있도록 구현하였습니다.
그런데 이렇게 만든 구조가 어색하다는 영이의 리뷰를 보고 다시 생각해봤을 때,
Deck이 직접 카드를 나누어주는 행위를 수행할 수도 있겠다고 생각이 들었습니다.
현실과 객체지향 세계는 다르니까요 🙂
하지만 그렇게 설계하게 된다면,
분명 게임 내에서 다른 행위를 수행함에도 불구하고,
딜러와 플레이어의 객체 내부 구현이 거의 똑같은 상태가 되어버려서
두 객체의 역할의 구분이 모호해진다고 생각이 되었습니다.
때문에 Dealer에게 Deck을 가지고 있도록 하고 싶은데,
영이는 이러한 의견에 대해 어떻게 생각하시는지 궁금합니다!
There was a problem hiding this comment.
delaer가 deck을 가지고 있는게 어색한것 같아요!
지금의 구조라면 오히려 상속을 사용해서 얻는 이점이 크게 없는것 같기도 하고요!
그리고 혹시 위에서 말씀하신 이 내용은
dealer가 deck을 가지고 있는 부분에 의해, 지금의 구조가 상속의 이점을 얻을 수 없다고 말씀하신 내용일까요??
제가 제대로 이해했다면 이 부분에 대해서 조금만 더 자세하게 설명해주실 수 있을까요??
상속을 통해서는 Player와 Dealer의 공통 로직을 한 곳에 모아 관리함으로써 이점을 얻을 수 있다고 생각했는데
Deck을 가지는 것과 상속의 이점을 얻지 못하는 것의 연관성이 있다면 그 부분에 대해 자세히 알고 싶습니다!
There was a problem hiding this comment.
구현체 상속의 이점은 중복되는 코드를 줄여서 클래스의 크기를 줄이는게 큰 장점이라고 생각하는데
deck을 별도로 필드로 가지면서 클래스의 크기는 크게 줄어들지 않는것 같아서요!
There was a problem hiding this comment.
구현체 상속의 이점은 중복되는 코드를 줄여서 클래스의 크기를 줄이는게 큰 장점이라고 생각하는데 deck을 별도로 필드로 가지면서 클래스의 크기는 크게 줄어들지 않는것 같아서요!
음음 그렇게 보이기도 하네요!
그리고 영이의 말대로 제가 객체지향에 현실세계를 계속 반영하려하면서 인지부조화가 자꾸 오고 있는 것 같습니다 🥲
덱도 하나의 살아있는 객체이기 때문에 딜러에게 가지고 있게 하기보다 우선 한 번 분리해서
메시지를 주고받으면서 기능을 수행하도록 변경해보겠습니다!
그리고 혹시 영이가 객체지향을 공부하시면서 이해가 잘 되었던 팁같은 게 있으면 배울 수 있을까요?
객사오 책을 읽으면서 공부해보았는데 아직 이해가 잘 가지 않는 것 같네요..
There was a problem hiding this comment.
너무 좋은 리뷰들 남겨주셔서 감사합니다 영이!
제가 오늘 있던 테코톡을 준비하는 과정에 생각보다 리소스가 많이 들어가서
오늘 리뷰 답변을 달고 반영을 시작하게 되었습니다..
늦게 답변드린 만큼 열심을 다해 답변하고 반영해보고 있습니다!
소중한 시간내어 리뷰해주셨는데 늦게 리뷰에 답 드려서 죄송합니다 🥲
그리고 이건 제가 판단할 영역인지 모르겠으나
제가 이번에 남긴 답변들이 길고 많다고 느껴져서
리뷰어의 리뷰 피로도를 줄여드리자는 목적으로
답변 및 변경 이후 영이가 이모지만 눌러주신 부분에 있어서는
resolve 처리를 해두었습니다!
혹시 리뷰이의 판단 하에 resolve처리 한 부분이 좋지 않은 태도로 느껴지셨다면 사과드립니다..
그리고 물론 회사마다 다르겠지만 영이는 현업에서 어떤 부분에 resolve 처리를 하게 되시는 지도 알려주신다면 앞으로의 리뷰 활동에 참고하겠습니다..!
감사합니다!
| return Arrays.stream(values()) | ||
| .filter(value -> value.text.equals(text)) | ||
| .findFirst() | ||
| .orElseThrow(() -> new IllegalArgumentException("존재하지 않는 키워드입니다.")); |
There was a problem hiding this comment.
if 문으로 처리하는게 더 깔끔하지 않을까요?
저도 for문과 if문을 통해 처리해주는 방식도 좋다고 생각했는데요!
for (Command value : values()) {
if(value.text.equals(text)) {
return value;
}
}
throw new IllegalArgumentException("존재하지 않는 키워드입니다.");하지만 이렇게 구현하게 되었을 때, 미션 요구사항에 있는
indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다.
라는 내용을 지키지 못하게 되었습니다..
때문에 Stream을 활용해서 구현해보게 되었는데요!
혹시 indent depth 1을 지키면서 구현할 수 있는 더 좋은 방식이 있을까요?! 궁긍합니다 🙂
기존에는 createPlayers 메서드를 통해 생성하고 있었기 때문에 메시지가 코드 상에 전달이 되었지만 createPlayers 메서드를 제거했기 때문에 playerNames를 통한 Players 객체 생성에 대한 메시지를 코드 상에 나타내기 위해 정적 팩토리 메서드 사용
| validateDuplicate(cards); | ||
| validateSize(cards); |
There was a problem hiding this comment.
저는 우선 가독성의 면에서도 메서드를 분리하지 않는것이 좋다고 생각합니다
검증로직이 하나만 추가되어도 검증로직을 이해하기 위해 3번을 왔다갔다 해야할것 같아요!
이든이 생각하시기에 분리하는게 좋다고 생각하시면 지금처럼 진행해도 괜찮을것 같습니다🙏
어디까지나 제 주관적인 생각이니까요!
| public static final int INIT_CARD_COUNT = 2; | ||
| public static final int MAX_HIT_SCORE = 16; | ||
|
|
||
| private final Deck deck; |
There was a problem hiding this comment.
구현체 상속의 이점은 중복되는 코드를 줄여서 클래스의 크기를 줄이는게 큰 장점이라고 생각하는데
deck을 별도로 필드로 가지면서 클래스의 크기는 크게 줄어들지 않는것 같아서요!
| public static Dealer newInstance(final Deck deck) { | ||
| return new Dealer(deck, CardHand.createEmpty()); | ||
| } |
There was a problem hiding this comment.
public Dealer(final Deck deck) {
super(CardHand.createEmpty());
this.deck = deck;
}충분히 생성자로 표현할 수 있을것 같아요!
newInstance를 사용하셔도 상관없지만 자주 사용하는 방법은 아닌것 같아요! 참고만 해주세요
| LOSE; | ||
|
|
||
| public static GameResult of(final Dealer dealer, final Player player) { | ||
| if (player.isBust() || player.getScore() <= dealer.getScore()) { |
There was a problem hiding this comment.
Step2의 요구사항으로는 존재하는데, Step1에는 존재하지 않아서
일단 필요한 기능을 만들어보았습니다!
Step2에서 무승부를 반영할 계획입니다!
| public class Card { | ||
|
|
||
| // 52장의 Card를 미리 Pool에 생성 | ||
| private static final Map<String, Card> pool = Arrays.stream(Suit.values()) |
There was a problem hiding this comment.
| private static final Map<String, Card> pool = Arrays.stream(Suit.values()) | |
| private static final Map<String, Card> POOL = Arrays.stream(Suit.values()) |
static 상수라면 대문자로 사용해주세요
pool 보다는 조금 더 의미 있는 네이밍을 해주면 좋을것 같네요! 적어도 CARD_POOL 정도?
There was a problem hiding this comment.
static 상수라면 대문자로 사용해주세요 pool 보다는 조금 더 의미 있는 네이밍을 해주면 좋을것 같네요! 적어도 CARD_POOL 정도?
앗..!대문자 처리를 놓쳤네요 🥲
확인 감사드립니다!
CARD_POOL 네이밍도 너무 좋은 것 같습니다!
| public Card drawCard() { | ||
| return cards.remove(0); | ||
| } |
There was a problem hiding this comment.
이걸보니 Deck같은경우는 list보다는 다른 자료구조가 더 적합할것 같네요!
There was a problem hiding this comment.
이걸보니 Deck같은경우는 list보다는 다른 자료구조가 더 적합할것 같네요!
말씀에서 의도하신 자료구조가 LIFO 스택으로 사용할 수 있는 Deque 일 것 같다고 생각이 드는데요:)
계속해서 상단에 있는 카드를 꺼내오는 로직만 존재하기 때문에 저도
상단 하단에 있는 값 접근에 최적화된 Deque가 적합하다고 생각이 드는 것 같습니다!
`Stack` 대신 `Deque` 를 말씀드린 이유는
`Stack` 이 `Vector` 를 잘못된 구조로 상속하고 있어서 `Stack`보다 `Deque`를 사용하라고 하더라구요!
때문에 한번 Deque로 리팩토링 해보겠습니다:)
| return new Player(name, Cards.createEmpty()); | ||
| } | ||
|
|
||
| public boolean isBust() { |
안녕하세요 영이! 이든입니다!
리뷰 잘 부탁드려요:)
요청 🙌
영이가 남겨주실 리뷰에 대한 중요도를 수치로서 인지하고,
이를 통해 투자하는 시간을 적절히 분산하여 작은 시간동안 최고의 효율을 내는 리뷰 환경을 만들어보고 싶습니다!
때문에
Pn 리뷰 룰적용을 요청드리고 싶습니다!절대 필수적으로 부탁드리는 부분이 아니며, 괜찮으시다면 부탁드리겠습니다
시간내어 리뷰해주셔서 감사합니다🙂
궁금해요 🤔
테스트를 위한 메서드를 만드는 것에 대해 어떻게 생각하시나요?
미션을 수행하면서, 초기에 52장의 카드를 가지고 잇는 Deck이라는 객체가 존재하게 되었는데요.
Deck을 개발할 때 TDD를 진행하면서, Deck에서 카드를 draw했을 때, "뽑힌 카드는 Deck 내에 더이상 존재하지 않는다" 라는 기능에 대한 테스트를 작성하게 되었습니다.
하지만 Deck은
List<Card> cards의getter를 만들 필요가 없어서 이 테스트에 대해 두 가지 선택지를 고민하게 되었습니다.AssertJ의extracting을 통해 Deck 객체 내부의List<Card> cards필드 상태 확인하기Deck에contains(Card card)와 같이Deck에게List<Card> cards의 상태를 물어보는 메서드를 만들어서 확인하기1번은
extracting을 통해 내부 인스턴스 필드를 특정할 때, 문자열을 통해 필드 이름을 지정해주어야 하기 때문에,필드 이름이 변경되는 경우가 발생하면 런타임 에러가 발생하며 테스트가 실패하게 됩니다.
extracting을 사용하는 것은contains를 만들어 사용하는 것보다 가독성이 떨어집니다.2번은
contains라는 메서드를 만들게 되어,테스트만을 위한 코드를 작성하게 됩니다.extracting을 사용하는 것보다 가독성이 높으며, 필드명 수정이 발생하더라도 에러가 발생하지 않아 코드 수정의 수고를 줄여줍니다.저희는 아래와 같은 근거에 의해 1번이 더 낫다는 판단을 세워 1번 방식을 선택하게 되었습니다.
extracting을 사용하는 부분에서 런타임 에러가 발생하기 때문에, 필드명 변경 시 테스트 코드 변경을 놓치더라도 빠른 대응이 가능하다.영이는 이러한 상황에서 어떤 선택을 하셨을지, 아니면 다른 해결책이 있는 지 등의 영이의 생각을 남겨주시면 참고하겠습니다!
감사합니다 🙂