[1단계 - 블랙잭 구현] 에어(김준서) 미션 제출합니다. #141
Conversation
|
안녕하세요 지노! 말씀해주신 피드백 적용해보았습니다!! 답변해주신 것에 대한 추가 궁금증1.답변 주신 VO 관련해서 고민해봤어요! Cards 일급 컬렉션의 경우 카드가 같아도 다른 Cards가 될 수 있다고 생각했지만, 이 블랙잭 게임의 경우 카드가 52장만 존재하니까 같은 카드를 가진 Cards는 서로 같게 봐야한다고 판단했어요. 그렇게 해야 Participant 추상 클래스가 Nickname과 Cards를 가지고 있는데, Nickname과 Cards가 둘 다 같아야지 같은 Participant로 볼 수 있다고 생각했어요. 다만 Players 같은 경우에는 같은 Player들이 있더라도 각각 다른 모임?으로 볼 수 있을 것 같아서 VO로 판단하지 않았습니다! 2.의존성 주입에 대해 공부해봤습니다. 외부에서 객체를 생성해서 주입해주면 클래스를 객체 생성과 독립적으로 만들 수 있습니다. 클래스 내부에서 객체를 생성하게 된다면 클래스의 수정없이 독립적으로 인스턴스 생성을 변경할 수 없게되어 유연하지 못하고 재사용할 수도 없게됩니다! 따라서 Players 외부에서 List를 만들어 준 후, Players에 주입해주면 후에 다른 형식의 input이 들어와도 Players의 코드는 변경 없이 List로 만들어주는 부분만 수정하면 된다고 생각해요! 3.view에서 도메인 객체의 메서드를 호출하는 식으로 작성된 코드를 어떻게 바꿔야하나 고민하다가 dto를 적용해보라는 뜻으로 이해를 해서 적용해보았습니다. dto를 한 번도 써본 적이 없어서 제대로 적용한지는 잘 모르겠네요 ㅠㅠ dto의 멤버변수를 어떤 타입으로 둬야할지를 잘 모르겠더라고요. 1)번 방법으로 하려니 도메인 객체에서 getter를 써야해서 현재는 2)번 방법으로 했어요! 또 현재 각 도메인에서 toXXXDto 라는 메서드를 만들어서 Dto로 변환해주고 있는데 이렇게 사용해도 되는건지 궁금해요! 4.테스트가 더 필요한 부분에 대하여 고민해보고 몇가지 테스트들을 추가했어요! 혹시 부족한 부분이 있으면 말해주세요! 새로운 궁금증추가적으로 궁금한 것이 생겼어요! Q1. 처음 service를 분리한 것이 controller가 너무 커지는 것 같아서였어요! 그래서 View와 연결되지 않은 메서드들만 service로 분리했었는데, 이런 로직을 controller에서 처리해주면 controller가 너무 길지는 않을까요? 페어에게서 Controller를 게임 세팅 컨트롤러, 게임 진행 컨트롤러 등으로 나누는 것은 어떠냐는 의견도 들었는데 어떻게 생각하시나요?? 아니면 아에 다른 도메인 객체를 만들어야할까요?(이 경우는 시도해보지 않아서 잘 모르겠지만 controller에서 get을 사용해야할거 같다는 생각도 들긴하네요ㅠㅠ) Q2. 현재 CardDeck의 변수가 Cards 하나 밖에 없는 상태에요. 이러다보니 굳이 클래스를 나눠야하나? 라는 생각이 들기도 해요. Q3. 코딩 컨벤션을 찾아봤는데 이렇게 나와있었어요.
보통 toString, equals, hashCode가 제일 하단. 그 위에 getter까지는 들은 적이 있는데 static 메서드와 일반 메서드가 같이 존재할 경우 static 메서드는 따로 위로 빼줘야할까요 아니면 상관없이 기능에 따라 배치하면 될까요? Q4. 만약 정적 팩토리 메서드를 사용한다면 정적 팩토리 메서드 내부에서 검증을 해줘야 할까요? 아니면 생성자에서 검증을 해줘야할까요? 생성자와 정적 팩토리 메서드의 차이에 대해 공부하다가 크루에게 그런데 만약 인수가 다른 두 정적팩토리 메서드가 있을 때 각각 정적 팩토리 메서드에서 검증을 해주게되면 중복이 발생한다고 생각이 들어서 생성자에서 검증을 해주는 것이 좋지 않나라는 생각이 들었어요. 혹시 지노는 어떻게 생각하시나요? 예시(이번 미션 코드와 상관없는 그냥 예시입니다!) // 정적 팩토리 메서드에서 검증
private Card(String name, int number) {
this.name = name;
this.number = number;
}
public static Card valueOf(String name) {
validateName();
return new Card(name, 0);
}
public static Card valueOf(String name, int number) {
validateName();
return new Card(name, number);
}// 생성자에서 검증
private Card(String name, int number) {
validateName();
this.name = name;
this.number = number;
}
public static Card valueOf(String name) {
return new Card(name, 0);
}
public static Card valueOf(String name, int number) {
return new Card(name, number);
}Q5. 쓰다보니 질문이 너무 많아진 것 같네요 😢 😢 |
|
+) 위에서 피드백 적용한 것과 별개로 오늘 수업 시간에 배운 상태 패턴을 추가로 적용해 봤어요. [디자인패턴] 상태패턴 - 3내용
링크[디자인패턴] 퍼사드패턴 - 2내용
링크[DI] 의존성 주입 - 3내용
링크 |
|
안녕하세요 에어 지노입니다! Dto를 적용해보자 라는 말은 아니였는데...🙄 개인적으로 레벨 1에서 dto까지는 필요 없다고 느껴요!
VO를 Dto의 필드에 뒀다는 말씀 같아요! VO는 불변이겠죠? 불변객체는 상태가 변하지 않기 때문에 사이드 이펙트가 없습니다! view로 나가도 좋습니다 :) 굳이 모든 원시값을 dto의 필드에 둘 필요는 없어요!
controller가 길어진다면 도메인 설계가 잘 되었는지 확인해보면 좋을 것 같아요! 적절한 도메인 객체 만들어 책임과 역할을 부여하고 컨트롤러에서는 그 도메인 객체를 사용해 비즈니스 요구사항을 풀어내면 됩니다. 그러다 도저히 안되면 controller에서 처리하면 될 것 같아요!
Deck도 List 를 가지는 일급컬렉션으로도 구현이 가능할 것 같아요 :) 그렇다면 Deck과 Cards의 역할이 좀 더 분명할 것 같네요!
찾아보니 딱히 순서는 정해진게 없나봐요! class Test {
상수
클래스 변수
인스턴스 변수
생성자
팩토리 메서드
메서드
기본 메서드 (equals, hashcode, toString...)
}
static 메서드는 어플리케이션이 종료될 때까지 메모리에 남아있습니다. 정적 팩터리 메서드에서 검증을 하게된다면 검증에 대한 메서드는 private static일 가능성일 높을 것 같아요! 😀 그렇다면 생성자에서 검증을 하는 편이 저는 더 나은 방법이라고 생각해요.
상속은 확장 이고 인터페이스는 구현입니다. 각각의 키워드가 extends, implements로 다르구요. |
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| Participant that = (Participant) o; | ||
| return Objects.equals(nickname, that.nickname) && Objects.equals(state, that.state); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(nickname, state); | ||
| } |
There was a problem hiding this comment.
Participant는 nickname, state가 모두 같아야 같은 객체로 취급하나요??
There was a problem hiding this comment.
아 state는 달라도 nickname만 같다면 동일 객체로 볼 수 있겠네요!
VO가 같은 값일 경우 같은 객체로 취급하는건데, participant는 state가 달라도 nickname만 같으면 같은 객체로 취급할 수 있잖아요. 이렇게 필드의 일부분만으로 같은 객체 판단을 하는 경우도 VO로 볼 수 있는 건가요??
VO판단 핵심을 어떤 클래스의 일부 필드가 같을 때 같은 객체로 볼 수 있는 경우로 생각해도 될까요? 🤯
There was a problem hiding this comment.
아닙니다! VO의 동등성은 모든 필드의 값이 같아야 합니다.
주소로 비교를 하자면 아파트 이름이 같으면 다 같다고 볼 수 없는 것과 같습니다.
아파트 이름, 동, 호수 까지 같아야 같은 주소로 취급합니다 :)
| boolean isDuplicate = players.stream() | ||
| .distinct() | ||
| .count() != players.size(); |
| public final ParticipantDto toParticipantDto() { | ||
| Cards cards = state.getCards(); | ||
| return new ParticipantDto(this.nickname, cards, cards.calculateScore()); | ||
| } |
There was a problem hiding this comment.
도메인 모델이 dto의 존재를 알고 있네요!
dto에 변화가 생기면 도메인 모델도 변경 해줘야 할 것 같아요. 도메인 모델은 Dto를 모르는 편이 나을 것 같습니다! :)
| public List<ParticipantDto> toPlayersDto() { | ||
| return players.stream() | ||
| .map(Player::toParticipantDto) | ||
| .collect(Collectors.toList()); | ||
| } |
| public Map<MatchResult, Integer> getDealerResult() { | ||
| Map<MatchResult, Integer> dealerResult = new EnumMap<>(MatchResult.class); | ||
| for (MatchResult matchResult : MatchResult.values()) { | ||
| dealerResult.put(matchResult, dealerResultCount(matchResult)); | ||
| } | ||
| return dealerResult; | ||
| } |
There was a problem hiding this comment.
테스트를 제외한다면 이 클래스 내부에서만 사용하는 메서드네요. private이 더 어울릴거 같아요.
그리고 테스트에선는 이 메서드를 사용하는 다른 메서드를 통해 간접적으로 테스트하면 더 좋을 것 같습니다 :)
There was a problem hiding this comment.
toDealerResultDto() 에서만 사용되네요.
BlackJackResult를 playerResult, dealerResult로 분리하는 것도 방법일 것 같아요 :)
고민만 해주세요!
| if (playerState instanceof BlackJack && !(dealerState instanceof BlackJack)) { | ||
| return true; | ||
| } | ||
| if (playerState instanceof Stay && dealerState instanceof Bust) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
instanceof는 OOP에서 다형성으로 해결 가능합니다. isBlackJack() 같은 메서드도 만들수 있을 것 같네요. 다형성과 OCP에 대해서 학습해보면 좋을 것 같습니다 :)
|
|
||
| public class DealerTest { | ||
| private Dealer dealer; | ||
|
|
| state.draw(new Card(Shape.DIAMOND, Denomination.KING)); | ||
| }).isInstanceOf(IllegalStateException.class); | ||
| } | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
마지막줄은 개행으로 끝납니다 :) 다른 클래스들도 확인 부탁드려요!
안녕하세요 지노!! 이번 미션을 함께하게 된 에어라고 해요! 잘 부탁드립니다 😄
이번 미션을 진행하는데 시간에 맞춰서 하기가 정말 쉽지 않네요.. 시간 맞춰 하려 하다보니 스스로 고칠 것이 너무 많이 보이는 것 같아요ㅠㅠ
TDD가 감이 오는 듯 했는데 이번 미션 진행하면서 다시 감이 흐려진 느낌이네요 ㅠㅠ 이번 코드 리뷰를 통해서도 많이 배울 것을 기대하고 있습니다! ㅎㅎ 많은 피드백 부탁드려요~~
이해해주신 덕분에 코드를 조금 다듬긴 했는데 좋은 코드라는 생각은 들지 않네요 ㅠㅠ 리뷰를 통해 좋은 코드로 변화시켜보고 싶습니다. 많은 힌트와 리뷰 부탁드릴게요!! 👍🏻
궁금증
이번 미션을 진행하면서 페어랑 이야기해보면서 많은 토론을 했는데 물어보고 싶은게 많아요
Q1. 객체 생성 테스트가 궁금해요. 페어랑 생성 테스트에 대해 이야기해봤는데, isNotNull로 객체가 생성된 것만 확인해도 생성 테스트가 되지 않느냐는 의견을 들었어요. 여태까지는 isEqualTo를 사용해서 생성테스트를 했었는데 isNotNull로만 생성 테스트를 해줘도 괜찮을까요?
Q2. 카드덱에서 카드를 뽑으면서 동시에 제거를 해주려고 했는데, List를 포장한 일급컬렉션인 Cards를 불변 객체로 하려다가 보니 CardDeck에서 카드를 한 장 뽑을 때, 뽑을 카드를 확인하는 메서드와 실제로 CardDeck에서 카드를 제거하는 메서드가 분리되게 되었어요. 이렇게 분리해서 사용하는 게 괜찮을까요?
Q3. 카드 점수 계산에 관해서 질문이있어요. 여러가지 방법이 있을 것 같은데
현재는 1)번 같이 구현했는데 계속해서 중복된 계산을 하게 될 것 같아서 지노의 의견을 듣고 싶어요
Q4. 마찬가지로 점수 계산 관련 질문인데요. 저는 cards에서 계산을 해줘도 된다고 생각했는데, cards는 카드를 뽑고, 확인하고 하는 기능만 가지고 있는게 맞는 것 같다는 의견을 들었어요. 현재는 카드에서 계산하게 구현했습니다! cards에서 점수계산 하는 것이 올바른 책임이 아닌 걸까요?
Q5. 만약 inputView 내에서 try-catch를 하게 된다면 InputView에서 에러 메세지를 출력하기 위해 OutputView를 호출해도 되는 걸까요?
Q6. 현재 컨트롤러 역할을 하는 BlackJackGame의 distributeCards()와 playersTurn()에서 getter를 사용해서 하고 있는데 getter를 사용하지 않는 방법이 있을까요? 있다면 힌트 부탁드려요!
Q7. player와 dealer가 person을 상속하는데 person에서 protected 접근제한자를 사용중입니다. player와 dealer에서는 super.xx 으로 person의 멤버변수에 접근하여 사용중인데, person의 접근제한자를 private으로 두고 getter로 사용하는 것이 더 좋다는 말을 들었습니다. 어떻게 생각하시나요?
Q8. person과 같은 추상 클래스는 어떻게 테스트를 해야할까요?
Q9. view에서는 출력을 위해 getter를 사용할 수 밖에 없다고 알고 있습니다. 그런데 객체들이 너무 포장이 되어 있어서 원래 값을 꺼낼 때 getter가 너무 이어지게 되는 것 같아요 ㅠㅠ view라서 괜찮은가? 라는 생각이 들기도 하지만 너무 데메테르 법칙을 위반하는 것 같기도 하여 조언을 구하고 싶어요ㅠㅠ
Q10. getter 사용을 지양하라는 말을 많이 들었어요. 그런데 stream을 사용할 때 .map으로 객체의 어떤 타입으로 매핑하는 경우가 있는데 이런 경우에는 get을 사용해도 되는것인가요?
이 예시는 이번 미션에서 사용한 코드는 아니지만 궁금해서 여쭤봐요!
학습로그
[OOP] 상속 - 3
내용
링크
[TDD] TDD 적용 - 3
내용
링크
[JDK] Stream API - 3
내용
링크
[Java] Enum - 3
내용
링크
[OOP] 일급콜렉션 사용 - 3
내용
링크
[자료구조] LinkedHashMap - 1
내용
링크
[OOP] 원시값 포장 - 1
내용
링크