Skip to content
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단계 - 블랙잭(베팅)] 유콩(김유빈) 미션 제출합니다. #330

Merged
merged 43 commits into from
Mar 21, 2022

Conversation

kyukong
Copy link

@kyukong kyukong commented Mar 17, 2022

안녕하세요 토니!
블랙잭 2단계 미션 제출합니다

블랙잭 피드백 강의에서 접한 상태패턴 을 적용해보았습니다.
상태패턴을 처음으로 사용해보는거라 익숙해지려고 강의 구조와 동일하지만
토니가 이해하기 쉽도록 클래스다이어그램을 그려보았습니다.
간단하게 클래스 간 상속 및 구현 구조만을 나타냈습니다.

🖊😎 질문

🐥 함수형 인터페이스 이름

지난 피드백에서 제가 작성한 이름만으로는 코드가 이해하기 어려울 수 있다 고 말해주셨습니다. 그건 파라미터의 이름을 의미하는 것이 맞을까요? 일단 이름만 수정하였습니다.

다른 크루들의 의견을 듣고보니 동일하게 도메인에 뷰 함수를 전달하였지만 새로 함수형 인터페이스를 만들었더라구요. 저는 따로 클래스를 만들정도로 역할이 큰 클래스도 아니고 파라미터 이름만으로도 충분하게 의미를 전달할 수 있다고 생각하여 추가하지 않았습니다. 토니는 다음과 같이 함수형 인터페이스를 사용해야 하는 상황에서 미리 생성되어 있는 기본 함수형 인터페이스 를 사용하시나요? 아니면 커스텀 함수형 인터페이스 를 사용하시나요? 사용하신다면 또 그 이유가 궁금합니다!!!


코드리뷰를 너무 늦게 보내서 기다리셨을텐데 이해해주셔서 감사합니다!
이번에도 잘 부탁드립니다.🙇‍♀️

- Converter -> InputConverter 로 이름 수정
- bet 메소드를 호출하지 않고 다른 상태로 바뀔 경우 null 이 전달될 수 있으므로 검증 로직 추가
- 게임을 계속해서 진행할 수 있는지 판단 -> Players 에게 위임
Copy link

@toneyparky toneyparky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 유콩!

1단계 리뷰를 반영하심과 동시에 상태 패턴을 적용해보셨네요!

코드 잘 확인했고 리뷰도 완료했어요. 처음 학습하신 패턴을 이렇게 적용해보신게 멋지네요.
리뷰를 확인해주시고 궁금한 부분은 댓글이나 DM 부탁드릴게요!

화이팅입니다 😉

README.md Show resolved Hide resolved
src/main/java/blackjack/domain/card/Suit.java Show resolved Hide resolved
src/main/java/blackjack/domain/game/Gamer.java Outdated Show resolved Hide resolved
src/main/java/blackjack/domain/state/Started.java Outdated Show resolved Hide resolved
src/main/java/blackjack/view/InputConverter.java Outdated Show resolved Hide resolved
src/main/java/blackjack/view/OutputView.java Outdated Show resolved Hide resolved
src/test/java/blackjack/domain/state/HitTest.java Outdated Show resolved Hide resolved
@kyukong
Copy link
Author

kyukong commented Mar 21, 2022

토니! 마지막 코드리뷰 요청 드립니다!😂

이번 블랙잭 미션 2단계에는 아쉬운 점이 너무 많네요. 피드백을 보아하니 제가 사소한 것들을 제대로 지키지 못했던 것 같습니다.(네이밍 컨벤션, 메서드 위치, 접근 제어자 등등) 2단계에서 저만의 목표는 피드백을 적게 받는 코드를 짜보자! 였었는데 마음과는 달리 꼼꼼하게 살펴보지 못했습니다. 괜히 너무 사소한것들까지 피드백을 하게 만든것 같아서 죄송하네요...🥲

그래도 정규식, 상속 구조 설계들의 주의할 점들을 알게 되었고 새로운 패턴도 사용해보았기 때문에 아쉽긴해도 얻어가는 것도 있었던 미션이었습니다. 감사했습니다!!

Copy link

@toneyparky toneyparky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 유콩!

코멘트에 답변 달아주셔서 저도 답변과 의견을 달아봤어요. 상태 패턴을 새로 접하고 바로 적용하시느라 힘드셨을텐데 잘 마무리가 되었네요. 이번에 리팩터링하시면서 보다 깔끔하고 이해하기 쉬운 코드가 되었다고 생각해요. 질문 하나 남겼으니 확인 부탁드려용

리뷰어는 유콩의 성장을 돕기 위해 있기에 미안해하지 않으셔도 됩니다 😬
컨벤션과 관련된 부분을 잘 지키며 다음 미션도 진행해보죠! 다음 미션은 이번보다 더 재미있을거에요!
고생 많으셨어요 👍

@toneyparky toneyparky merged commit e2aa7fc into woowacourse:kyukong Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants