[🚀 사이클1 - 미션 (블랙잭 게임 실행)] 아티 미션 제출합니다.

[ParkJiYeoung8297]

블랙잭

체크 리스트

• 미션의 필수 요구사항을 모두 구현했나요?
• Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
• 애플리케이션이 정상적으로 실행되나요?

어떤 부분에 집중하여 리뷰해야 할까요?

1. BlackjackResultTest.java의 참가자가_딜러보다_점수_낮음() 테스트

• private인 메서드 테스트를 위해서, 최종 결과 dto를 비교해서 간접적으로 private 메서드를 검증했습니다.
  하나만의 메서드가 아니라 여러 private 메서드를 거친 결과인데, 최종 dto로 검증하는 방식이 해당 기능을 검증했다고 볼 수 있는지 궁금합니다.

2. SOLID 측면에서 개선할 사항이 있는지 궁금합니다.
   특히 SRP 면에서, 하나의 책임으로 메서드가 잘 구성되어있는지 피드백 받고 싶습니다.

3. inputView와 OutputView를 유틸로 하는 것이 좋을지, 인스턴스로 하는 것이 좋을 지 궁금합니다.

4. if문 분기처리에 대해 가이드라인을 알고 싶습니다. enum을 기반으로 구현할 때 switch 없이 구현하게 되면
   if문이 코드에 많이 나오게 되는 것을 경험했습니다. values()를 기반으로 for문을 돌려 유연하게 처리되는 경우는 그렇게 해결했지만, 이때도 매끄럽지 않게
   해결되는 경우가 있었습니다. 이때는 다른 방향이 있을지 궁금합니다.

---

구현 내용

필수 요구 사항 체크리스트

• 블랙잭 게임은 딜러와 플레이어 중 카드의 합이 21 또는 21에 가장 가까운 숫자를 가지는 쪽이 이긴다.
• Ace는 1 또는 11로 계산한다.
• King, Queen, Jack은 각각 10으로 계산한다.
• 게임을 시작하면 플레이어는 두 장의 카드를 지급 받는다.
• 21을 넘지 않을 경우 원한다면 얼마든지 카드를 계속 뽑을 수 있다.
• 딜러는 처음에 받은 2장의 합계가 16이하이면 반드시 1장의 카드를 추가로 받아야 한다.
• 딜러는 처음에 받은 2장의 합계가 17점 이상이면 추가로 받을 수 없다.
• 게임을 완료한 후 각 플레이어별로 승패를 출력한다.

추가로 정의한 제약 사항

1. 플레이어가 모두 burst라면, dealer는 추가 카드를 받지 않고 무조건 승리한다.
2. 플레이어 카드 burst라면, 추가 카드를 받지 못하고 턴을 넘긴다.

기능 목록

• 참가자 입력 기능 구현
• 참가자 이름과 카드 개수, 카드 출력
• 카드 랜덤 생성하기
• 참가자 카드 배정하기
• 추가 카드 여부 입력 구현
• 추가 카드 출력 구현
• 딜러 추가 카드 여부 판단 로직
• 카드 합계 기능 구현
• Ace 처리 기능 구현
• 버스트 판단 여부 기능 구현
• 딜러 추가 카드 반복 배부 기능 구현
• 카드 결과 판단 기능 구현
• 블랙잭 승부 계산 (burst 고려)
• 모든 player burst 판단 로직 추가
• 딜러 승부 개수 계산 기능
• 딜러 카드 출력 기능 구현
• 카드 21 초과이면 턴 넘어가기 기능 추가

---

페어프로그래밍 사전 학습

미션 중 할 일

1. 토론 활동에서 정한 규칙을 의식하며 코드 작성
2. 규칙 때문에 코드를 변경한 곳 기록
3. 막히는 순간 기록

미션 중 기록

필수 기록:

• 규칙을 적용해서 변경한 코드 1곳 이상
• 테스트 작성이 어려웠던 코드 1곳 이상
• 막힌 순간 1회 이상

1. 적용한 규칙 : 다중 책임
   처음 구현할 때, 우선 기능을 먼저 구현하는 것에 초점을 두었다.
   그러다보니 Controller의 run 메서드에 모든 로직을 집중되었다.
   이를 해결하기 위해 도메인 생성과 초기 설정에 관한 기능들을 service 코드에 분리하였다.

2. 테스트 작성이 어려웠던 곳 : 일급 컬렉션을 사용하면서 Cards의 길이를 테스트하기 어려웠다.

3. 막힌 순간 :

   1. Player와 Dealer의 중복되는 행동이 있는데 클래스를 어떻게 구현할지.
      공통되는 메서드를 모은 하나의 클래스를 따로 구현하기 vs Player에 구현 후 상속받아서 사용하기
      → Player에 메서드를 구현한 후 Dealer가 이를 상속받아 사용하도록 하였다.
      
      
      
   2. Cards를 공용으로 사용해야하는데 어디서 관리할지.
      main에서 생성 후 service 필드로 두기 vs service 메서드 내에서 생성하기
      → 필드로 두지 않고 Service 메서드 내에서 생성해서 Controller로 넘겨주었다.
      
      
      
   3. 어떤 것을 테스트 해야하지?
      shuffle을 하는 것은 우리가 검증할 필요가 없다고 생각해서,
      카드 사이즈 비교를 통해 원하는 기능이 구현되었는지 테스트하였다.
      예시. 1) 카드 생성 후 우리가 원하는 카드 52장이 생성 2) 플레이어와 딜러가 초기에 2장씩 나눠받았는지

미션 완료 조건

• 요구사항 구현
• 규칙에 의한 코드 변경 1회 이상
• 미션 중 기록 작성

기록

1. 다중 책임 규칙
   처음에 controller에서 모든 기능이 모여있어서 기능 별로 테스트가 어려웠다

고민했던 점

1. controller에서 입출력 기능 메서드를 private으로 해야하는지
2. InputView, OutputView 유틸 클래스 아니면, 인스턴스 클래스로?

---

코드 리뷰 반영 사항 (260311 추가)

• BlackjackService에서 cards를 shuffle하는 책임 분리
  → 인터페이스 사용, 외부에서 인스턴스를 주입하게끔 구조를 변경해
  테스트 시 ShuffledCardsGenerator를 FakeShuffledCardsGenerator로 대체해서
  shuffle 값을 조작가능하도록 함.
• Dealer의 이름 상수에 대한 책임을 Controller에서 Dealer 클래스로 변경
• List대신 Players라는 일급 컬렉션 사용
• 테스트에서만 사용되는 메서드 getCardCount() 삭제
• 키와 값을 모두 사용할 때는 entrySet() 사용
• 사용하지 않는 import 제거

질문

1. 상수화
   Dealer 클래스에 둔 아래의 상수의 경우 outputView에서 Dealer.ADDITIONAL_THRESHOLD라고 사용해도 될까요?
   캡슐화의 관점으로 Dealer 내부로 옮긴건데 outputView에서 사용하면 의도대로 작동한게 맞나 싶어서요.
   책임만 Dealer에게 할당하고, 사용은 어처피 상수여서 OutputView에서 사용해도 상관없을까요?

public static final int ADDITIONAL_THRESHOLD = 16;

2. 인터페이스 사용
   shuffle은 랜덤 추출처럼 통제할 수 없는 값이라,
   이 값을 테스트에서 편하게 조작해주기 위해 CardsGenerator라는 인터페이스를 추가했습니다.
   덕분에, FakeShuffledCardsGenerator를 사용해서 테스트 시 제가 직접 순서를 조작할 수 있었습니다.
   통제할 수 없는 값을 테스트 할때, 인터페이스를 사용해서 주로 하나요? 아니면 다른 방법도 있나요?
   추가로, 지금 인터페이스와 이를 이용한 테스트 코드가 잘 구현되어있는지 궁금합니다!
   (참고 테스트 코드 : BlackjackServiceTest의 셔플_카드_순서대로_배부_정상_테스트())

3. 더 추가해야할 테스트가 있을지 궁금합니다.

[jurlring]

아티 안녕하세요! 블랙잭 리뷰를 맡게된 주디입니다 🐰 반가워요!

아티의 생각이 궁금한 부분과 수정하면 좋을 부분을 코멘트로 남겨두었어요.
시간 괜찮으실 때 확인해주세요~
그리고 이번 미션에서 학습하고 싶으신게 무엇인지 궁금해요.
학습해야할 것이 무엇인지, 내가 부족한게 뭔지 인지하는 것은 너무너무 중요하거든요!
미션이 끝나고 제가 확인할게요!

[jurlring]

controller가 필요할까요?

[ParkJiYeoung8297]

controller가 알맞은 로직을 호출하는 역할을 하고 있기 때문에 필요하다고 생각합니다!🙂

입출력 관련해서는 InputView와 OutputView를 호출하고, 적절한 비즈니스 로직의 경우 주로 Service를 호출합니다.
물론 지금은 Service가 하나 밖에 없지만, 추후 Service가 늘어날 확장성을 고려해서도 분리해주는 것이 좋다고 생각합니다!
지금도 입출력과 비즈니스 로직 중 어느 로직을 호출할지 안내해주는 네비게이션 같은 역할을 하고 있다고 생각해요.

