[1단계 - 블랙잭 구현] 조엘(조영상) 미션 제출합니다. #146
Conversation
|
안녕하세요 지노! 우테코 3기 조엘입니다.
피드백 기다리고 있겠습니다! |
1. Domain 클래스로 Players VS Controller에서 사용하는 List 리팩토링 하면서 이 부분에 대해서 고민을 가장 많이 했던 것 같아요. private void progressEveryPlayerGame(final Players players, final CardDeck cardDeck) {
for (final Player player : players.getPlayers()) {
progressSinglePlayerGame(cardDeck, player);
}
} 이럴바엔 List를 Controller에서 정의해주고 사용하는게 낫겠구나 싶어, 하지만, 모든 플레이어의 이름이 중복되지 않게 초기화 되어야 함을 깨닫고 이를 책임질 클래스가 있어야 겠구나 생각이 들었고, 결국 모든 플레이어들의 게임 진행 및 결과 생성을 책임지는 Players 클래스를 생성해주었습니다. 2. View에서 많은 getter가 필요함 현재 코드 상에서는 view단에서 필요한 정보를 출력해주기 위해서는 많은 getter가 필요해요. #146 (comment) 해당 코드의 단점으로는, 도메인 객체가 어떻게 구성되어있는지 명확히 알아야 필요한 정보를 정확히 가져올 수 있지 않나 라는 생각이 들어요. 해결책으로는 두 가지가 생각나는데요, [1 - 한 번에 내부 구현체를 가져오게 한다] 하지만 이 방법은 좋지 않아보여요. [2 - DTO를 도입한다] 지노의 생각은 어떤가요? View의 getter 로직을 줄이기 위해 어떤 방법이 있을까요? |
학습로그[JDK] Stream.forEach vs Collection.forEach - 2내용
링크
[OOP] Enum 활용 - 1내용
링크 |
|
+ 추가고민) check라는 네이밍이 근데 좋은지 모르겠네요ㅜㅜ 괜히 메서드명만 봤을때에는 boolean을 리턴 받아야 할 것 같거든요. 게임 결과를 확인하고 필요한 자료구조를 반환하는 메서드의 이름은 무엇으로 지으면 좋을까요? + 리팩토링) |
학습로그[JDK] new ArrayList<>() vs Arrays.asList() - 2내용
링크
[JDK] HashMap vs LinkedHashMap - 3내용
링크 |
|
안녕하세요 조엘! 질문 확인이 늦었네요! 😭
player에 대한 일급컬랙션은 아주 좋은 방법이라고 생각합니다! :) 일급컬렌션이 필수는 아니지만 저는 얻는 이점이 많다고 생각해요!
현재는 view에서 어떠한 방법을 쓰셔도 괜찮습니다! :) 개인적으로 뷰에서는 get get도 나쁘지 않다고 생각해요! List<player> elements = players.getElements();
for(player element : elements) {
..
}
private void print(Player player) {
..
}
현재는 저는 그다지 추천하지는 않습니다. DTO는 date transfer object로 데이터 전달 오브젝트를 뜻해요! 전에도 말했지만 조엘이 DTO를 도입한다는 걸 저는 말리지는 않을 거지만 현재는 그다지 필요는 없을 것 같습니다 :) |
| OutputView.showDealerGameResult(dealer, dealer.generateEveryResult(players)); | ||
| OutputView.showPlayerGameResult(players.generateEveryPlayerResult(dealer)); | ||
| } | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
마지막 줄은 개행으로 끝납니다 :)
| public CardDeck() { | ||
| this.cards = new LinkedList<>(); | ||
| initialize(); | ||
| } | ||
|
|
||
| private void initialize() { | ||
| for (final CardSuit type : CardSuit.values()) { | ||
| Arrays.stream(CardLetter.values()) | ||
| .forEach(number -> cards.add(new Card(number, type))); | ||
| } | ||
| Collections.shuffle(cards); | ||
| } |
There was a problem hiding this comment.
cards를 내부에서 초기화하는 것과 생성자로 주입 받는 것은 어떤 차이가 있을까요??
| public void receiveInitialCard(final CardDeck cardDeck) { | ||
| hand.add(cardDeck.distribute()); | ||
| hand.add(cardDeck.distribute()); | ||
| } | ||
|
|
||
| public void receiveAdditionalCard(final Card card) { | ||
| hand.add(card); | ||
| } |
There was a problem hiding this comment.
추상클래스의 메서드를 override 못하게 하려면 메서드에 final 키워드를 붙여 주면 됩니다!
| public static Players of(final List<String> playerName) { | ||
| validateDuplicate(playerName); | ||
| final List<Player> players = playerName.stream() | ||
| .map(Player::new) | ||
| .collect(Collectors.toList()); | ||
| return new Players(players); | ||
| } | ||
|
|
||
| private static void validateDuplicate(final List<String> playerName) { | ||
| final int distinctName = (int) playerName.stream().distinct().count(); | ||
| if (distinctName != playerName.size()) { | ||
| throw new IllegalArgumentException("입력된 플레이어의 이름이 중복됩니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
player는 이름이 같으면 같은 객체로 취급하나요?
그렇다면 player에 equals(), hashCode()를 재정의하고 player를 set에 넣어보는 것도 방법이겠네요! :)
There was a problem hiding this comment.
이름이 같으면 같은 객체로 고려하도록 equals(), hashCode()를 정의해주었고, 테스트코드 작성을 완료했습니다!
| @Override | ||
| boolean condition(int score, int opponentScore) { | ||
| return score > opponentScore; | ||
| } |
There was a problem hiding this comment.
functional interface도 적용할 수 있을 것 같네요! :)
There was a problem hiding this comment.
- https://dev3m.tistory.com/entry/ENUM-Funcational-Interface%EB%A1%9C-if%EB%AC%B8-%EC%A4%84%EC%9D%B4%EA%B8%B0
- https://woowabros.github.io/tools/2017/07/10/java-enum-uses.html
해당 글을 참고하여 수정해봤습니다! 맞게 한건지는 잘 모르겠으나 테스트케이스가 통과하여 우선 commit 했습니다 :)
| private int countAce() { | ||
| return (int) cards.stream().filter(Card::isAce).count(); | ||
| } |
There was a problem hiding this comment.
| private int countAce() { | |
| return (int) cards.stream().filter(Card::isAce).count(); | |
| } | |
| private int countAce() { | |
| return (int) cards.stream() | |
| .filter(Card::isAce) | |
| .count(); | |
| } |
개행으로 가독성을 높일 수 있겠네요! :)
안녕하세요 지노! 우테코 3기 조엘입니다.
블랙잭 미션을 페어들과 함께 작성하고, 추가적인 개인 리팩토링은 아직 많이 하지 않았어요.
지노의 의견을 들어보고 리팩토링에 들어갈 생각입니다.
1. Controller에서 처리해줄 로직이 너무 많다.
TDD를 통해 객체를 설계하고, 이를 블랙잭 게임에 알맞게 컨트롤러에서 조립하다보니 컨트롤러가 이전 미션에 비해 길어지게 됐어요.
이를 해결할 방법으로 도메인에 게임을 진행해주는 클래스를 생성하거나, 컨트롤러를 역할 별로(초기화/게임진행/승패결정) 나누자라는 의견이 나왔어요.
하지만 게임을 진행하는 클래스를 도메인에 생성하는 것은 컨트롤러의 역할을 위임한다고 밖에 생각이 들지 않았고,
컨트롤러를 잘게 나누는 것은 그저 컨트롤러가 길기 때문에 나누는 느낌이 강하게 들었어요.
다른 크루가 컨트롤러가 너무 길어진 것은 어쩌면 domain에서 처리해줘야할 일을 컨트롤러에서 해주는 거일 수 있다고 했어요.
리팩토링하면서 이에 대해 더 생각해볼 예정이에요.
2. Participant 클래스의 List<Card>
Participant 클래스는 인스턴스 변수로 List<Card>를 가지고 있어요.
페어들과 함께 List<Card>를 일급 컬렉션으로 만들까에 대한 의논을 했었는데, 결과적으로 현재는 따로 빼지 않기로 했어요.
Participant 클래스 내의 로직들이 주로 List<Card>를 관리하는 로직들이였어요.
카드를 받고, 현재 카드들의 가치를 계산하고, Bust가 되었는지 확인하는,
일급 컬렉션이였으면 아마 같이 분리되었을 로직들을 Participant가 가지고 있더라고요.
이를 따로 일급컬렉션으로 분리하려고 하니까, Participant에서 실질적으로 처리하는 로직은 자신의 이름을 받고, getter로 넘겨주는 것 밖에 안남더라고요.
물론 따로 일급컬렉션으로 List<Card>를 분리했다면, 재활용이 더 용이하고, List<Card>에 대한 상태/행위만 관리하기 때문에 장점도 분명히 있을 것 같아요.
어쩌면 이름이 생겨 가독성이 더 좋아질 수도 있고요.
하지만 참가자의 카드 목록을 컨트롤러로 가져오기 위해, 두 번의 getter가 필요하고 (DTO를 도입한다면 해결이 될 듯 하기도 하네요)
참가자 별로 List<Card>를 관리하는 객체를 하나 더 생성하는 것이 필수적인가에 대해 의문이였던것 같아요.
(사실... 생각을 글로 정리하고 나니, DTO를 도입해 getter에 대한 문제를 해결하고, 일급컬렉션으로 List<Card>를 포장하면 장점만 취하는 좋은 방안이 될 수 있을것 같네요)
1. DTO 도입
사실 이전 미션까지는 DTO의 필요성을 느끼지 못했고, 그에 따라 DTO를 도입하지 않았었어요.
불변 객체를 만들면 레이어 간에 안전한 운반이 가능하다고 생각했어요.
하지만 이번에 진행한 블랙잭 미션은 그렇지 않았어요.
우선 participant내에 Dealer와 Player같은 경우, 승패를 가리는 필드가 유동적이에요.
그렇기 때문에 불변성을 보장할 수 없었고, DTO가 이를 해결해줄 수 있지 않을까 생각하고 있어요.
지금은 우선 controller에서 필요한 데이터를 getter를 통해 가져온 다음, view에 보내주고 있어요.
2. 게임 결과를 만들고 관리하는 클래스 생성
현재는 controller에서 플레이어와 딜러간의 결과를 비교하고, 결과를 필드값으로 지정해줬어요.
하지만 게임의 결과를 관리해주는 클래스를 만들어줘야 할 필요성을 느껴요.
우선 현재 코드의 문제점은 controller에서 플레이어와 딜러의 결과값을 하나하나 비교해주고,
플레이어와 딜러에게 각각 승패를 지정해주는 것이라고 생각합니다.
플레이어가 직접 딜러의 결과를 보고 스스로 승패를 정하는 방식으로 코드를 작성하는게 더 좋은 코드라는 생각이 드네요.
혹은 Map을 사용해 플레이어가 승리했는지, 패배했는지를 인스턴스 변수로 저장해 둘 수도 있을 것 같고요.
이는 추후에 더 고민해볼게요.
3. Dealer클래스와 Player 클래스의 추상화
이 부분이 개선이 되면 코드가 훨씬 간결해지지 않을까 싶어요.
사실 현재는 Participant 클래스로 공통된 메서드와 필드를 추출한 정도에 지나지 않아요.
추상화를 잘 해냈다면, Dealer와 Player를 한 번에 관리하는 클래스를 도메인에 만들수도 있을 것 같고,
controller에서 Dealer객체와 List<Player>를 따로따로 만들어줘 비슷한 로직을 컨트롤러에서 수행해준 부분도 개선이 가능할 것 같아요.
리뷰 기다리고 있겠습니다!
코로나 조심하세요 :)