[🚀 사이클1 - 미션 (블랙잭 게임 실행)] 라텔 미션 제출합니다.#1024
[🚀 사이클1 - 미션 (블랙잭 게임 실행)] 라텔 미션 제출합니다.#1024bin1225 wants to merge 75 commits intowoowacourse:bin1225from
Conversation
- 멤버변수를 가질 필요가 없으므로 유틸리티 클래스로 변경하여 생성 로직 간소화
- 메서드 기능을 직관적으로 드러내도록 수정
|
|
||
| import java.util.Arrays; | ||
|
|
||
| public enum DrawCardIntetion { |
There was a problem hiding this comment.
플레이어에게 카드를 더 받을지 물어볼 때 받는 응답값을 정의한 enum 클래스입니다.
There was a problem hiding this comment.
플레이어가 입력하는 문자열을 enum 타입으로 관리할 이유가 있을까요~?
입력값은 변경 가능성이 높고 예측 불가능하기 때문에 enum 타입으로 관리하기 적절치 않아보이기도 하네요!
There was a problem hiding this comment.
좋은 의견 감사합니다.
입력하는 문자열 (y, n) 을 감싸고 관련된 검증과 처리를 담당하는 목적으로 DrawCardIntetion Enum 클래스를 정의했던 것 같습니다.
Enum 타입을 사용한 이유는 내부 값의 종류가 한정적이라고 생각했기 때문이에요. 코드 작성할 때 Enum을 사용하는 기준을 값의 종류가 한정적일 때로 잡았었습니다.
당장은 카드를 뽑는 의사 표현이 y,n으로 한정되지만, 추후 다른 형태로 바뀔 수 있으니 Enum이 아니라 조금 더 유연한 VO로 구현하는 게 조금 더 적절하다고 이해했습니다.
제가 이해한 게 맞는지 궁금합니다!
| public static GameResultAnalysisDto analyze(Players players, Dealer dealer) { | ||
|
|
||
| List<PlayerGameResult> playerGameResults = players.stream().map(player -> { | ||
|
|
||
| if (player.isBusted()) { | ||
| return PlayerGameResult.of(player, GameResult.LOSS); | ||
| } | ||
|
|
||
| if (dealer.isBusted()) { | ||
| return PlayerGameResult.of(player, GameResult.WIN); | ||
| } | ||
|
|
||
| int dealerResultScore = dealer.getResultScore(); | ||
| GameResult gameResult = GameResult.judge(dealerResultScore, player.getResultScore()); | ||
| return PlayerGameResult.of(player, gameResult); | ||
| }).toList(); | ||
|
|
||
| return GameResultAnalysisDto.from(playerGameResults); | ||
| } |
There was a problem hiding this comment.
이 메서드를 개선하려고 했을 때 if문이 연속적으로 사용된 게 지저분하게 느껴졌습니다.
그래서 dealer와 player의 busted 여부에 따라 승패 결과를 초기화하여 PlayerGameResult가 반환되면 좋을 것 같다고 생각했습니다. 하지만 PlayerGameResult 객체는 데이터를 전달하기 위해 추가했기 때문에, 계산 로직 추가 시 전달과 함께 승패 결과 처리 책임도 담당하게 되는 것 같습니다.
메서드 개선을 위해 PlayerGameResult에 메서드를 추가할지, 아니면 현재 클래스(GameResultAnalyzer)에서 메서드를 분리할지, 또는 설계의 변경을 고려할지 고민이 됩니다. 어떤 방향으로 고민해보는 게 좋을까요?
There was a problem hiding this comment.
GameResultAnalyzer 를 게임 결과를 계산하는 책임을 가진 객체로 보셨다면 해당 클래스 내에서 별도 판정 메서드를 두어도 괜찮아보이네요!
There was a problem hiding this comment.
게임 결과를 판정하기 위한 목적으로 GameResultAnalyzer 를 정의했기 때문에, 말씀하신대로 내부에서 메서드 책임 분리하는 방향이 맞는 것 같습니다.
오히려 GameResult 에 있는 Player, Dealer의 점수를 기준으로 승패를 판정하는 judge 메서드가 GameResultAnalyzer 로 이동하는 게 맞는 것 같아서 수정했습니다.
NewWisdom
left a comment
NewWisdom
left a comment
There was a problem hiding this comment.
라텔 안녕하세요~! 이번 미션 리뷰어로 함께하게 된 아마찌라고 합니다 🙂
미션 아주 깔끔하게 잘 구현해주셨네요!
코멘트를 잘 남겨주셔서 어떤 부분을 고민하셨는지 확인하기 수월했네요 :)
몇가지 사소한 코멘트와 질문에 대한 답변 남겨놓았는데 한번 확인해보시고 라텔의 의견대로 반영해주시면 좋을 것 같습니다 :)
궁금하신 사항은 언제든지 코멘트로 편히 남겨주세요!
이번 미션 끝까지 화이팅입니다👏
| public record BlackjackGameConfiguration(ApplicationView view, CardDeckInitializer gameCardDeckInitializer) { | ||
|
|
||
| public BlackjackGameConfiguration() { | ||
| this(ApplicationView.of(new InputReader(), new OutputWriter()), new DefaultCardDeckInitializer()); |
There was a problem hiding this comment.
InputReader, OutputWriter 등 유틸성 클래스들은 메서드들을 불필요하게 객체 생성을 하지 않고 static 으로 만들어 사용해보아도 괜찮을 것 같아요
There was a problem hiding this comment.
사실 이전까지는 View 관련 기능을 정적 클래스로 정의하고 사용했습니다.
그런데 이번 페어 프로그래밍 과정에서, View를 Util 클래스처럼 두고 어디서든 접근 가능하게 하면 Controller 외의 도메인 계층에서도 View를 호출하게 되어 결합이 생길 수 있지 않냐는 질문을 받았고, 그 부분에 대해 명확하게 답하지 못했습니다.
혹시 입출력 관련 기능은 Controller에서만 접근한다고 정했을 때도 입출력 메서드를 static으로 만드는 장점이 있을까요?
지금 떠오르는 장점은 인스턴스를 생성하지 않고 바로 사용할 수 있어서 객체 생성이 필요없어진다는 점인 것 같습니다.
추가로 이 부분에 대해서 View 관련 객체를 생성해서 사용하는 코드를 작성한 크루들에게 이유를 물었을 때,
입출력 기능은 콘솔 기반에서 변경될 수 있기 때문에, 변경 시 구현체 주입만 수정하여 기능을 변경할 수 있다는 장점이 있다는 답을 들었습니다.
추상화를 적용한다면 충분히 이유가 될 수 있겠다고 생각했습니다.
리뷰어님은 어떻게 생각하시는지 궁금합니다.
| public static GameResultAnalysisDto analyze(Players players, Dealer dealer) { | ||
|
|
||
| List<PlayerGameResult> playerGameResults = players.stream().map(player -> { | ||
|
|
||
| if (player.isBusted()) { | ||
| return PlayerGameResult.of(player, GameResult.LOSS); | ||
| } | ||
|
|
||
| if (dealer.isBusted()) { | ||
| return PlayerGameResult.of(player, GameResult.WIN); | ||
| } | ||
|
|
||
| int dealerResultScore = dealer.getResultScore(); | ||
| GameResult gameResult = GameResult.judge(dealerResultScore, player.getResultScore()); | ||
| return PlayerGameResult.of(player, gameResult); | ||
| }).toList(); | ||
|
|
||
| return GameResultAnalysisDto.from(playerGameResults); | ||
| } |
There was a problem hiding this comment.
GameResultAnalyzer 를 게임 결과를 계산하는 책임을 가진 객체로 보셨다면 해당 클래스 내에서 별도 판정 메서드를 두어도 괜찮아보이네요!
|
|
||
| import java.util.List; | ||
|
|
||
| public class CardBundleTest { |
There was a problem hiding this comment.
현재 테스트 코드에 중복되는 초기화 로직이 많다고 느꼈습니다. 이런 경우 객체 설계를 다시 고민해야 하는 것인지 궁금합니다. 또한 테스트 코드의 가독성과 중복을 개선할 수 있는 작성 방법이 있다면 알고 싶습니다.
이런 경우가 항상 객체 설계의 문제라고 보기는 어렵고, 테스트 데이터를 준비하는 과정에서 자연스럽게 드는 고민인 것 같아요!
테스트 코드의 중복을 줄이기 위해 test fixture를 활용하거나 @beforeeach 등을 사용해서 공통 초기화 로직을 분리하는 방법도 충분히 고려해볼 수 있을 것 같은데요,
괜찮다면 이러한 방법들에 대해 한 번 학습해보고 적용해보셔도 좋을 것 같네요 :)
테스트 코드에서 정확한 검증을 위해 추가 코드가 필요하다고 느낀 경우가 있었습니다. (예: getter와 같은 조회 메서드) 테스트만을 위한 추가 코드 작성에 대해 리뷰어님의 의견이 궁금합니다. 이런 경우 설계 문제인지, 아니면 테스트를 위해 필요한 코드 추가가 권장되는지 알고 싶습니다.
테스트를 작성하다 보면 말씀해주신 것처럼 검증을 위해 getter와 같은 조회 메서드가 필요하게 되는데요, 개인적으로는 테스트를 위해 어느 정도의 조회 메서드를 추가하는 것은 크게 문제되지 않는다고 생각합니다!
다만, 현재 객체가 외부에서 확인할 수 있는 상태를 적절히 제공하고 있는지 한 번 정도는 점검해보는 것도 도움이 될 것 같습니다. 경우에 따라서는 테스트를 위해 메서드를 추가하기보다는, 객체의 행위를 통해 상태를 검증할 수 있도록 설계를 개선할 여지가 있는 경우도 있는 것 같아요 :)
그래서 보통은
• 먼저 객체의 public 행위를 통해 검증할 수 있는지 고민해보고,
• 그래도 필요하다면 조회 메서드를 추가하는 방식도 도움이 될 것 같습니다!
- 메서드명에 변환 의도가 드러나도록 수정
- BlackjackGame 객체에서 Players를 생성하지 않고, PlayerName 리스트를 넘겨주는 방식으로 수정 - 관련 테스트 검증 방식 개선을 위해 도메인에 equals 오버라이딩
- 클래스와 변수가 명확히 구분되도록 하여 가독성 개선
- 가독성 향상, 클래스와 변수가 명확히 구분할 수 있게 수정
3 participants
안녕하세요 아마찌, 8기 백엔드 수강중인 라텔입니다!
많이 부족한 코드지만 잘 부탁드립니다. 피드백 해주시면 최대한 이해하고 반영하겠습니다.
코드 관련해서 추가적으로 궁금하다고 느끼는 부분과 아마찌에게 설명할 필요가 있다고 생각되는 부분은 코멘트 추가해뒀으니 확인 부탁드립니다! 추가로 설명이 필요하다고 느끼시는 부분은 질문 주시면 최대한 빠르게 답변하겠습니다.
미션 진행 과정에서 생각하고 느낀 점입니다.
페어와 함께 객체의 책임을 분리하고 중복을 줄일 방법을 고민하면서, 직관적으로 생각할 수 있는 객체 역할 분리와 메서드 책임 분리는 적용했다고 생각합니다.
player 리스트를 일급 컬렉션으로 감싸 캡슐화했지만, 구현 과정에서 내부 리스트에 직접 접근이 필요하다고 판단해 stream 반환 메서드를 추가했습니다. 이 과정에서 player 리스트에 대한 처리를 일급 컬렉션이 완전히 담당하지 못하게 된 것 같고, 일급 컬렉션을 정의한 목적이 흐려진 것 같습니다.
구현하면서 Player와 Dealer가 공통적으로 가지는 기능이 많다고 느꼈는데, 이로 인해 발생하는 중복 문제는 아직 해결하지 못했습니다. 공통 기능을 가진 클래스를 정의하고, 상속을 통해 개선할 계획입니다.
DTO의 출력 형식을 만드는 기능이나 시스템에서 사용하는 상수(버스트 판정 기준 = 21) 관리처럼 어떤 객체가 책임을 가져야 할지 명확히 판단되지 않는 부분들이 있었습니다. 그래서 책임 분리가 명확하지 않거나 일관성이 부족한 코드가 일부 있는 것 같습니다.
제가 생각했을 때 리뷰가 필요한 부분입니다!
각 클래스에서 책임 분리가 필요한 부분이 있다면 알고 싶습니다. 또 추상화가 추가로 적용되면 좋을만한 부분들이 있다면 알고 싶습니다.
이번 미션에서 일급 컬렉션 적용을 위해 CardDeck, Players 클래스를 정의했습니다. 이론 상으로는 일급 컬렉션이 내부 컬렉션 처리를 담당하고 캡슐화 하기 위함이라고 알고 있지만, 실제로 잘 사용한 것인지 확신이 생기지 않습니다. 일급 컬렉션에 정의되어야 하는 기능이 외부에 존재하는 부분이 있다면 알고싶습니다.
현재 테스트 코드에 중복되는 초기화 로직이 많다고 느꼈습니다. 이런 경우 객체 설계를 다시 고민해야 하는 것인지 궁금합니다. 또한 테스트 코드의 가독성과 중복을 개선할 수 있는 작성 방법이 있다면 알고 싶습니다.
테스트 코드에서 정확한 검증을 위해 추가 코드가 필요하다고 느낀 경우가 있었습니다. (예: getter와 같은 조회 메서드) 테스트만을 위한 추가 코드 작성에 대해 리뷰어님의 의견이 궁금합니다. 이런 경우 설계 문제인지, 아니면 테스트를 위해 필요한 코드 추가가 권장되는지 알고 싶습니다.
출력 계층에서 사용하기 위한 각 객체의 Dto들을 추가하고 사용했습니다. 작성 후 리팩토링 하는 과정에서 Dto 관련 객체의 구조와 사용이 난해하다는 느낌을 받았는데, 명확한 이유를 정의하기가 힘들었고 개선 방향성을 잡지 못했습니다. 리뷰어님이 보시기에 문제점이나 개선 방향성이 존재한다면 알고싶습니다.