질문을 주셔서 생각해봤는데 Controller가 없다면 어떤 방법으로 구성해야하는지 감이 잘 안오네요.🥲
혹시 Controller를 사용하지 않는 경우도 있나요?

[jurlring]

MVC 패턴을 고려해서 Controller를 만들어주신 걸까요?

패턴은 개발자들이 비슷한 문제를 반복적으로 겪으면서, 이런 상황에서는 이렇게 설계하는 것이 좋다는 경험이 쌓여 만들어진 것이라고 생각해요. 그래서 구조가 어느 정도 정형화되어 있어 코드를 읽을 때 전체적인 구조를 예측하기 쉽고 이해하기 쉽다는 장점도 있는 것 같아요.

그래서 이번 구현에서 왜 MVC 패턴이 필요하다고 생각하셨는지 궁금해서 질문드렸습니다 🙂

다만 Controller라는 형태가 아니더라도 구현할 수 있는 방법은 다양할 것 같아요.
패턴은 여러 구현 방법 중 하나이기 때문에 반드시 고수해야 하는 것은 아니라는 점을 함께 말씀드리고 싶었어요~!

[ParkJiYeoung8297]

추후의 웹개발로 가면 controller-service-domain 구조를 사용하기 때문에,
순수 자바에서도 그 계층별 역할을 뚜렷하게 이해해보고 싶어서 controller-domain 계층으로 코드 작성을 했습니다.
시작할 때는 service가 굳이 필요하지 않다고 생각했던 거 같아요.

근데 코드를 작성하다보니 controller에서 객체 생성이나 dto 생성까지 섞여버리면 코드가 너무 길고 클래스가 무거워져서
service에게 이런 중간 역할의 책임을 옮겼습니다.

사이클 2를 진행하면서 도메인 구조를 다시 재구성했는데, 그 결과 아래와 같이 책임이 나눠진 것 같습니다.

InputView : 입력 책임
OutputView: 출력 책임
Controller: 입력, 출력, service중 어는 것을 호출할지 결정
Service: 도메인을 생성하거나 dto를 생성
Domain: 핵심 비즈니스 로직들

주디 피드백처럼 controller가 꼭 필요한 과제는 아니였어서, 또 다른 방식의 구현도 추가로 찾아보는게 좋을 거 같네요.
좀 더 찾아보고 공부해보겠습니다. 피드백 감사합니다!☺️

[jurlring]

CardGenerator 클래스에 final에 붙이신 이유가 궁금해요 :)

클래스, 메서드, 필드에 각각 final을 붙일 때 어떤 의미일까요~?
그리고 final이 왜 존재하는지 아티의 생각도 궁금해요 :)

[ParkJiYeoung8297]

유틸 클래스이기 때문에, 상속을 막기 위해서 final을 사용했습니다.
유틸 클래스는 내부에 static 메서드들만 있기 때문에, 상속이 필요없다고 생각합니다.

final의 의미

자바 docs 문서를 참고해서 final의 의미에 대해서 정리해보았습니다.

Oracle java docs

1. 클래스 final
   클래스 자체의 상속을 막는다.
   객체의 불변 상태를 유지한다.

2. 메서드 final
   상속받은 클래스에서 메서드를 오버라이드 하지 않게 막는다.
   생성자 메서드의 경우 일반적으로 final로 정의하여, 재정의되는 것을 막는다.

3. 필드 final
   필드의 상태가 불변하도록 막는다.

정리하자면, final은 객체의 불변성을 보장하고 캡슐화하기 위해 존재하는 것이라고 생각합니다.
외부에서 이 객체의 타입에 대한 조작을 할 수 없어서, 안정적으로 객체를 관리할 수 있습니다.

[jurlring]

CardGenerator.generate로 Cards가 나온다면 CardsGenerator가 더 맞는 의미 아닐까요~?

또한 BlackjackService가 cards를 shuffle하는 책임까지 가져야한다고 생각하시는지 궁금해요 :)

[ParkJiYeoung8297]

피드백 주신 대로 CardsGenerator가 더 적절할 것 같네요. ☺️

BlackjackService가 cards를 shuffle하는 부분도 페어랑 논의했던 부분인데요
저는 shuffle을 따로 분리해서 사용하는 것이 더 낫다고 생각합니다.
해당 코드를 작성할 때는 페어에게 제대로 설명하지 못해서,
우선은 기능 개발을 목표로 Service에 작성 후 리팩토링하기로 했었습니다.

피드백 주신 덕분에 다시 한번 고민하고 결론을 내려봤습니다!
인터페이스로 분리하면 Service와 shuffle에 대한 의존성이 낮아지고, 외부에서 shuffle 값을 mocking 해서 넣어줄 수 있어 테스트에 용이할 것 같습니다. 그래서 수정한다면 외부에서 의존성 주입으로 shuffle의 인터페이스를 넣어줄 것 같습니다.

[jurlring]

인터페이스 사용
shuffle은 랜덤 추출처럼 통제할 수 없는 값이라,
이 값을 테스트에서 편하게 조작해주기 위해 CardsGenerator라는 인터페이스를 추가했습니다.
덕분에, FakeShuffledCardsGenerator를 사용해서 테스트 시 제가 직접 순서를 조작할 수 있었습니다.
통제할 수 없는 값을 테스트 할때, 인터페이스를 사용해서 주로 하나요? 아니면 다른 방법도 있나요?
(참고 테스트 코드 : BlackjackServiceTest의 셔플_카드_순서대로_배부_정상_테스트())

네 예측할 수 없는 값을 테스트할때는 mocking하거나, 인터페이스로 분리하기도 합니다 :)

추가로, 지금 인터페이스와 이를 이용한 테스트 코드가 잘 구현되어있는지 궁금합니다!

FakeShuffledCardsGenerator에서 고정된 cards만 나올 수 있는 구조로 보이는데, FakeShuffledCardsGenerator에 상태를 가지게 하고, 생성할때마다 넣어주고 generateShuffledCards에서 응답하게 해도 좋을 것 같아요~

[ParkJiYeoung8297]

상태를 갖게하면 테스트마다 다른 카드로 넣어줄 수 있어서, 코드를 재사용하기 좋겠네요!
반영해보겠습니다 😁

class FakeShuffledCardsGenerator implements CardsGenerator {
    private final List<Card> cards;

    public FakeShuffledCardsGenerator(List<Card> cards) {
        this.cards = cards;
    }

    public Cards generate() {
        return new Cards(cards);
    }
}

[jurlring]

입력 횟수에 대한 제한을 두고 이 횟수를 넘어가면 게임이 종료되도록 구현해주셨군요!
사용자의 단순 실수로 게임이 종료되어 사용자의 불편함을 개선한 부분이라고 보이네요! 멋져요! 💯
(반영하지 않아도 됨) 다만 서비스 개발자로서 고민해보면 좋을 포인트는 몇 회까지 재시도하는 것이 좋을까~? 에 대한 부분이에요. 많다고 좋지도 않고 적다고 좋지도 않지요!

[ParkJiYeoung8297]

보통 비밀번호는 5회, 10회 기준으로 많이 설정하는데
게임이기 때문에 5회보다는 10회가 적절할 것이라고 생각했습니다!

[jurlring]

여기 뿐만아니라, 전체적으로 사용되지 않는 import가 있어요.
습관적으로 IDE에서 제공해주는 Code Reformat를 활용하면 좋아요 :)

[jurlring]

Cards를 사용하지 않으신 이유가 있나요?

[ParkJiYeoung8297]

Player가 추가로 카드를 받을 수 있기 때문에, 일급 컬렉션인 Cards보다는 List가 적절하다고 생각했습니다!

[jurlring]

헉 그럼 일급컬렉션을 만든 의미가 없지 않을까요!?
Cards에도 카드를 추가적으로 받게하면 안되나요?

[ParkJiYeoung8297]

Cards를 전체 카드 묶음이라 생각해서 Player에서 사용하지 않았던 것 같습니다.
다시 생각해보니, 전체 카드 묶음과 클레이어가 가진 카드 묶음은 행동의 차이가 있어서 따로 구분해서 사용하는 것이 좋겠네요.

사이클 2에서 Deck(전체 카드)과 Cards(플레이어 소유 카드)로 분리해서 구현하고,
플레이어에서 Cards라는 일급컬렉션을 사용하도록 수정하겠습니다.🙂

[jurlring]

테스트에서만 사용되는 메서드로 보여요.

[ParkJiYeoung8297]

저도 테스트만을 위해서 메서드를 추가는 것에 대해 고민을 많이 했었습니다.
결론을 내리지 못한 상태로 코드를 작성했는데, 추가로 팀원들과 논의를 해보고 나니
테스트만을 위한 메서드는 최소한으로 하는 것이 좋은 것 같습니다.

이 메서드 보다는 위에 getCards 메서드를 이용해서 cards.size()로 값을 가져오면,
getCardCount()는 사용하지 않아도 될 것 같습니다!

[jurlring]

