-
Notifications
You must be signed in to change notification settings - Fork 467
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
[2단계 - 블랙잭 베팅] 비타(오연택) 미션 제출합니다 #873
base: taek2222
Are you sure you want to change the base?
Conversation
- 결합도를 낮추기 위해,일급 컬렉션로 파라미터 인자를 전달로 변경했습니다.
public class GameManager { | ||
|
||
private final CardPack cardPack; | ||
private final Players players; | ||
|
||
public GameManager(final CardPack cardPack, final Players players) { | ||
public GameManager(final CardPack cardPack, final List<Gambler> gamblers) { |
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.
GameManager
객체 생성할 때, 카드 초기 2장 발급 로직 때문에 인자를 List<Gambler> gamblers
로 받았습니다. 혹시, 생성자에서 일급 컬렉션이 젹용이 안된 현재와 같이 컬렉션 형태로 받아도 괜찮을까요? 🤔
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.
GameManager가 List를 받아 직접 Players를 생성하고 있어서, Players를 직접 받는 것이 더 객체지향적일 것 같아요 :)
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.
승패를 판정하는 로직은 원래 Players
클래스에 포함되어 있었습니다. 그러나 Players
클래스가 승패 판정까지 담당할 필요는 없다고 판단하여, 이를 다음과 같이 구성하였습니다. 응집도를 높이기 위해 개선을 시도했는데, 이 방향이 적절한지 궁금합니다. 🤔
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.
승패 판정은 어디에서 담당해야 할까?
에 집중하여 개선해주셨네요.
Players 클래스는 딜러와 플레이어 관리에 집중하고,
GameResult는 게임 결과를 판단하고 저장하는 역할로 보았다면 적절하다고 생각합니다 👍
public List<Card> getOpenedCards() { | ||
if (this.getCards().isEmpty()) { | ||
if (this.getHand().getCards().getCards().isEmpty()) { | ||
return List.of(); | ||
} | ||
return getCards().subList(0, 1); | ||
return getHand().getCards().getCards().subList(0, 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.
View에서 사용되는 데이터이므로 디미터 법칙
을 준수하지 않았습니다. 이 경우에도 규칙을 적용하는 것이 맞을까요?
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.
프로덕션 코드 전체에 같은 규칙을 적용해야 한다고 생각합니다 :)
View에서 사용되는 데이터라면 디미터 법칙을 준수하지 않아도 된다고 생각한 이유가 궁금합니다
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단계도 잘 구현해주셨네요
질문주신 내용과 몇 가지 코멘트 남겼습니다
확인부탁드려요 :)
- 객체 파라미터 제한에 대한 결정
코드에 작성해주신 근거가 명확하고 현재 선택이 제어와 가독성 측면에서 적절하게 작성해주셨다고 생각합니다 :)
- 디미터 법칙 (높은 응집도, 낮은 결합도) 개선, 객체 간 메시지 전달
디미터 법칙을 엄격하게 적용하면 중간 다리 역할을 하는 메서드가 많아질 수 있다고 생각합니다. 다만, 이어지는 메서드 체인이 너무 길다면, 객체의 역할이 지나치게 분산된 것일 수도 있다고 생각합니다.
또한 이 원인으로 관리가 어려워지면, “이 객체가 직접 처리할 수 있는 역할인가?“를 고민해 일부 기능을 내부에서 처리하도록 리팩토링할 필요가 있을 것 같아요.
객체지향 생활체조 원칙
한 줄에 점을 하나만 찍는 것도 지켜보면서 개선해볼까요?
- 테스트 코드 작성
public과 private 배치와 비슷한 고민이라고 생각이 드네요! 일반적으로 해당 테스트 바로 아래에 배치하는 방법을 선택할 것 같아요.
OutputView outputView = new OutputView(); | ||
|
||
BlackjackController controller = new BlackjackController( | ||
new InputView(), | ||
new OutputView() | ||
outputView | ||
); |
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.
InputView
도 OutputView
와 같이 생성 후 주입으로 맞추어볼까요?
try { | ||
controller.start(); | ||
} catch (RuntimeException e) { | ||
outputView.printErrorMessage(e.getMessage()); | ||
} | ||
} |
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.
RuntimeException
만 잡고 있네요. 그 외의 에러가 발생하는 경우 처리가 없는 것 같아요
List<Gambler> gamblers = players.getGamblers(); | ||
for (Gambler gambler : gamblers) { | ||
while (!gameManager.isPlayerBust(gambler) && inputView.readOneMoreDealCard(gambler)) { |
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의 조건부분을 메서드로 분리하면 좋을 것 같아요
public class GameManager { | ||
|
||
private final CardPack cardPack; | ||
private final Players players; | ||
|
||
public GameManager(final CardPack cardPack, final Players players) { | ||
public GameManager(final CardPack cardPack, final List<Gambler> gamblers) { |
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.
GameManager가 List를 받아 직접 Players를 생성하고 있어서, Players를 직접 받는 것이 더 객체지향적일 것 같아요 :)
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.
승패 판정은 어디에서 담당해야 할까?
에 집중하여 개선해주셨네요.
Players 클래스는 딜러와 플레이어 관리에 집중하고,
GameResult는 게임 결과를 판단하고 저장하는 역할로 보았다면 적절하다고 생각합니다 👍
System.out.println(player.getName() + "는 한장의 카드를 더 받겠습니까?(예는 y, 아니오는 n)"); | ||
String input = console.nextLine(); | ||
System.out.println(player.getName().getName() + "는 한장의 카드를 더 받겠습니까?(예는 y, 아니오는 n)"); | ||
String input = CONSOLE.nextLine(); | ||
validateYesOrNo(input); | ||
|
||
return input.equals(YES); |
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.
return input.equals(YES); | |
return input.equalsIgnoreCase(YES); |
깨알같지만 대분자 Y
도 받을 수 있게될 것 같아요 ㅎㅎ
public void printDealerHitAndDealCard() { | ||
System.out.println("\n딜러는 16이하라 한장의 카드를 더 받았습니다."); | ||
System.out.println(NEW_LINE + "딜러는 16이하라 한장의 카드를 더 받았습니다."); | ||
} |
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.
만약 16이라는 기준이 바뀐다면 테스트코드, 출력코드 모두 수정되어야할 것 같아요
String cards = player.getHand().getCards().getCards().stream() | ||
.map(this::formatCardMessage) | ||
.collect(Collectors.joining(", ")); |
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.
여기도 디미터법칙을 준수해봅시다
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
@DisplayName("참가자 테스트") | ||
public class GamblerTest { |
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.
junit5는 public 접근 제어자를 사용하지 않는 편입니다
|
||
@Test | ||
@DisplayName("게임 참가자는 모든 카드를 공개한다") | ||
void test() { |
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.
테스트명이 test
네요!
체크 리스트
test
를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
Step 1
이후로 계속해서 리팩토링을 진행했으며,객체지향 생활체조 요구사항
의 내용은 전체적으로 점검해 위와 같은 점수를 부여했습니다! 📚어떤 부분에 집중하여 리뷰해야 할까요?
객체 파라미터 제한에 대한 결정 🤔
부모 클래스:
Player.class
, 자식 클래스:Dealer.class
,Gambler.class
Step 1 리뷰에서 제기된 질문을 고민한 끝에, 객체 생성 시 타입을 명확하게 지정하는 방향으로 결정했습니다. ✅
여러 크루의 의견을 수렴한 결과, 다형성을 최대한 활용하기 위해서는 부모 클래스를 활용하는 것이 이상적이지만, 최종적으로 적절한 제한을 두는 것이 프로그램의 가독성과 제어 측면에서 더 유리하다고 판단했습니다. 🤔
따라서 부모 클래스가 아닌 자식 클래스에서 타입을 명시적으로 지정하는 방식을 선택했습니다. 이러한 결정에 대한 전체적인 코드의 피드백을 받고 싶습니다.☺️
디미터 법칙 (높은 응집도, 낮은 결합도) 개선 ⚙️
객체 지향 프로그래밍의 원칙을 보다 자연스럽게 적용하기 위해, 이번
Step2
에서디미터 법칙
을 준수하며 구조를 개선했습니다. 이를 통해 전체적으로디미터 법칙
을 잘 지켰는지 검토받고 싶습니다. 🤔또한,
디미터 법칙
을 적용하는 과정에서높은 응집도
와낮은 결합도
를 유지하도록 리팩토링을 진행했습니다. 이에 따라, 클래스 간의응집도
와결합도
가 객체 지향 설계에 적절하게 반영되었는지에 대한 피드백을 받고 싶습니다. 🙇🏻예시로는
GameManager
클래스에서 -> ... -> ... ->Cards
카드 발급 로직의 중간 로직이 상당히 길다고 느껴집니다.객체 간 메시지 전달 ✉️
디미터 법칙을 준수하기 위해 객체 간에
getter
사용을 최소화하고, 직접 메시지를 주고받도록 설계했습니다. 이를 구현하면서 계층 간 단순한 메시지 전달을 위한 기본적인 메서드들이 증가했습니다. 결과적으로, 중간 다리 역할을 하는 메서드가 많아졌는데, 이러한 현상이 객체 간 메시지 전달 방식에서 자연스러운 흐름인지 궁금합니다. 🤔에 대한 피드백을 받고 싶습니다.☺️
테스트 코드 작성 📚
테스트 코드 작성에 관심이 많습니다. 테스트 코드 구성에 대한 피드백을 주시면 감사하겠습니다. 또한,
@MethodSource
를 자주 사용하는데, 해당 메서드를 어디에 배치하는 것이 적절할까요? 구현한 테스트 코드 바로 아래에 두는 것이 좋을까요, 아니면 파일의 가장 아래에 위치시키는 것이 더 적절할까요? 🤔소중한 시간을 내어 리뷰해 주셔서 진심으로 감사드립니다. 🙇🏻
부족한 부분이 있다면 깨닫고 성장할 수 있도록 적극적으로 피드백을 받아들이겠습니다.
세세한 부분까지 짚어주시면 충분한 학습을 거쳐 개선해 나가겠습니다. 🎓