Merged
Conversation
…h frame to be displayed
…an entire score string
javajigi
suggested changes
Apr 12, 2018
Contributor
javajigi
left a comment
There was a problem hiding this comment.
테스트 코드도 잘 구현했고, 전체적으로는 깔끔하게 구현했네. 👍
피드백 몇 개 남긴다.
| this.playerName = playerName; | ||
| } | ||
|
|
||
| public boolean updateScores(int frameNumber, int pinsKnocked) { |
Contributor
There was a problem hiding this comment.
너무 프로그램의 상태를 변경하는 관점으로 보지 말고 볼링 공을 투구한다는 의미의 메소드 이름이면 좋겠음.
| import bowling.domain.score.Scores; | ||
|
|
||
| public abstract class Frame { | ||
| Scores scores = new Scores(); |
Contributor
There was a problem hiding this comment.
인스턴스 변수는 특별한 이유는 없는 한 private으로 구현한다.
객체 외부에서 상태 값을 임의로 변경할 수 있도록 하는 것은 캡슐화 위반이다.
부모 클래스이지만 이 인스턴스도 private으로 구현해 본다.
| return scores.isSpare(); | ||
| } | ||
|
|
||
| public abstract boolean updateScore(int pinsKnocked); |
Contributor
There was a problem hiding this comment.
너무 프로그램의 상태를 변경하는 관점으로 보지 말고 볼링 공을 투구한다는 의미의 메소드 이름이면 좋겠음.
| } | ||
|
|
||
| public boolean isSpare(int frameNumber) { | ||
| return frames.get(frameNumber).isSpare(); |
Contributor
There was a problem hiding this comment.
메소드의 frames.get(frameNumber) 중복도 제거한다.
|
|
||
| public abstract int updateScore(int pinsKnocked); | ||
|
|
||
| public abstract String toString(boolean allPinsDown); |
Contributor
There was a problem hiding this comment.
구현 부가 없는 경우 interface로 구현 가능함.
public interface Score {
boolean isPlayed();
int updateScore(int pinsKnocked);
String toString(boolean allPinsDown);
}
|
|
||
| private Score firstScore = new FirstScore(); | ||
| private Score secondScore = new SecondScore(); | ||
| private int pinsStanding = ALL; |
Contributor
There was a problem hiding this comment.
이 인스턴스 변수는 왜 필요한 거임?
없으면 안되나?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.