Dealer가 Player를 상속받도록 구현하신 이유가 있나요?
딜러와 플레이어가 is-a 관계라고 생각하시는지 궁금해요 :)

[ParkJiYeoung8297]

이 부분은 페어와 고민했던 점이었습니다!
딜러도 사실 플레이어로도 역할을 하고 있어서, 딜러와 플레이어의 행동이 겹친다고 생각했습니다.

1. 본인의 카드를 소유한다.
2. 본인 카드의 합을 구한다.

페어가 행동이 겹친다면 상속을 받아서 사용하자고 제안해주어서, 해당 방법으로 구현해보았습니다.
사용해보니 중복 코드가 없어지는 장점이 있는 것 같습니다.

다만, 강한 결합이 생기고, 클래스의 캡슐화가 깨질 수 있다는 점에서, 상속 대신 해당 역할만 가진 클래스로 아에 분리하는 방법도 고려해볼 수 있을 것 같습니다!

[jurlring]

네 말씀주신대로 추상클래스를 상속하는 것은 결합도가 엄청 높은 작업이에요.
그렇다보니 요구사항이 변경된다면 수정이 어렵지요!

플레이어는 카드를 꺼내는 행위를 하는데 상속받는 딜러는 불필요한 행위인데 할 수 있게 되어 있는 것으로 보이는데요
딜러가 불필요한 행위를 할 수 있게 열어두는게 맞을까요?

[ParkJiYeoung8297]

딜러에게 불필요한 행위는 막아두는 것이 안정적이겠네요. 🙂
상속받는 객체도 해당 행동을 할 수 없게 private로 수정하겠습니다!

[jurlring]

좋은 고민이라고 생각합니다~! if를 완전히 없애는 것이 항상 좋은 방향은 아니기 때문에, 상황에 따라 어떤 방식이 더 적절한지 고민해 보는 것이 중요해요!

enum을 사용할 때 switch 대신 if가 많아지는 경우가 있는데, 이때는 조건 분기 자체를 줄일 수 있는 방향이 있는지 먼저 고민해 보면 좋을 것 같습니다. 예를 들어 enum에 각 상황에 맞는 행동이나 로직을 함께 두어 분기를 enum 내부로 이동시키는 방법도 하나의 방식이 될 수 있을 것 같아요 :)

[ParkJiYeoung8297]

enum 내부로 이동시키는 방법 너무 좋네요!
생각해보니 프리코스 동안에 코드를 구현할 때도, enum으로 만든 후에 enum 값을 순회해서 조건 분기를 해결하려고 했던게 생각나네요. if가 많아지는 것을 막을 수 있고, 코드도 훨씬 깔끔해질 것 같습니다.
피드백 감사합니다!

[ParkJiYeoung8297]

피드백 주셔서 너무 감사합니다! 많이 생각해볼 수 있는 리뷰여서 너무 도움되었어요. ☺️
내일 나머지 커멘트에도 리뷰 달겠습니다.

[ParkJiYeoung8297]

전역적으로 사용하는 상수의 경우 따로 뺐고, 클래스 내부에서 사용하는 상수의 경우 클래스 내부에 위치하도록 했습니다.

1. 블랙잭의 조건 → 21
2. 딜러가 카드를 더 받는 조건 → 16

지금 리뷰 주신 내용으로 다시 생각해보니, 2번의 경우에는 딜러에게만 해당하기 때문에 딜러 클래스에 두는게 나을거 같네요.

[ParkJiYeoung8297]

이 부분은 페어와 고민했던 점이었습니다!
딜러도 사실 플레이어로도 역할을 하고 있어서, 딜러와 플레이어의 행동이 겹친다고 생각했습니다.

1. 본인의 카드를 소유한다.
2. 본인 카드의 합을 구한다.

페어가 행동이 겹친다면 상속을 받아서 사용하자고 제안해주어서, 해당 방법으로 구현해보았습니다.
사용해보니 중복 코드가 없어지는 장점이 있는 것 같습니다.

다만, 강한 결합이 생기고, 클래스의 캡슐화가 깨질 수 있다는 점에서, 상속 대신 해당 역할만 가진 클래스로 아에 분리하는 방법도 고려해볼 수 있을 것 같습니다!

[ParkJiYeoung8297]

Player가 추가로 카드를 받을 수 있기 때문에, 일급 컬렉션인 Cards보다는 List가 적절하다고 생각했습니다!

[ParkJiYeoung8297]

11도 상수로 두는 것이 좋을 것 같습니다!
숫자나 문자와 같이 변경될 가능성이 있거나 전역적으로 사용하는 값을 상수로 만들면,
관리하기도 수월하고 테스트에서도 동일한 값을 사용할 수 있어서 좋은 것 같습니다.

