Skip to content

Step1 - 볼링 점수판(그리기) 리뷰 요청드립니다. #37

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

Merged
merged 44 commits into from
Jul 19, 2019

Conversation

Integerous
Copy link

많이 늦었습니다.. 지우고 지우고 지웠습니다..
코드가 별로일 수 있습니다.. 머지가 안돼도 좋습니다..
밤낮으로 고민했지만 쉽지 않았습니다.. 많은 지적과 조언 부탁드립니다..

안녕하세요ㅎㅎ
마지막 수업에서 같은 테이블이었는데 자리가 멀어 거의 대화를 나누지 못해 아쉬웠습니다..ㅠ
최대한 TDD로 작성하려고 노력했지만.. 지우고 작성하고를 반복했기 때문에 문제가 많을 것으로 예상됩니다..!!

기능적으로는 잘 동작하는 것으로 여겨지는데
OutputView에서 출력되는 모양새를 아직 제대로 잡지 않았습니다.. (프레임 결과에 상관없이 너비가 고정된 상태)
화면 출력은 급한 불만 끈 상태라 다른 부분들 먼저 봐주시면 감사하겠습니다..!!

혹시나 커밋 메세지를 보신다면, NoramlFrame, FinalFrame 삭제 이 커밋메세지 이후부터 보시면 됩니다.

바쁘시겠지만 많은 지적과 조언 부탁드리겠습니다..!! 👍 👍

Integerous and others added 30 commits July 11, 2019 00:01
Copy link

@boorownie boorownie left a comment

Choose a reason for hiding this comment

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

안녕하세요:) 리뷰어 류성현입니다!
요구사항 기능 구현 아주 잘 진행하셨습니다 👍
군데군데 객체지향 생활체조 규칙4, 한 줄에 점을 하나만 찍는다. 에 대해서 고민을 좀 더 해보시면 좋을 것 같아요 :)
추가적인 피드백은 다음 미션 피드백에 포함하도록 하겠습니다~

}
Frame bowledFrame = currentFrame().fillFrame(fallenPins);

if (bowledFrame.getIndex().isSameIndex(currentFrame().getIndex())) {

Choose a reason for hiding this comment

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

List frames 의 일급컬렉션을 만들어 if분의 분기를 일급컬렉션으로 분리해보면 좋을 것 같습니다 :)

if (bowledFrame.getIndex().isSameIndex(currentFrame().getIndex())) {
frames.set(lastFrameIndex(), bowledFrame);
}
if (!bowledFrame.getIndex().isSameIndex(currentFrame().getIndex())) {

Choose a reason for hiding this comment

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

규칙 4: 한 줄에 점을 하나만 찍는다.

Frame에서 Index를 꺼내와서 비교하는 로직이네요~
꺼내와서 처리하는 방법보다 직접 처리하는 메서드를 가지는 방법으로 구현해보세요 :)

import java.util.Collections;
import java.util.List;

public class BowlingGame {

Choose a reason for hiding this comment

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

BowlingGame에 대한 단위 테스트가 있으면 좋을 것 같아요 :)

private static final String SPARE_SYMBOL = "/";

private Pins firstFallenPins;
private Pins secondFallenPins;

Choose a reason for hiding this comment

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

사용하지 않고있는 필드로 보입니다 :)

@boorownie boorownie merged commit 7a7f5d8 into next-step:Integerous Jul 19, 2019
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.

2 participants