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

[중앙대 동네] 한성재 레이싱카 미션 #66

Open
wants to merge 19 commits into
base: bingle625
Choose a base branch
from

Conversation

bingle625
Copy link

@bingle625 bingle625 commented Oct 6, 2024

4단계 미션

  • 도메인 모델, View, Controller 단을 분리하는 MVC 패턴에 입각하여 클래스를 분리하려고 노력했습니다.
  • 도메인 모델 클래스의 getter 메서드와 constructor 메서드를 만들때 클래스의 공개범위를 어떻게 하면 좋을지 고민이 많이 되었습니다.
    • 클래스의 구성에 대해서 피드백 주시면 감사하겠습니다.

@bingle625 bingle625 changed the title [중앙대 동네] 레이싱카 미션 [중앙대 동네] 한성재 레이싱카 미션 Oct 12, 2024
System.out.println(String.join(",", winnerNames) + "가 최종우승했습니다.");
}

public static void projectCars(List<Car> cars) {

Choose a reason for hiding this comment

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

어떤건 final을 붙였고, 어떤건 안붙였군요 성재님만의 기준이 있으신가요?

Copy link
Author

Choose a reason for hiding this comment

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

final에 대한 기준이 아직 모호해서 발생하는 것 같습니다.
아직은 final을 명확히 붙여야겠다는 생각이 드는 파라미터나 변수에서 사용하게 되는거 같습니다.

따라서 기준이 있다기보다는 익숙치 않은 사용으로 봐주시면 될거 같습니다.ㅎㅎ

this.round = round;
this.cars = new ArrayList<>();

List<String> names = Arrays.stream(namesText.split(","))

Choose a reason for hiding this comment

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

Name들이 , 단위로 구분되어있는 것은 입력과 관련있는 정보인 것 같아요. 이를 RacingGameHost로 이동해보면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

말씀하신 내용대로 names를 array화 하는 로직을 RacingGameHost로 이동했습니다.
0d474fd

.toList();

for (String name : names) {
this.addCar(new Car(new RandomNumberGenerator(), 0, name));

Choose a reason for hiding this comment

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

위의 stream에서 한번에 Car로 만들수도 있을 것 같아요.

.map(String::strip)
.map(new Car(new RandomNumberGenerator(), 0 , name))
    .toList()

이런 식으로요

Choose a reason for hiding this comment

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

그리고 list를 다른 list에 추가할땐 addAll이라는 메서드도 있답니다.

Copy link
Author

Choose a reason for hiding this comment

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

addAll 이라는 메서드에 대해서는 처음 알았습니다.
감사합니다!

Copy link
Author

Choose a reason for hiding this comment

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

해당 내용에 대해서는 stream을 다루는 로직을 RacingGameHost로 이동하면서 로직유지를 하게되었습니다

private final String name;
private final NumberGenerator numberGenerator;

public Car(NumberGenerator numberGenerator, int coordinateX, String name) {

Choose a reason for hiding this comment

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

production에선 NumberGenerator나 coordinateX의 경우에는 현재 요구사항에선 고정되어있어요.

numberGenerator -> RandomNumberGenerator
coordinateX -> 0

name만 받는 생성자를 만들어보면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

좋은 방법인 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

name 만 인자로 받는 static 메서드를 Car 클래스 내부에 추가했습니다.
해당 방식은 어떨지 피드백 부탁드립니다.
95e3a35

public class Application {

public static void main(String[] args) {
Scanner sc = new Scanner(System.in);

Choose a reason for hiding this comment

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

깔끔하게 분리가 되었네요. 잘하셨습니다!

return this.round;
}

public int getCurrentCarCount() {

Choose a reason for hiding this comment

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

요 메서드는 test에서만 쓰이는 것 같아요.
test에서만 쓰이는 메서드는 production에 있는 것에 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

해당 메서드는 getCars 메서드가 추가된 이후, 필요가 없어진 메서드인데 미처 리팩토링 과정에서 삭제하지 못한 메서드입니다.
테스트에서만 쓰이는 메서드가 대체불가하다면, 필요할 수 있겠지만, 해당 경우는 getCars의 size로 대체되는게 옳은 것 같습니다.
감사합니다.

Copy link
Author

Choose a reason for hiding this comment

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

e7257ab
해당 커밋으로 수정했습니다.

}


public static void playGame(final RacingGame game) {

Choose a reason for hiding this comment

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

RacingGameHost에서 입출력 뿐 아니라 게임도 진행시키는군요.

이렇게 설정하신 이유가 있을까요??

Copy link
Author

Choose a reason for hiding this comment

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

해당 부분에 대해서 좀 고민을 했는데, progressBar가 시스템 상에서 출력이 되는 부분이 RacingGameHost 에 있는 편이 더 자연스럽다고 생각한것 같습니다.
이 부분에 대해서는 수업시간에 좀더 이야기해보고 싶습니다.

@hong-sile
Copy link

제 생각에 클래스 구성은 필요한 부분만 깔끔하게 해주신 것 같아요.
수고하셨습니다!!

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