[ParkJiYeoung8297]

저도 테스트만을 위해서 메서드를 추가는 것에 대해 고민을 많이 했었습니다.
결론을 내리지 못한 상태로 코드를 작성했는데, 추가로 팀원들과 논의를 해보고 나니
테스트만을 위한 메서드는 최소한으로 하는 것이 좋은 것 같습니다.

이 메서드 보다는 위에 getCards 메서드를 이용해서 cards.size()로 값을 가져오면,
getCardCount()는 사용하지 않아도 될 것 같습니다!

[ParkJiYeoung8297]

피드백 주신 대로 CardsGenerator가 더 적절할 것 같네요. ☺️

BlackjackService가 cards를 shuffle하는 부분도 페어랑 논의했던 부분인데요
저는 shuffle을 따로 분리해서 사용하는 것이 더 낫다고 생각합니다.
해당 코드를 작성할 때는 페어에게 제대로 설명하지 못해서,
우선은 기능 개발을 목표로 Service에 작성 후 리팩토링하기로 했었습니다.

피드백 주신 덕분에 다시 한번 고민하고 결론을 내려봤습니다!
인터페이스로 분리하면 Service와 shuffle에 대한 의존성이 낮아지고, 외부에서 shuffle 값을 mocking 해서 넣어줄 수 있어 테스트에 용이할 것 같습니다. 그래서 수정한다면 외부에서 의존성 주입으로 shuffle의 인터페이스를 넣어줄 것 같습니다.

[ParkJiYeoung8297]

제가 일급 컬렉션에 대한 개념이 확실하지 않아서 찾아봤는데, 아래와 같이 나왔습니다.

일급 컬렉션(First Class Collection)은 컬렉션(List, Set, Map 등)을 Wrapping하면서,
해당 컬렉션 이외의 다른 멤버 변수를 가지지 않는 클래스를 의미합니다.

우선 코멘트 달아주신 해당 클래스는 밑에 다른 변수가 있기 때문에, 일급 컬렉션은 아닌 것 같습니다.
저와 페어가 이 클래스를 만들었을 때도 단순 집게용이나 결과 저장용으로 클래스를 만들었던 것 같아요.

질문 주셨던 일급 컬렉션을 왜 사용해야하는지에 대해서는 아래와 같이 정리해보았습니다!

1. 객체의 불변성 보장 (또는 캡슐화)
2. 여러 객체를 대상으로 한 비즈니스적인 검증과 행동을 묶어서 클래스 구성 가능 (응집성을 더욱 향상시켜줌)

우선 일급 컬렉션으로 클래스를 생성하고, 내부에서 추가, 수정, 삭제 등을 private으로 메서드를 구현하면
외부에서는 추가, 수정, 삭제 등을 수행할 수 없습니다. 이것이 객체의 불변성을 보장해준다고 생각합니다.
또한 객체의 상태를 일급 컬렉션 내부에서만 조작하기 때문에 더더욱 캡슐화에 용이해집니다.

List 와 같이 카드 하나가 아닌 여러 개를 대상으로 검증이나 행동을 하고 싶을 때,
일급 컬렉션을 사용해서 내부에 메서드를 구현하면 응집성있게 코드를 구현할 수 있을 것이라고 생각합니다.

피드백 주신 덕분에 잘 몰랐던 일급 컬렉션에 대해 조금이나마 알게된 것 같아요. 감사합니다!
혹시 다르게 생각하시는 부분이 있다면 리뷰 부탁드립니다. ☺️

[ParkJiYeoung8297]

enum 내부로 이동시키는 방법 너무 좋네요!
생각해보니 프리코스 동안에 코드를 구현할 때도, enum으로 만든 후에 enum 값을 순회해서 조건 분기를 해결하려고 했던게 생각나네요. if가 많아지는 것을 막을 수 있고, 코드도 훨씬 깔끔해질 것 같습니다.
피드백 감사합니다!

[ParkJiYeoung8297]

entrySet()을 사용하면 키와 값을 동시에 갖고 올 수 있군요! 감사합니다.

[ParkJiYeoung8297]

입력값 검사에서 검증을 여러 개 할수도 있어서, 좀 더 자유롭게 하기 위해 인터페이스를 사용했던 것 같습니다.

Validator를 인터페이스로 구현해야, Validator 묶음을 아래와 같이 간단하게 추가할 수 있어요

String input = readInput(List.of(
        Validator::validateNotBlank,
        Validator::validateChoice
));

