Skip to content

Conversation

@BLINK-ONCE
Copy link

No description provided.

BLINK-ONCE added 3 commits May 2, 2021 17:45
Make InputView receive inputs. getWinnersName() in Winners
get winners name by stream
@BLINK-ONCE BLINK-ONCE changed the title [10 주차] kmk [10주차] kmk May 2, 2021
Copy link
Member

@PandaHun PandaHun left a comment

Choose a reason for hiding this comment

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

  1. build와 gradle폴더에 gitignore추가 부탁드려요.
  2. 공통적으로 코드에 마지막 줄이 없는 경우가 있습니다
    이 또한 컨벤션이니 맞춰주세요
  3. 이번 주차의 실습 요구사항을 맞춰주세요

재리뷰 요청드립니다.

nameCheck = Validator.validateName(carNames);
ErrorCatcher.catchNameError(nameCheck);

if (nameCheck[0] == 0 && nameCheck[1] == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

뭘 의미하는 거죠?

cycle = inputView.inputCycle();

nameCheck = Validator.validateName(carNames);
ErrorCatcher.catchNameError(nameCheck);
Copy link
Member

Choose a reason for hiding this comment

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

전형적인 절차지향코드 형태입니다.

객체지향을 살려서 코드를 구현해 주세요.

선언과 할당이 분리될 필요가 없습니다


public String[] inputCarNames(){
System.out.println("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)");
return SCANNER.nextLine().split(",");
Copy link
Member

Choose a reason for hiding this comment

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

매직 넘버란 코드 안에 작성된 구체적인 숫자 혹은 문자열을 말합니다. 그리고 매직 넘버의 단점은 아래와 같습니다.

  • 의미를 이해하기 어렵다.
  • 복수의 장소에서 사용된다면 변경하기 힘들다.

그런 매직 넘버는 의미를 나타낼 수 있는 상수로 치환하여 코드의 가독성을 높여 보는 것은 어떨까요?

.collect(Collectors.toList());
OutputView.printResultTitle();

for (int i = 0; i < cycle; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

for -> stream 변경해주세요

}

public List<Car> moveCar() {
for (Car car : cars) {
Copy link
Member

Choose a reason for hiding this comment

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

for -> stream

List<String> winnerName = new ArrayList<>();

winnerName = cars.stream()
.filter(x -> x.getPosition() == maxPosition)
Copy link
Member

Choose a reason for hiding this comment

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

해당부분을 메서드로 추출할 수 있을것 같은데요?

if (nameErrors[1] == 1) {
System.out.println("[ERROR] 이름의 길이가 부적절합니다.");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

누군가가 이 코드를 받는다면 이해할 수 있을까요?


// index 0에 이름 중복 검사 결과
// index 1에 이름 길이 검사 결과
validationResult[0] = isNameOnly(carNames);
Copy link
Member

Choose a reason for hiding this comment

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

주석이 필요한 코드는 충분한 의미를 담지 못했다고 생각됩니다

validationResult[1] = 1;
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

indent, for 등 요구사항을 만족시켜주세요

return 1;
}
nameCount.put(carNames[i], "이것은 존재한.");
}
Copy link
Member

Choose a reason for hiding this comment

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

indent, for 등 요구사항을 만족시켜주세요

array for car name validation has been changed to primitive
type variables. and method for car name validation has been
changed stream form. I madd main method length short.
I replaced magic number, SCANNER to not magic number,
scanner.
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