Step3 - 볼링 점수판(리팩토링) 리뷰 요청드립니다. #44
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javajigi
approved these changes
Jul 26, 2019
| @@ -6,6 +6,8 @@ | |||
| static final String ALERT_OUT_OF_PINS_RANGE = "쓰러진 핀의 개수는 최소 0개에서 최대 10개 입니다."; | |||
| public static final int STRIKE_PINS = 10; | |||
| public static final int GUTTER_PINS = 0; | |||
| public static final Pins STRIKE = Pins.from(STRIKE_PINS); | |||
| public static final Pins GUTTER = Pins.from(GUTTER_PINS); | |||
There was a problem hiding this comment.
저는 이런 상수 값들이 지금과 같이 의존관계가 높은 Pins와 같은 객체에 있는 것이 더 좋다고 생각해요.
|
|
||
| public static Score ofStrike() { | ||
| return new Score(STRIKE_PINS, 2); | ||
| } |
There was a problem hiding this comment.
이와 같은 정적 팩토리 메소드 또한 api를 사용하는 입장에서는 유용한 메소드라 생각해요. 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요!
2단계 빠르게 리뷰해주셔서 감사합니다!
3단계에서는 제 눈에 밞히는 것들을 수정했는데, 대부분 정적팩토리 메서드를 추가한 것입니다.
혹시나 더 개선할 부분이 있다면 조언부탁드리겠습니다-!!
그 외에 고민했던 것은,
프로젝트 이곳 저곳에 비슷한 상수들이 조잡하게(?) 사용되는 것 같아서
상수 유틸리티 클래스 혹은 열거 타입 사용을 고민했습니다.
하지만 해당 클래스와 강하게 결합되는 경우가 많은 것으로 판단해서 (틀린 판단일 수 있습니다.)
고민하다가 해당 클래스 내에 존재하도록 내버려두었습니다..
비즈니스에 따라 다르겠지만 볼링 게임만 고려할 경우에
각각의 클래스에 흩뿌려진 상수를 한 곳에 모아서 관리하는 것이 좋을지,
지금처럼 강한 결합을 보이는 각각의 클래스에 두는 것이 좋을 지 궁금합니다..!
바쁘시겠지만 리뷰 부탁드리겠습니다-! 감사합니다ㅎ