Consumer를 이용해서 사용하는 방법도 있지만,
"List<Validator> validators"가 "List<Consumer> validators"와 같이 더 가독성이 떨어지기도 하고,
검증이라는 의미의 Validator를 이름으로 사용하는 것이 좋다고 판단했습니다.

가벼운 검증의 경우 말씀해주신 대로 static 검증 코드로 사용하는 것도 좋을 것 같습니다! 😄

[ParkJiYeoung8297]

코드 리뷰 반영 사항

• BlackjackService에서 cards를 shuffle하는 책임 분리
  → 인터페이스 사용, 외부에서 인스턴스를 주입하게끔 구조를 변경해
  테스트 시 ShuffledCardsGenerator를 FakeShuffledCardsGenerator로 대체해서
  shuffle 값을 조작가능하도록 함.
• Dealer의 이름 상수에 대한 책임을 Controller에서 Dealer 클래스로 변경
• List대신 Players라는 일급 컬렉션 사용
• 테스트에서만 사용되는 메서드 getCardCount() 삭제
• 키와 값을 모두 사용할 때는 entrySet() 사용
• 사용하지 않는 import 제거

질문

1. 상수화
   Dealer 클래스에 둔 아래의 상수의 경우 outputView에서 Dealer.ADDITIONAL_THRESHOLD라고 사용해도 될까요?
   캡슐화의 관점으로 Dealer 내부로 옮긴건데 outputView에서 사용하면 의도대로 작동한게 맞나 싶어서요.
   책임만 Dealer에게 할당하고, 사용은 어처피 상수여서 OutputView에서 사용해도 상관없을까요?

public static final int ADDITIONAL_THRESHOLD = 16;

2. 인터페이스 사용
   shuffle은 랜덤 추출처럼 통제할 수 없는 값이라,
   이 값을 테스트에서 편하게 조작해주기 위해 CardsGenerator라는 인터페이스를 추가했습니다.
   덕분에, FakeShuffledCardsGenerator를 사용해서 테스트 시 제가 직접 순서를 조작할 수 있었습니다.
   통제할 수 없는 값을 테스트 할때, 인터페이스를 사용해서 주로 하나요? 아니면 다른 방법도 있나요?
   추가로, 지금 인터페이스와 이를 이용한 테스트 코드가 잘 구현되어있는지 궁금합니다!
   (참고 테스트 코드 : BlackjackServiceTest의 셔플_카드_순서대로_배부_정상_테스트())

3. 더 추가해야할 테스트가 있을지 궁금합니다.

[ParkJiYeoung8297]

안녕하세요 주디.
피드백 주셔서 감사합니다. 생각해볼 질문들이 많아서 너무 도움되었어요. 😁

커멘트에 남겨주신 질문에 대해서 정리해보았습니다.

그리고 이번 미션에서 학습하고 싶으신게 무엇인지 궁금해요.
학습해야할 것이 무엇인지, 내가 부족한게 뭔지 인지하는 것은 너무너무 중요하거든요!

이번 미션에서는 테스트에 대해서 강조하는 것 같아서, 테스트에 대해 집중해보고 싶어요.
그래서 피드백 반영 시에도 인터페이스를 사용해서 shuffle에 대한 테스트를 진행했습니다.
흔히 말하는 mock 방식의 테스트가 이런 방식인지는 조금 더 공부해봐야할 것 같아요. 😅

사이클 2까지 하면서 아래의 질문에 저의 나름대로 답을 내려보는 것이 목표입니다!

1. 테스트해야하는 메서드인지 판단하는 기준
2. private, shuffle 등 테스트가 어려울 때, 테스트 할 수 있는 대체 방법

[jurlring]

아티 안녕하세요!
수정할 부분과 여쭤보고 싶은 부분 코멘트로 남겨두었습니다~!
다음 사이클에서 반영주세요 :)

[jurlring]

record에 대해서 아시나요~? 학습해보시면 좋을 것 같아요 :)

[ParkJiYeoung8297]

저는 Card를 도메인 모델로 생각하고 record를 사용하지 않았는데,
다시보니 도메인 모델 내에서 VO와 같은 성격을 띄고 있네요.
지금까지 record는 Dto에서만 사용했었는데, VO에서도 사용한다는 점을 배울 수 있었습니다! ☺️

[Record 정리]

1. 모든 필드가 private final로 선언되어 불변 객체이다. (setter 제공 X)
2. equals(), hashcode()등을 자동을 만들어줘서 코드가 간결해진다.
3. Dto, Vo 등에서 불변 데이터를 담을 때, 사용될 수 있다.

[jurlring]

MVC 패턴을 고려해서 Controller를 만들어주신 걸까요?

