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

[8주차] kmk #55

Open
wants to merge 8 commits into
base: kmk
Choose a base branch
from
Open

[8주차] kmk #55

wants to merge 8 commits into from

Conversation

BLINK-ONCE
Copy link

No description provided.

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로도 충분한 객체지향 프로그래밍이 가능합니다.

import java.util.List;
import racingcar.Winner;

public class OutputView {
Copy link
Member

Choose a reason for hiding this comment

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

정적 메서드만을 제공하는 정적 클래스입니다.
생성자를 막아줌으로써 사용의 혼동을 막아주세요.

import racingcar.Winner;

public class OutputView {
public static final String bar = "-";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static final String bar = "-";
public static final String BAR= "-";

하지만 bar보다 더 좋은 네이밍이 있지 않을까요?

public static void printWinner(Winner winner) {
System.out.println("최종 우승자");
System.out.print(winner.getWinnersName());
}
Copy link
Member

Choose a reason for hiding this comment

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

해당 내용들 전부 해당 객체의 내부 구현을 매우 잘 알고 있습니다.
다른 객체에서는 해당 객체의 내부 구현을 몰라도 사용할 수 있게 구현을 개선해주세요

}
System.out.println();

if (maxReach < car.getPosition()) {
Copy link
Member

Choose a reason for hiding this comment

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

책임이 늘어났네요?
결과를 출력해주는 객체가 아니라 게임의 결과를 판별하고 있습니다.


public class Application {

public static void main(String[] args) {
Copy link
Member

Choose a reason for hiding this comment

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

메서드의 10줄 이상은 메인 메서드 또한 마찬가지 입니다.

throw new IllegalArgumentException("[ERROR] 랜덤값이 잘못되었습니다.");
}

if (startInclusive < 0) {
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.

이 내용에 주석이 삭제된다면 알 수 있을까요?
외국인이 코드를 유지보수 하게 된다면?
우리는 우리나라 사람이랑만 협업하지 않습니다

for (int i = 0; i < carNames.length; i++) {
String name = carNames[i];

if (isNameLong(name)) {
Copy link
Member

Choose a reason for hiding this comment

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

인덴트

}

public static boolean isNameLong(String name) {
if (name.length() > 5) {
Copy link
Member

Choose a reason for hiding this comment

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

매직넘버

public static int isNameOnly(String[] carNames) {
Map<String, String> nameCount = new HashMap<String, String>();

for (int i = 0; i < carNames.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

인덴트

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