Skip to content

[자동차 경주] 김하빈 미션 제출합니다.#11

Open
KIM-GOING wants to merge 2 commits intoJava-JavaScript-Language-Stuty:mainfrom
KIM-GOING:kim-going
Open

[자동차 경주] 김하빈 미션 제출합니다.#11
KIM-GOING wants to merge 2 commits intoJava-JavaScript-Language-Stuty:mainfrom
KIM-GOING:kim-going

Conversation

@KIM-GOING
Copy link
Copy Markdown

1주차에서 코드의 분리가 하나도 되지 않아 코드 분리를 하고 클래스 및 메소드의 기능을 더욱 세세하게 할 수 있도록 노력했습니다.

또한 java가 가지고 있는 고유한 기술을 사용할 수 있도록 공부하며 stream, string 등을 적용해보았습니다.

Copy link
Copy Markdown

@daeGULLL daeGULLL left a comment

Choose a reason for hiding this comment

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

확실히 이런 간단한 과제에서는 MVC패턴을 도입하지 않았을 때 이렇게 깔끔한 코드가 나올 수 있는 것 같습니다. 정말 수고하셨습니다~~ 다음번에는 연습 겸 MVC 패턴 써보셔도 좋을 것 같습니다!!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

기능을 4개 정도로 분리하신 것 같은데, 기능에 따라 여러번 커밋해주시면 더 좋을 것 같습니다~~

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MVC패턴을 도입해볼 의향이 있으시면 이걸 model 디렉토리 하에 넣어 모델 취급해주셔도 좋을 것 같아요~~

private void printStatus() {
for(Car car : cars) {
System.out.println(car.getStatus());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

위쪽에서 car를 순회하는 for문이 이미 있으니 이걸 따로 메소드로 뽑기보단 저 for문에 넣는게 더 좋을 것 같아요!! 메소드 유지하시려면 메소드 내 for문은 없애고 마찬가지로 기존 for문 내에 넣으시면 더 좋을 것 같습니다

}
return carNames;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

위쪽 car 생성자에서 이미 예외처리를 해두셨던데 validator를 중심으로 사용하실거면 생성자 내의 예외처리는 없애셔도 될 것 같아요~

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

동의합니다!validator로 분리했다면 예외처리는 이쪽에서 다루는게 좋아보여용!


// 객체 움직임 정의 :
public void move() {
int value = Randoms.pickNumberInRange(0,9);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

여기에서 랜덤값추출하는건 상수화와 함께 따로 util로 분리하는 것은 어떨까용?

}

public String getStatus() {
return carName + " : " + "-".repeat(position);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MVC패턴을 도입하신다면 입출력은 이 객체에서 분리하는게 좋을듯해용


// 경주할 자동차 입력과 관련한 예외
public static List<String> validateCarNames(String input) {
List<String> carNames = Arrays.asList(input.split(","));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arrays.asList(input.split(","));에서 불변리스트를 생성해서 요소 자체는 변경할 수 있지만, 리스트크기는 변경할 수 없을 것 같아요! 나중에 혹시 모를 확장성을 고려하여 new ArrayList<>(Arrays.asList(input.split(",")));로 변경하는것은 어떨까요?? 만약, carNames가 변결되지않는다면, 차라리 진짜 불변리스트로 List.of()를 활용하는것이 좋아보여요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

@seulnan seulnan left a comment

Choose a reason for hiding this comment

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

너무 고생하셨습니다:)

가독성이 좋은 것 같아요! 저도 많이 배워가는 리뷰였습니다!


// 경주할 자동차 입력과 관련한 예외
public static List<String> validateCarNames(String input) {
List<String> carNames = Arrays.asList(input.split(","));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

}

// 최종 우승자 출력 메소드
private void printWinner() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
private void printWinner() {
private void printWinner() {
String winners = cars.stream()
.filter(car -> car.getPosition() == winnerDistance)
.map(Car::getName)
.collect(Collectors.joining(", "));
System.out.println("최종 우승자 : " + winners);

이런식으로 코드를 한줄로 최적화할 수 있을 것 같아요!


// 게임 진행 메소드
private void run() {
for(int i = 0; i < attempts; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

여기서 stream대신 이중 for문을 사용하신 이유가 있을까용??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

stream 사용이 어색해 아직 stream을 사용해야겠다 생각 못한 정도..

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.

3 participants