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

Step5 - 리펙토링 #6073

Merged
merged 7 commits into from
Mar 22, 2025
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -52,3 +52,11 @@
* 우승자는 차량 이름으로 표시한다.
* ResultView에서 우승자 목록을 출력한다.
* 우승자가 여러명일 경우 쉼표(,)로 구분하여 표시한다.

### Step5
* 아래 3가지 개발 원칙을 적용해본다
* 모든 원시값과 문자열을 포장한다
* 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.
* 한 줄에 점을 하나만 찍는다
* 일급 콜렉션을 쓴다
* 도메인 객체를 테스트가 가능하도록 수정
48 changes: 48 additions & 0 deletions src/main/java/race/CarList.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package race;

import java.util.ArrayList;
import java.util.List;
import java.util.Random;

public class CarList {
private final Random random = new Random();

Choose a reason for hiding this comment

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

이 클래스의 역할과 책임이 너무 비대한 것 같습니다. 우선 CarList 는 일급 콜렉션임을 염두에 두고 개발 하셨나요?
그렇다면 random 변수는 조금 과하다는 생각이 듭니다. 아울러, 제가 지난 스텝에서도 한번 말씀드린 부분이기는 한데, 인스턴스 변수에 직접 new 를 호출해 값을 할당함으로써 테스트 가능성이 극적으로 줄어든 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

다음과 같이 moveWithRandom 에서 random을 주입 받도록 수정했습니다.

public void moveWithRandom(Random random) {
    for (RacingCar car : cars) {
        car.moveWithSeed(random.nextInt());
    }
}


private final List<RacingCar> cars;

public CarList() {
this(new ArrayList<>());
}

public CarList(List<RacingCar> cars) {
this.cars = cars;
}

public void add(RacingCar car) {
cars.add(car);
}

public void moveWithRandom() {
for (RacingCar car : cars) {
car.moveWithSeed(random.nextInt());
}
}

public List<RacingCar> getList() {
return cars;
}

public List<RacingCar> getWinners() {
Position maxPosition = new Position();
for (RacingCar car : cars) {
maxPosition = car.getMaxPosition(maxPosition);
}

Choose a reason for hiding this comment

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

  • List 의 stream API로 개선 시도도 해보시면 좋겠습니다.
  • Position maxPosition = new Position(); 은 없앨 수 있을 것 같습니다. 이것을 없애는 것을 목표로 stream API 사용을 시도해봐 주세요.

Copy link
Author

Choose a reason for hiding this comment

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

comparable 을 구현하여 아래와 같이 sort 를 적용해보았습니다.
코멘트 주신 것 처럼 CarList에서는 우승한 RacingCar 객체만 신경쓰는게 더 적절한 것 같습니다.

cars.sort(Collections.reverseOrder());
RacingCar winner = cars.get(0);


List<RacingCar> winners = new ArrayList<>();

for (RacingCar car : cars) {
if (car.isSamePosition(maxPosition)) winners.add(car);
}
return winners;

Choose a reason for hiding this comment

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

여기도 stream API 로 개선 시도해주세요.

}

}
15 changes: 15 additions & 0 deletions src/main/java/race/CarName.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package race;

public class CarName {
private final String name;

public CarName(String name) {
this.name = name;
}

@Override
public String toString() {
return name;
}

}
8 changes: 0 additions & 8 deletions src/main/java/race/DefaultPositionPrinter.java

This file was deleted.

12 changes: 0 additions & 12 deletions src/main/java/race/DefaultRandomNumberGenerator.java

This file was deleted.

14 changes: 0 additions & 14 deletions src/main/java/race/FixedNumberGenerator.java

This file was deleted.

44 changes: 44 additions & 0 deletions src/main/java/race/Position.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package race;

import java.util.Objects;

