-
Notifications
You must be signed in to change notification settings - Fork 48
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
base: bingle625
Are you sure you want to change the base?
Conversation
- 움직이는 자동차 TODO 리스트 작성
복수 우승자 테스트 실패
- winnerNames를 가져온다는 조금더 명시적인 이름으로 변경
System.out.println(String.join(",", winnerNames) + "가 최종우승했습니다."); | ||
} | ||
|
||
public static void projectCars(List<Car> cars) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤건 final을 붙였고, 어떤건 안붙였군요 성재님만의 기준이 있으신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final에 대한 기준이 아직 모호해서 발생하는 것 같습니다.
아직은 final을 명확히 붙여야겠다는 생각이 드는 파라미터나 변수에서 사용하게 되는거 같습니다.
따라서 기준이 있다기보다는 익숙치 않은 사용으로 봐주시면 될거 같습니다.ㅎㅎ
src/main/java/RacingGame.java
Outdated
this.round = round; | ||
this.cars = new ArrayList<>(); | ||
|
||
List<String> names = Arrays.stream(namesText.split(",")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name들이 , 단위로 구분되어있는 것은 입력과 관련있는 정보인 것 같아요. 이를 RacingGameHost로 이동해보면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀하신 내용대로 names를 array화 하는 로직을 RacingGameHost로 이동했습니다.
0d474fd
src/main/java/RacingGame.java
Outdated
.toList(); | ||
|
||
for (String name : names) { | ||
this.addCar(new Car(new RandomNumberGenerator(), 0, name)); |
There was a problem hiding this comment.
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()
이런 식으로요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 list를 다른 list에 추가할땐 addAll이라는 메서드도 있답니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addAll 이라는 메서드에 대해서는 처음 알았습니다.
감사합니다!
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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만 받는 생성자를 만들어보면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 방법인 것 같습니다!
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔하게 분리가 되었네요. 잘하셨습니다!
src/main/java/RacingGame.java
Outdated
return this.round; | ||
} | ||
|
||
public int getCurrentCarCount() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 메서드는 test에서만 쓰이는 것 같아요.
test에서만 쓰이는 메서드는 production에 있는 것에 어떻게 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 메서드는 getCars 메서드가 추가된 이후, 필요가 없어진 메서드인데 미처 리팩토링 과정에서 삭제하지 못한 메서드입니다.
테스트에서만 쓰이는 메서드가 대체불가하다면, 필요할 수 있겠지만, 해당 경우는 getCars의 size로 대체되는게 옳은 것 같습니다.
감사합니다.
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RacingGameHost에서 입출력 뿐 아니라 게임도 진행시키는군요.
이렇게 설정하신 이유가 있을까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분에 대해서 좀 고민을 했는데, progressBar가 시스템 상에서 출력이 되는 부분이 RacingGameHost
에 있는 편이 더 자연스럽다고 생각한것 같습니다.
이 부분에 대해서는 수업시간에 좀더 이야기해보고 싶습니다.
제 생각에 클래스 구성은 필요한 부분만 깔끔하게 해주신 것 같아요. |
- 5자 이하 입력만 가능하도록 exception 추가
- Car의 static 메서드로 추가
4단계 미션