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

Step4 리뷰 요청드립니다. #5528

Open
wants to merge 11 commits into
base: zhenxikim
Choose a base branch
from

Conversation

ZhenxiKim
Copy link

step 3의 최신 코드를 pull받으면서 commit이 같이 딸려왔습니다.
리뷰 요청드립니다.

Copy link

@mskangg mskangg left a comment

Choose a reason for hiding this comment

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

4단계 구현 잘해주셨네요.
그런데 요구사항대로 출력이 안되고있어서 확인후 재요청 부탁드릴게요 🙇

Comment on lines +3 to +4
- [X] 자동차 이름은 5자를 초과할 수 없다.
- [X] 우승자는 한명 이상일 수 있다.
Copy link

Choose a reason for hiding this comment

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

👍

gameCount = getGameCount();
System.out.println("실행 결과");
for (int i = 0; i < gameCount; i++) {
job();
}

Winner winner = new Winner();
System.out.println(FormattingUtil.formattingResult(winner.getWinners(cars)) + "가 최종 우승했습니다.");
Copy link

Choose a reason for hiding this comment

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

image

d, e 가 우승했는데 제대로 출력되고있지 않네요 😅

Comment on lines 13 to 18
for (int i = 0; i < size; i++) {
sb.append(list.get(i));
if (i < size - 1) {
sb.append(",");
}
}
Copy link

Choose a reason for hiding this comment

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

규칙 1: 한 메서드에 오직 한 단계의 들여쓰기(indent)만 한다.
indent가 2이상인 메소드의 경우 메소드를 분리하면 indent를 줄일 수 있다.
else를 사용하지 않아 indent를 줄일 수 있다.

인덴트가 2이상이군요 🤔

@ZhenxiKim
Copy link
Author

실행 결과 수정 하였습니다. 추가로 리뷰주신 내용도 반영했습니다. 인덴트를 2로 유지하면서 메서드를 쪼개는게 아직 어색하네요~

Copy link

@mskangg mskangg left a comment

Choose a reason for hiding this comment

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

피드백 반영 잘해주셨네요!
추가로 코멘트 남겨드렸는데, 확인 부탁드립니다 🙇

Comment on lines +20 to +26
public List<String> getWinners(Car[] cars) {
this.filterMaxLength(cars);
for (Car car : cars) {
this.checkWinnerCondition(car);
}
return this.winners;
}
Copy link

Choose a reason for hiding this comment

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

로직에 문제가 있는것 같아요 😅
우승자가 두번 출력되고 있습니다

image


private void filterMaxLength (Car[] cars) {
for (Car car : cars) {
int carStatusLength = car.getStatus().length();
Copy link

Choose a reason for hiding this comment

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

자동차의 길이를 꺼내어 처리하고 있네요!
디미터 법칙에 어긋나고 있습니다
메세지를 보내어 처리해보는건 어떨까요? 🤔

https://mangkyu.tistory.com/147

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