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

3단계 - 자동차 경주(우승자) #320

Merged
merged 28 commits into from Jun 18, 2019

Conversation

minseok-kr
Copy link

3단계 - 자동차 경주(우승자) 입니다.

  1. WinnerMakerTest 쪽의 setup 부분이 불필요하게 커진 느낌이 드는데... 이는 어쩔 수 없는 것 일까요?

감사합니다!

shinyeonjong and others added 28 commits March 8, 2019 22:01
Copy link

@young891221 young891221 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다. 피드백을 반영하기 노력한 흔적이 보이네요ㅎㅎ

추가적으로 개선사항들을 남겨두웠습니다. 다음 단계에서 같이 반영 부탁드립니다!

@@ -3,9 +3,11 @@
import racing.strategy.DrivingStrategy;

public class Car {
public int position;
private String name;
private int position;

Choose a reason for hiding this comment

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

Car 객체를 일급 객체로 만들기 위해서는 필드값을 하나만 들고 있도록 해야 합니다.
앞으로 모든 객체는 필드값 1개만 가지도록 지향해 보세요!
힌트는 https://jojoldu.tistory.com/412 여기를!

@@ -7,8 +7,8 @@
private List<Car> cars;
private int moves;

Choose a reason for hiding this comment

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

이쪽도 일급 객체로 변환 부탁드려요~!

@@ -28,11 +30,11 @@ private void move() {
Printer.printResult(cars);
}

private void setCars(int numOfCars) {
cars = new ArrayList<>(numOfCars);
private void setCars(List<String> carNames) {

Choose a reason for hiding this comment

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

setter는 이번 과정에서 지양해야될 습관중 하나입니다.
setter 사용하지 않고 로직을 처리하도록 변경 부탁드립니다.

import java.util.List;
import java.util.stream.Collectors;

public class WinnerMaker {

Choose a reason for hiding this comment

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

WinnerMaker는 깔끔하게 잘 구현하셨네요! 💯

@@ -18,6 +18,8 @@ public void startRacing() {
for (int i = 0; i < this.moves; i++) {
move();
}

Printer.printEndGame(new WinnerMaker(cars).getWinners());
}

private void move() {

Choose a reason for hiding this comment

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

잘 짜신 코드에 비해 단위 테스트가 많이 부족합니다.
저는 그 원인중 하나가 void형 반환에 있다고 생각이 드는데요.
작은 단위 테스트를 만들어 최대한 하나의 메서드는 어떠한 결과값을 반환하도록 코딩을 해보세요.
훨씬 단위 테스트하기 쉬워질 것입니다.

@young891221 young891221 merged commit f5b31aa into next-step:minseok-kr Jun 18, 2019
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.

None yet

4 participants