Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Step3 볼링점수판 #992

Open
wants to merge 41 commits into
base: parkyunhwan
Choose a base branch
from
Open

Conversation

Parkyunhwan
Copy link

안녕하세요. 리뷰어님~

FinalFrame으로 마지막프레임에 대한 로직을 추가해보았습니다.

점수출력을 위해 Score를 넣었는데 OutputView출력부분에서 더 어려움을 느낀 것 같습니다.

미션을 기간내에 다 끝내보고 싶었는데 그러지 못하게 되었네요 ㅎㅎ..

현재 프레임의 위치와 Frame의 볼링을 친 갯수에 따라 다음 프레임 진행여부를 판단함
HitRecord에 각 투구 당시의 상황에 따른 볼링친 갯수와 그에따른 볼링용어를 기록하도록 함
BowlingGame과 Frames는 어떤 프레임인지 모르고 실행해도 실객체에 따라 알아서 스트라이크 조건과 프레임완료조건이 다르게 실행됨.
현재 프레임의 위치와 Frame의 볼링을 친 갯수에 따라 다음 프레임 진행여부를 판단함
Copy link

@JunHoPark93 JunHoPark93 left a comment

Choose a reason for hiding this comment

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

안녕하세요 볼링 3단계 구현하느라 고생많으셨습니다.
구조관련해서 몇 가지 코멘트 남겨두었는데 확인 후 의견 및 반영부탁드려요 😊

@@ -0,0 +1,22 @@
package bowling;

public class FinalFrame extends Frame {

Choose a reason for hiding this comment

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

Frame을 상속하는 FinalFrame을 만들어주셨네요!
다만 일반 Frame은 Frame객체를 만들어서 사용중인데요, FinalFrame과 공통된 기능은 묶고 서로 다르게 동작해야하는 것들은 추상메서드로 만들어보면 어떨까요?

Comment on lines +41 to +42
protected boolean finishFrame() {
return clearAllFrame() || hitRecords.hitTimes(HIT_TWICE);

Choose a reason for hiding this comment

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

위와 같은 내용이지만 FinalFrame이 이 메서드를 override를 하는것보다 각기 구현하는게 구조가 더 나아보여요~

Comment on lines +23 to +28
if (!bowilingTerm.equals(BowilingTerm.STRIKE)) {
ListIterator<Frame> frameListIterator = frames.listIterator(frameNumber.retrieveIndexNumber());
while (frameListIterator.hasPrevious()) {
Frame previous = frameListIterator.previous();
previous.calculateBonus(hitCount);
}

Choose a reason for hiding this comment

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

depth를 줄이는 방향으로 고민해보면 어떨까요?

@@ -11,14 +11,15 @@ public BowlingGame() {
}

public FrameNumber pitchingBall(int hitCount) {

Choose a reason for hiding this comment

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

pitchingBall메서드에서 반환값이 필요할까요? 🤔

Comment on lines +9 to +10
private final List<HitRecord> hitRecords;
private Score score;

Choose a reason for hiding this comment

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

HitRecord안에 Score를 관리할 수 있는 구조로 개선해보면 어떨까요? 구조가 좀 많이 바뀔거같긴한데 한번 참고만 해보셔요!
추가적으로 도메인에 대한 테스트도 추가되면 더 좋을거같아요 :)

import java.util.ArrayList;
import java.util.List;

public class Scores {

Choose a reason for hiding this comment

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

쓰이지 않는것 같네요 :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants