Skip to content

[자동차 경주 미션 1,2단계] 강대현 미션 제출합니다.#175

Open
Kdahyn wants to merge 6 commits intonext-step:kdahynfrom
Kdahyn:Kdahyn
Open

[자동차 경주 미션 1,2단계] 강대현 미션 제출합니다.#175
Kdahyn wants to merge 6 commits intonext-step:kdahynfrom
Kdahyn:Kdahyn

Conversation

@Kdahyn
Copy link

@Kdahyn Kdahyn commented Mar 16, 2026

숫자 생성 역할을 분리하기 위해 NumberGenerator 인터페이스와 구현 클래스를 나누었는데,
이 과정에서 클래스가 다소 많아지는 느낌이 들어 적절한 설계인지 고민이 있었습니다.
실제 랜덤 생성과 테스트용 숫자 생성을 분리한 방식이 괜찮은지,
혹은 현재 단계에서 더 단순하고 좋은 방법이 있는지 궁금합니다.
또한 자바가 아직 익숙하지 않아,
제가 더 공부하면 좋을 개념이나 키워드가 있다면 함께 알려주시면 감사하겠습니다!

@@ -0,0 +1,3 @@
public interface NumberGenerator {
int generate();
} No newline at end of file

Choose a reason for hiding this comment

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

파일 끝에 개행이 누락된 것 같아요! POSIX 표준에서는 텍스트 파일의 마지막 줄도 개행으로 끝나야 하는데, 지금 파일의 경우 개행 없이 끝나서 경고가 발생하고 있어요.

IntelliJ를 사용중이시라면 SettingsEditorGeneralEnsure every saved file ends with a line break 설정을 체크하면 파일 끝에 자동으로 개행을 적용해주니 참고해보세요!

Copy link
Author

Choose a reason for hiding this comment

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

피드백 감사합니다! IntelliJ 설정 적용했고, 코드에도 반영했습니다.

winners.add(car);
}
}
} No newline at end of file
Copy link

@chemistryx chemistryx Mar 16, 2026

Choose a reason for hiding this comment

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

public int generate() {
return random.nextInt(BOUND);
}
} No newline at end of file
Copy link

@chemistryx chemistryx Mar 16, 2026

Choose a reason for hiding this comment

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


assertThat(car.getPosition()).isEqualTo(0);
}
} No newline at end of file
Copy link

@chemistryx chemistryx Mar 16, 2026

Choose a reason for hiding this comment

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

.extracting(Car::getName)
.containsExactlyInAnyOrder("pobi", "crong");
}
} No newline at end of file
Copy link

@chemistryx chemistryx Mar 16, 2026

Choose a reason for hiding this comment

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

index++;
return number;
}
} No newline at end of file
Copy link

@chemistryx chemistryx Mar 16, 2026

Choose a reason for hiding this comment

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

@chemistryx
Copy link

대현님 안녕하세요~! 저도 리뷰가 처음인지라 잘 부탁드리겠습니다 ㅎㅎ
README.md 파일을 잘 작성해주셔서 전반적인 흐름을 이해하는 데 많은 도움이 되었는데요, 말씀해주신 질문에 대한 답변과 몇가지 개선 및 학습 포인트에 대해서 말씀을 드려보려고 해요.

질문에 대한 답변

숫자 생성 역할을 분리하기 위해 NumberGenerator 인터페이스와 구현 클래스를 나누었는데,
이 과정에서 클래스가 다소 많아지는 느낌이 들어 적절한 설계인지 고민이 있었습니다.
실제 랜덤 생성과 테스트용 숫자 생성을 분리한 방식이 괜찮은지,
혹은 현재 단계에서 더 단순하고 좋은 방법이 있는지 궁금합니다.

저도 이 미션을 진행하면서 테스트 단계에서 랜덤성을 어떻게 고정해서 테스트할 수 있을까에 대한 고민을 했었는데요, 아마 대현님도 동일한 부분에서 고민을 하신걸로 보여요.
관련해서 제 의견을 말씀드리자면 저도 대현님과 동일한 방식으로 구현을 진행했었습니다. 클래스가 늘어나는 것이 처음에는 과한 설계처럼 느껴질 수 있지만, 이처럼 인터페이스로 추상화하면 테스트 시 TestNumberGenerator로 교체하기 쉬워지고, 프로덕션 코드와 테스트 코드가 서로 독립적으로 유지된다는 장점이 있다고 생각합니다.
관련해서 더 궁금하신 부분이 있다면 의존성 역전 원칙(DIP) 에 대해서 학습해보시는 걸 추천드립니다!

개선 포인트

  1. 패키지 미선언
    현재 모든 클래스가 default package에 선언되어 있는데요, Java 컨벤션상 패키지를 명시하는 것을 권장드립니다. default package는 다른 패키지에서 import가 불가능하고 클래스 이름 충돌 위험도 있어요.

    패키지 적용 구조 예시
    src/
    - main/
    -- java/
    --- racingcar/
    ---- Car.java
    ---- RacingGame.java
    

학습 포인트

  1. 의존성 역전 원칙 (DIP) - 앞선 질문에서 어느정도 답변이 된 내용이긴 한데, 한번 학습해보시는걸 추천드려요!

  2. Stream API - getMaxPosition()이나 getWinners()처럼 컬렉션을 순회하는 코드를 Stream으로 표현하면 더 선언적으로 작성할 수 있어요. 기존 for문과 Stream으로 작성한 버전을 비교해보는 것도 좋은 학습이 될 것 같습니다!

    Stream 적용 예시
    private int getMaxPosition() {
        return cars.stream()
                .mapToInt(Car::getPosition)
                .max()
                .orElse(0);
    }
  3. 일급 컬렉션 - 현재 RacingGameList<Car>를 직접 다루다 보니, 자동차 목록과 관련된 로직(getMaxPosition(), getWinners() 등)이 RacingGame의 책임으로 들어와 있어요. List<Car>Cars라는 클래스로 감싸면 자동차 컬렉션에 관한
    책임을 분리할 수 있고, 컬렉션 조작도 Cars 내부에서만 가능하도록 제한할 수 있어요.

private final NumberGenerator numberGenerator;

public RacingGame(List<Car> cars, NumberGenerator numberGenerator) {
this.cars = cars;

Choose a reason for hiding this comment

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

외부에서 받은 List<Car>를 그대로 참조하고 있는데, 외부에서 리스트를 변경하면 게임 내부 상태도 함께 바뀌는 상황이 생길 수 있어요. 어떻게 개선할 수 있을까요?

}

private int getMaxPosition() {
int maxPosition = 0;

Choose a reason for hiding this comment

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

maxPosition의 초기값이 0으로 설정되어있어서, 아직 아무도 전진하지 않은 상태에서 getWinners()를 호출하면 모든 차가 우승자로 반환될 것 같아요. 이 동작이 의도된 것인지 한번 생각해보시면 좋을 것 같습니다!

Comment on lines +12 to +24
public void move(int number) {
if (isMovable(number)) {
position++;
}
}

private boolean isMovable(int number) {
return number >= MOVE_CONDITION;
}

public String getName() {
return name;
}

Choose a reason for hiding this comment

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

현재 isMovable()move()getName()사이에 위치해 있어서 publicprivatepublic 순서가 되고 있어요. 일반적으로 자바에서 통용되는 메소드 배치 순서가 있는데, 이 글 참고하셔서 한번 정리해보시는 것도 좋을 것 같습니다!

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