Skip to content

Conversation

@JangYEhoon00
Copy link

No description provided.

@KimYjoo
Copy link

KimYjoo commented Oct 28, 2025

커밋 메시지에 테스트 코드 추가는 test 머릿말을 사용하는게 더 좋을 것 같아요!
지금은 기능 코드인지 테스트 코드인지 한눈에 안보이는 것 같아요

@KimYjoo
Copy link

KimYjoo commented Oct 28, 2025

전체적인 코드에서 역할 분리가 명확했으면 좋았을 것 같아요
대표적으로 input을 처리하는 곳에서 출력까지 처리하고 있어서 하나의 함수 혹은 파일에서 여러가지 책임을 동시에 지니고 있어요
입력은 입력만,
출력은 출력만,
혹은 게임 진행만 처리하도록 역할 분리하는 것이 나중에 유지보수할 때 수월해지거든요.
대표적으로 MVC 패턴이라는 게 있는데 참고하시면 좋을것 같아요!

@JangYEhoon00
Copy link
Author

앗 나름 분리한다고 해봤는데 부족했나봅니다! MVC패턴 참고해서 작성해보겠습니다!

Copy link

@pangil5634 pangil5634 left a comment

Choose a reason for hiding this comment

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

2주차도 고생 많으셨습니다.

전반적인 코드를 보면서 가장 크게 느꼈던 점은 MVC 패턴과 같이 디자인 패턴을 적용해보시는 것은 어떨까 하는 바램이 듭니다.

또한, constants 폴더를 만들고 필요한 상수를 따로 분리시키는 것도 유지보수성에 있어서 효과가 있을 것 같아서, 이러한 점을 고려하여 3주차를 진행하시면 더욱 좋을 것 같습니다.

수고 많으셨습니다.

Comment on lines +8 to +10
const CAR_NAMES = await GameInput.getCarNames();
const ROUND_COUNT = await GameInput.getRoundCount();

Choose a reason for hiding this comment

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

입력 받는 부분을 하나의 함수로 분리해서, 자동차 이름 입력과 라운드 입력을 받는 하나의 입력 단계로 App.js에 구성하는 것은 어떨까요?

const [carNames, roundCount] = await initRacingGame();
async function initRacingGame() {
  const carNames = await GameInput.getCarNames();
  const roundCount = await GameInput.getRoundCount();

  return [carNames, roundCount];
}

Comment on lines +14 to +15
const WINNERS = RACE_GAME.getWinners();
Console.print(`최종 우승자 : ${WINNERS.join(", ")}`);

Choose a reason for hiding this comment

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

이후 로직에서 배열을 다시 사용하지 않기에, join하는 부분을 getWinners()와 함께 사용하는 것은 어떨까요?

1안

Console.print(`최종 우승자 : ${RACE_GAME.getWinners().join(", ")}`);

2안

const WINNERS = RACE_GAME.getWinners().join(", ");
Console.print(`최종 우승자 : ${WINNERS}`);

Comment on lines +17 to +22
playRound(RandomNumber) {
this.cars.forEach((car) => {
const randomNumber = makeRandomNumber();
car.move(randomNumber);
});
}

Choose a reason for hiding this comment

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

인자를 제거하는 것은 어떨까요?

현재 RandomNumber 매개변수를 받지만, 실제로 사용하고 있지 않는 코드로 보입니다.

이것은 혼동을 줄 수 있는 죽은 파라미터와 같아서, 제거하는 것이 깔끔해 보입니다!

혹시 인자를 추가하신 이유가 있으실까요?

Comment on lines +12 to +14
const NAME_ARRAY = carNames.split(",");

return NAME_ARRAY.map((name) => new CarInfo(name.trim()));

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.

3 participants