public class Position {
private int value;

public Position() {
this(0);
}

public Position(int value) {
this.value = value;
}

public Position move() {
this.value++;
return this;
}

public int getValue() {
return this.value;
}

public Position getMax(Position comparePosition) {
if (this.value > comparePosition.getValue()) {
return this;
}
return comparePosition;
}

Choose a reason for hiding this comment

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

이런 역할을 할 수 있는 java.lang.Object 의 좋은 메소드 가 있습니다.
이것을 오버라이드 하시는 것이 좋겠습니다.

Copy link
Author

Choose a reason for hiding this comment

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

아래와 같이 compareTo 오버라이드 하였습니다

@Override
public int compareTo(Position o) {
    return Integer.compare(this.value, o.value);
}


@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (obj == null || getClass() != obj.getClass()) return false;
Position position = (Position) obj;
return value == position.value;
}

@Override
public int hashCode() {
return Objects.hash(value);
}
}
5 changes: 0 additions & 5 deletions src/main/java/race/PositionPrinter.java

This file was deleted.

48 changes: 24 additions & 24 deletions src/main/java/race/RacingCar.java
Original file line number Diff line number Diff line change
@@ -1,48 +1,48 @@
package race;

public class RacingCar implements Comparable<RacingCar> {
public class RacingCar {
private static final int MAX_NAME_LENGTH = 5;
private final String name;
private int position;
private final RandomNumberGenerator randomNumberGenerator;
private final PositionPrinter positionPrinter;
private static final int MOVE_CRITERIA = 4;
private final CarName name;
private final Position position;

private RacingCar(String name, RandomNumberGenerator randomNumberGenerator, PositionPrinter positionPrinter) {
this.name = name;
this.randomNumberGenerator = randomNumberGenerator;
this.positionPrinter = positionPrinter;
public RacingCar(String name) {
this(name, 0);
}

@Override
public int compareTo(RacingCar other) {
return Integer.compare(this.position, other.position);
public RacingCar(String name, int position) {
this.name = new CarName(name);
this.position = new Position(position);
}

public static boolean validateName(String carName) {
return !carName.isEmpty() && carName.length() <= MAX_NAME_LENGTH;
}

private boolean shouldMove(int num) {
return num >= 4;
return num >= MOVE_CRITERIA;
}

public int move() {
int randomValue = randomNumberGenerator.generate();
if (shouldMove(randomValue)) {
position++;
public Position moveWithSeed(int seed) {
if (shouldMove(seed)) {
return this.position.move();
}
return position;
return this.position;
}

public void printPosition() {
positionPrinter.printPosition(name, position);
public CarName getName() {
return this.name;
}

static RacingCar create(String racingName, RandomNumberGenerator randomNumberGenerator, PositionPrinter positionPrinter) {
return new RacingCar(racingName, randomNumberGenerator, positionPrinter);
public Position getPosition() {
return this.position;
}

public String getName() {
return name;
public Position getMaxPosition(Position comparePosition) {
return this.position.getMax(comparePosition);
}

public boolean isSamePosition(Position comparePosition) {
return this.position.equals(comparePosition);
}
}
22 changes: 0 additions & 22 deletions src/main/java/race/RacingCarFactory.java

This file was deleted.

48 changes: 10 additions & 38 deletions src/main/java/race/RacingTrack.java
Original file line number Diff line number Diff line change
@@ -1,63 +1,42 @@
package race;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class RacingTrack {
private final int maxCarCount;
private final int maxAttemptCount;
private final List<RacingCar> cars;
private final TrackCondition trackCondition;
private final CarList cars;

public RacingTrack(int maxCarCount, int maxAttemptCount) {
this.maxCarCount = maxCarCount;
this.maxAttemptCount = maxAttemptCount;
this.cars = new ArrayList<>();
this.trackCondition = new TrackCondition(maxCarCount, maxAttemptCount);
this.cars = new CarList();
}

public void startRace(String[] carNames, int numOfAttempts) {
setupCars(carNames);

ResultView.printRaceStartMessage();
for (int i = 0; i < numOfAttempts; i++) {
moveAndShowCars();
ResultView.printRaceStatus(cars);
}
}

// getWinners() 메서드 테스틀 위하여 추가한 생성자
public void startRace(List<RacingCar> cars, int numOfAttempts) {
this.cars.addAll(cars);

ResultView.printRaceStartMessage();
for (int i = 0; i < numOfAttempts; i++) {
moveAndShowCars();
cars.moveWithRandom();
ResultView.printRaceStatus(cars);
}
}

private void setupCars(String[] carNames) {
for (String name : carNames) {
cars.add(RacingCarFactory.create(name, ResultView.createPositionPrinter()));
}
}

private void moveAndShowCars() {
for (RacingCar car : cars) {
car.move();
cars.add(new RacingCar(name));
}
}

public boolean validateCarCount(int num) {
return num >= 1 && num <= this.maxCarCount;
return this.trackCondition.validateCarCount(num);
}

public boolean validateAttemptCount(int num) {
return num >= 1 && num <= this.maxAttemptCount;
return this.trackCondition.validateAttemptCount(num);
}

public boolean validateCarNames(String[] names) {
if (names.length < 1 || names.length > this.maxCarCount) {
if (!this.trackCondition.validateCarCount(names.length)) {
return false;
}
for (String name : names) {
@@ -67,13 +46,6 @@ public boolean validateCarNames(String[] names) {
}

public List<RacingCar> getWinners() {
cars.sort(Collections.reverseOrder());
List<RacingCar> winners = new ArrayList<>();
winners.add(cars.get(0));

for (int i = 1; i < cars.size(); i++) {
if (cars.get(i).compareTo(cars.get(0)) == 0) winners.add(cars.get(i));
}
return winners;
return this.cars.getWinners();
}
}
5 changes: 0 additions & 5 deletions src/main/java/race/RandomNumberGenerator.java

This file was deleted.

17 changes: 4 additions & 13 deletions src/main/java/race/ResultView.java
Original file line number Diff line number Diff line change
@@ -3,30 +3,21 @@
import java.util.List;

public class ResultView {
public static PositionPrinter createPositionPrinter() {
return new PositionPrinter() {
@Override
public void printPosition(String carName, int position) {
System.out.println(String.format("%-5s", carName) + " : " + "-".repeat(position));
}
};
}

public static void printRaceStartMessage() {
System.out.println("\n### Racing Start!!! ###\n");
}

public static void printRaceStatus(List<RacingCar> cars) {
for (RacingCar car : cars) {
car.printPosition();
public static void printRaceStatus(CarList cars) {
for (RacingCar car : cars.getList()) {
System.out.println(String.format("%-5s", car.getName().toString()) + " : " + "-".repeat(car.getPosition().getValue()));
}
System.out.println();
}

public static void printRaceWinners(List<RacingCar> cars) {
String[] winners = new String[cars.size()];
for (RacingCar car : cars) {
winners[cars.indexOf(car)] = car.getName();
winners[cars.indexOf(car)] = car.getName().toString();
}
System.out.println(String.join(", ", winners) + "가 최종 우승했습니다.");
}
19 changes: 19 additions & 0 deletions src/main/java/race/TrackCondition.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package race;

public class TrackCondition {
private final int maxCarCount;
private final int maxAttemptCount;

public TrackCondition(int maxCarCount, int maxAttemptCount) {
this.maxCarCount = maxCarCount;
this.maxAttemptCount = maxAttemptCount;
}

public boolean validateCarCount(int num) {
return num >= 1 && num <= this.maxCarCount;
}

public boolean validateAttemptCount(int num) {
return num >= 1 && num <= this.maxAttemptCount;
}
}
13 changes: 0 additions & 13 deletions src/test/java/race/DefaultRandomNumberGeneratorTest.java

This file was deleted.

23 changes: 5 additions & 18 deletions src/test/java/race/RacingCarTest.java
Original file line number Diff line number Diff line change
@@ -8,29 +8,16 @@
public class RacingCarTest {
@Order(10)
@Test
void move() {
RacingCar car = RacingCarFactory.create("1", new FixedNumberGenerator(5));
assertThat(car.move()).isEqualTo(1);
assertThat(car.move()).isEqualTo(2);
}

@Order(20)
@Test
void not_move() {
RacingCar car = RacingCarFactory.create("1", new FixedNumberGenerator(2));
assertThat(car.move()).isEqualTo(0);
assertThat(car.move()).isEqualTo(0);
void moveWithSeed() {
RacingCar car = new RacingCar("1");
assertThat(car.moveWithSeed(5)).isEqualTo(new Position(1));
assertThat(car.moveWithSeed(2)).isEqualTo(new Position(1));
}

@Order(30)
@Test
void valid_car_name() {
void validate_car_name() {
assertThat(RacingCar.validateName("test")).isTrue();
}

@Order(40)
@Test
void invalid_car_name() {
assertThat(RacingCar.validateName("")).isFalse();
assertThat(RacingCar.validateName("aaaaaaa")).isFalse();
}
43 changes: 12 additions & 31 deletions src/test/java/race/RacingTrackTest.java
Original file line number Diff line number Diff line change
@@ -9,66 +9,47 @@

public class RacingTrackTest {
@Test
void valid_car_count() {
void validate_car_count() {
RacingTrack racingTrack = new RacingTrack(10, 10);
assertThat(racingTrack.validateCarCount(5)).isTrue();
}

@Test
void invalid_car_count() {
RacingTrack racingTrack = new RacingTrack(10, 10);
assertThat(racingTrack.validateCarCount(0)).isFalse();
assertThat(racingTrack.validateCarCount(15)).isFalse();
}

@Test
void valid_attempt_count() {
void validate_attempt_count() {
RacingTrack racingTrack = new RacingTrack(10, 10);
assertThat(racingTrack.validateAttemptCount(5)).isTrue();
}

@Test
void invalid_attempt_count() {
RacingTrack racingTrack = new RacingTrack(10, 10);
assertThat(racingTrack.validateAttemptCount(0)).isFalse();
assertThat(racingTrack.validateAttemptCount(15)).isFalse();
}

@Test
void valid_car_names() {
void validate_car_names() {
RacingTrack racingTrack = new RacingTrack(10, 10);
assertThat(racingTrack.validateCarNames(new String[]{"test1", "test2"})).isTrue();
}

@Test
void invalid_car_names() {
RacingTrack racingTrack = new RacingTrack(10, 10);
assertThat(racingTrack.validateCarNames(new String[]{"", "test2"})).isFalse();
assertThat(racingTrack.validateCarNames(new String[]{"testtesttest", "test2"})).isFalse();
assertThat(racingTrack.validateCarNames(new String[]{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11"})).isFalse();
}

@Test
void one_winner_test() {
RacingCar car1 = RacingCarFactory.create("1", new FixedNumberGenerator(5));
RacingCar car2 = RacingCarFactory.create("2", new FixedNumberGenerator(3));
List<RacingCar> cars = new ArrayList<>(List.of(car1, car2));
RacingTrack racingTrack = new RacingTrack(10, 10);
racingTrack.startRace(cars, 5);
List<RacingCar> winners = racingTrack.getWinners();
RacingCar car1 = new RacingCar("1", 4);
RacingCar car2 = new RacingCar("2", 2);
CarList cars = new CarList(new ArrayList<>(List.of(car1, car2)));
List<RacingCar> winners = cars.getWinners();
assertThat(winners.size()).isEqualTo(1);
assertThat(winners.get(0)).isEqualTo(car1);
}

@Test
void two_winner_test() {
RacingCar car1 = RacingCarFactory.create("1", new FixedNumberGenerator(5));
RacingCar car2 = RacingCarFactory.create("2", new FixedNumberGenerator(3));
RacingCar car3 = RacingCarFactory.create("3", new FixedNumberGenerator(5));
List<RacingCar> cars = new ArrayList<>(List.of(car1, car2, car3));
RacingTrack racingTrack = new RacingTrack(10, 10);
racingTrack.startRace(cars, 5);
List<RacingCar> winners = racingTrack.getWinners();
RacingCar car1 = new RacingCar("1", 4);
RacingCar car2 = new RacingCar("2", 2);
RacingCar car3 = new RacingCar("3", 4);
CarList cars = new CarList(new ArrayList<>(List.of(car1, car2, car3)));
List<RacingCar> winners = cars.getWinners();
assertThat(winners.size()).isEqualTo(2);
assertThat(winners.get(0)).isEqualTo(car1);
assertThat(winners.get(1)).isEqualTo(car3);