패턴은 개발자들이 비슷한 문제를 반복적으로 겪으면서, 이런 상황에서는 이렇게 설계하는 것이 좋다는 경험이 쌓여 만들어진 것이라고 생각해요. 그래서 구조가 어느 정도 정형화되어 있어 코드를 읽을 때 전체적인 구조를 예측하기 쉽고 이해하기 쉽다는 장점도 있는 것 같아요.

그래서 이번 구현에서 왜 MVC 패턴이 필요하다고 생각하셨는지 궁금해서 질문드렸습니다 🙂

다만 Controller라는 형태가 아니더라도 구현할 수 있는 방법은 다양할 것 같아요.
패턴은 여러 구현 방법 중 하나이기 때문에 반드시 고수해야 하는 것은 아니라는 점을 함께 말씀드리고 싶었어요~!

[jurlring]

인터페이스 사용
shuffle은 랜덤 추출처럼 통제할 수 없는 값이라,
이 값을 테스트에서 편하게 조작해주기 위해 CardsGenerator라는 인터페이스를 추가했습니다.
덕분에, FakeShuffledCardsGenerator를 사용해서 테스트 시 제가 직접 순서를 조작할 수 있었습니다.
통제할 수 없는 값을 테스트 할때, 인터페이스를 사용해서 주로 하나요? 아니면 다른 방법도 있나요?
(참고 테스트 코드 : BlackjackServiceTest의 셔플_카드_순서대로_배부_정상_테스트())

네 예측할 수 없는 값을 테스트할때는 mocking하거나, 인터페이스로 분리하기도 합니다 :)

추가로, 지금 인터페이스와 이를 이용한 테스트 코드가 잘 구현되어있는지 궁금합니다!

FakeShuffledCardsGenerator에서 고정된 cards만 나올 수 있는 구조로 보이는데, FakeShuffledCardsGenerator에 상태를 가지게 하고, 생성할때마다 넣어주고 generateShuffledCards에서 응답하게 해도 좋을 것 같아요~

[jurlring]

서비스나 도메인에서 인스턴스 객체로 두면 결국 동일할거라 계층간의 분리가 더 명확하다는 이해를 못했어요!
두 방법중에 정답은 없지만 계층마다 구현을 통일하는게 좋을 것 같습니다 :)

앞으로 어떤 계층이든 점점 넓어질텐데, 구현 방법이 제각각이면 개발할 때도 어떤 방법을 따라야할지 고민되고 읽을때도 좀 더 오래 걸릴 수 있을 것 같아요~~
정답이 없는 문제들은 컨벤션을 잡고 통일하는게 유지보수하기 쉬워진다고 생각해요!

[jurlring]

객체의 꼭 해야하는 행위를 public으로 두고, 가독성이 떨어지고 메서드 분리 하는 것이 나을 때 private로 분리하고있어요 :)

[jurlring]

헉 그럼 일급컬렉션을 만든 의미가 없지 않을까요!?
Cards에도 카드를 추가적으로 받게하면 안되나요?

[jurlring]

네 말씀주신대로 추상클래스를 상속하는 것은 결합도가 엄청 높은 작업이에요.
그렇다보니 요구사항이 변경된다면 수정이 어렵지요!

플레이어는 카드를 꺼내는 행위를 하는데 상속받는 딜러는 불필요한 행위인데 할 수 있게 되어 있는 것으로 보이는데요
딜러가 불필요한 행위를 할 수 있게 열어두는게 맞을까요?

[jurlring]

도메인이 DTO를 알아도 될까요? 도메인과 DTO에 대해서 각각 어떻게 생각하시는지 궁금해요

[ParkJiYeoung8297]

피드백 주셔서 한번 생각해보니, 도메인에서 DTO를 모르는 것이 좋을 것 같습니다!

DTO는 전송 객체이며, 변경이 빈번할 수 있습니다. (ex. 고객의 요청으로 카드 그림 값도 추가로 보내줘야한다면, dto가 변경된다.)
그에 반해, 도메인은 최대한 캡슐화되어있고, 변경에 영향이 적어야한다고 생각합니다.
dto가 변경될 때마다, 도메인을 수정해줘야한다면 문제가 생길 수 있을 것 같아요.

도메인에서는 값을 넘겨주고, 서비스에서 dto를 직접 생성하는 것이 좋을 것 같습니다.

이에 대해 주디는 어떻게 생각하시는지 궁금합니다!

[jurlring]

반영이 되지 않은 것 같아요 😿

[jurlring]

출력과 관련된 행위를 controller 가 하고 있네요 😿

[ParkJiYeoung8297]

OutputView의 책임인데, Controller가 하고 있네요.😅 수정하겠습니다!