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

step3 #6072

Merged
merged 2 commits into from
Mar 23, 2025
Merged

step3 #6072

merged 2 commits into from
Mar 23, 2025

Conversation

HanSangKkyu
Copy link

No description provided.


public static void printResult(final List<Car> cars) {
cars.forEach(car -> {
printCarPosition(car.getPosition());
Copy link
Author

Choose a reason for hiding this comment

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

어떻게 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.

출력을 위해선 getter 가 사용될 수 있다고 생각되는데요!
여기저기서 Car 객체로부터 값을 직접 꺼내어 활용하면 코드 복잡도가 올라가기 때문에
비즈니스 로직 수행중 무분별한 getter 활용을 자제하라는 피드백 아니었을까 싶네요 ㅎㅎ

Copy link
Member

@Hyeon9mak Hyeon9mak left a comment

Choose a reason for hiding this comment

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

안녕하세요 상규님! 3단계도 잘 진행해주셨습니다 👍
몇 가지 간단한 코멘트 남겼는데 확인해주세요!
궁금하신 점 언제든지 편하게 질문주세요~

Comment on lines +3 to +5
public final class InputView {

private InputView() {}
Copy link
Member

Choose a reason for hiding this comment

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

불필요한 생성자를 막아주셨군요 👍

Comment on lines +1 to +11
public class Car {
private static final int STANDARD_VALUE = 4;
private int position;

public Car(int position) {
this.position = position;
}

public Car() {
this(0);
}
Copy link
Member

Choose a reason for hiding this comment

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

다양한 생성자를 개방해서 개발편의 제공 👍

Comment on lines 10 to 12
public int getRandomNumber() {
return (int) (Math.random() * 10);
}
Copy link
Member

Choose a reason for hiding this comment

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

내부에서만 활용되는 메서드인 것 같은데요! 접근제한자를 변경해줄 수 있겠네요 ㅎㅎ

Comment on lines 9 to 19
final List<Car> cars = new ArrayList<>();
for (int i = 0; i < carNum; i++) {
cars.add(new Car());
}
final RacingManager racingManager = new RacingManager(cars);

ResultView.printTitle();
for (int i = 0; i < tryNum; i++) {
racingManager.play();
ResultView.printResult(cars);
}
Copy link
Member

Choose a reason for hiding this comment

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

"입력 받은 수 만큼 자동차를 준비한다.", "입력 받은 수 만큼 게임을 반복한다." 모두 비즈니스 로직으로 보이는데요!
Main 대신 다른 객체에게 책임을 넘겨보는 건 어떨까요?

그럼 동료 개발자들이 클래스 이름만 보고 "정책과 관련된 코드가 여기 있겠군" 의식하면서
원하는 코드를 찾기 훨씬 수월해질 것 같아요.

가령 RacingManager 의 생성자에서 마법이 일어난다던지?
RacingManager 에게 "게임이 끝났어?" 라고 물어본다던지... 여러가지 방법이 있겠네요 😄


public static void printResult(final List<Car> cars) {
cars.forEach(car -> {
printCarPosition(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.

출력을 위해선 getter 가 사용될 수 있다고 생각되는데요!
여기저기서 Car 객체로부터 값을 직접 꺼내어 활용하면 코드 복잡도가 올라가기 때문에
비즈니스 로직 수행중 무분별한 getter 활용을 자제하라는 피드백 아니었을까 싶네요 ㅎㅎ

Comment on lines 6 to 22
public class CarTest {
@Test
@DisplayName("기준값 보다 크면 1칸 이동한다")
void moveOneBy_go() {
final Car car = new Car();
car.moveOneBy(5);
assertThat(car.isPosition(1)).isTrue();
}

@Test
@DisplayName("기준값 보다 작으면 이동하지 않는다")
void moveOneBy_wait() {
final Car car = new Car();
car.moveOneBy(4);
assertThat(car.isPosition(0)).isTrue();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

😉 👍
혹시 자동차 수나, 게임진행 수가 음수로 입력되면 어떤 일이 벌어지나요?

@HanSangKkyu HanSangKkyu requested a review from Hyeon9mak March 22, 2025 03:07
Copy link
Member

@Hyeon9mak Hyeon9mak 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 +5 to +20
public class Cars {
private final List<Car> cars;

public Cars(int carNum) {
this.cars = Stream.generate(Car::new).limit(carNum).collect(Collectors.toList());
}

public Cars(Cars cars) {
this.cars = cars.cars.stream().map(Car::new).collect(Collectors.toList());
}

public void move() {
this.cars.forEach(car -> car.moveBy(RandomNumberUtil.getRandomNumberFromZeroToNine()));
}

public List<Car> getCars() {
Copy link
Member

Choose a reason for hiding this comment

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

자동차에 대한 일급 컬렉션을 만들어주셨군요! 👍
어떤 장점을 느끼셨을까요?

Comment on lines +6 to +12
public RacingManager(int carNum, int tryNum) {
if (carNum <= 0 || tryNum <= 0) {
throw new IllegalArgumentException("자동차 대수와 시도 횟수는 0보다 커야 합니다.");
}
this.tryNum = tryNum;
this.cars = new Cars(carNum);
carsSnapShots = new CarsSnapShots();
Copy link
Member

Choose a reason for hiding this comment

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

😉 👍

Comment on lines +8 to +22
@Test
@DisplayName("자동차 개수와 시도 횟수가 0보다 작을 때 예외를 던진다.")
void car() {
assertThrows(IllegalArgumentException.class, () -> {
new RacingManager(0, 5);
});

assertThrows(IllegalArgumentException.class, () -> {
new RacingManager(3, 0);
});

assertThrows(IllegalArgumentException.class, () -> {
new RacingManager(0, 0);
});
}
Copy link
Member

Choose a reason for hiding this comment

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

테스트 코드 작성이 훨씬 간결해졌네요! 💯

@Hyeon9mak Hyeon9mak merged commit b1c34ef into next-step:hansangkkyu Mar 23, 2025
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