Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 11 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@

## 프로젝트 구조
```
- domain/
- Car.java : 자동차 객체 및 관련 메소드 정의
- CarRegistry.java : 자동차 목록 관리
- Game.java : 전반적인 게임 실행 담당
- RoundResult.java : 게임 결과를 담는 객체
- generator/
- NumberGenerator.java : 숫자 생성기 Interface 정의
- FixedNumberGenerator.java : 고정 숫자 생성기
- RandomNumberGenerator.java : 랜덤 숫자 생성기
- Car.java : 자동차 객체 정의
- CarRegistry.java : 자동차 목록 관리
- Game.java : 전반적인 게임 실행 담당
- NumberGenerator.java : 숫자 생성기 Interface 정의
- RandomNumberGenerator.java : 랜덤 숫자 생성기
- view/
- InputView.java : 사용자로부터 입력 받는 기능 수행
- ResultView.java : 사용자에게 출력 하는 기능 수행
- Main.java : Main Entrypoint
```

## 시작하기
Expand Down
41 changes: 0 additions & 41 deletions src/main/java/io/suhan/racingcar/Game.java

This file was deleted.

24 changes: 24 additions & 0 deletions src/main/java/io/suhan/racingcar/Main.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package io.suhan.racingcar;

import io.suhan.racingcar.domain.Car;
import io.suhan.racingcar.domain.Game;
import io.suhan.racingcar.domain.RoundResult;
import io.suhan.racingcar.view.InputView;
import io.suhan.racingcar.view.ResultView;
import java.util.List;

public class Main {
public static void main(String[] args) {
List<String> names = InputView.getCarNames();
int count = InputView.getTrialsCount();

Game game = Game.of(count);
game.getCarRegistry().registerCars(names);

List<RoundResult> results = game.execute();
List<Car> winners = game.getWinners();

ResultView.printExecutionResult(results);
Copy link

Choose a reason for hiding this comment

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

image

현재 프로그램은 위의 기능 요구사항을 만족하지 않는 것 같아요. RoundResult들의 각 상태를 다시 체크해보면 좋겠네요.
프로그램에서 코드를 잘 설계하는 일은 중요하지만, 그것보다 더 중요한 건 기획대로 기능이 잘 동작하는가? 인 것 같아요.

여기서 아래 질문에 대해 고민해볼 수 있을 것 같아요.

테스트 코드를 작성 중이긴 하나, 현재 테스트가 필요한 부분이 생각날때 마다 수작업으로 작성하고 있습니다. 따라서 경우에 따라 테스트 코드가 누락되는 상황이 생길 수도 있을 것 같은데, 이에 대한 대비 혹은 대응 방안이 궁금합니다!

현실적인 질문을 주신 것 같습니다. 되게 좋은 질문인데요.
말씀하신대로 테스트는 언제든 누락될 위험이 있습니다. 여기서 중요한 건 테스트가 누락됐다는 사실을 어떻게 인지할 것인가? 인 것 같아요. 저는 아래 두 가지 정도가 떠오르네요.

A. 테스트 커버리지 확인하기

테스트 커버리지는 내가 현재 작성한 테스트 코드가 비즈니스 코드의 몇 퍼센트를 커버하는가를 정량적으로 측정하는 방법이에요.
대부분의 IDE에는 Test with Coverage 라는 기능이 포함되어 있고, 이걸로 커버리지를 측정할 수 있어요. 그러면 현재 내 코드에서 어떤 부분이 테스트로부터 보호받지 못하는지 알아챌 수 있습니다.

B. 기능을 직접 사용해보며 문제가 없는지 확인하기

저는 테스트가 기능의 스펙을 잘 정의하는 일이라고 생각합니다. 한 기능이 가지고 있는 여러 스펙이 존재할텐데, 이걸 테스트로 녹여내서 그 기능이 의도대로 잘 동작하는지를 검증하는 거죠.
하지만 테스트 코드의 한계가 있다고 한다면, 그건 개발자가 예상 가능한 영역 안에서만 검증 가능하다는 점입니다. 어떤 기능에 의한 사이드 이펙트는 대부분 예상하지 못한 곳에서 발생하다보니 테스트 코드로 사전에 검증을 한다는 게 되게 어렵습니다. 이건 커버리지로도 체크하기 어려워요. 겉보기에는 코드 논리 전개에 문제가 없거든요.
여기서 수한님의 예시를 대입해보면, 논리적으로는 RoundResult에 문제가 없을 것이라 예상하셨겠지만, 자바의 동작 방식을 간과한 사례가 되겠네요.
이 한계를 극복할 수 있는 좋은 방법은 기능을 직접 사용해보며 테스트 해보는 거예요. 사용을 하다가 문제가 발생했다면, 버그를 재현하기 위한 방법과 원인을 찾고, 그 케이스에 대한 스펙을 테스트로 다시 정의하는 거죠. 예상하지 못했던 것을 예상할 수 있는 지점으로 끌고 올 수 있는 좋은 방법이라 생각합니다.

아무리 잘 만든 소프트웨어라도 사이드 이펙트는 필연적으로 존재합니다. 즉, 예상하지 못한 지점에서 장애가 날 수 있는 확률이 항상 존재한다는 얘기이기도 해요.
만약 문제가 발생했다면, 그 케이스를 커버하는 테스트를 추가해서 추후에 다시 그런 일이 발생하지 않도록 견고한 검증부를 만들어 나가는 것이 중요한 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

어제 오프라인 리뷰에서 테스트 코드 관련해서 승용님의 좋은 조언들을 들을 수 있어서 유익한 시간이었던 것 같습니다.
RoundResult의 경우 최초 구현에서는 정상적으로 잘 작동했으나, 리팩토링 단계에서 영향이 있었던 것으로 보입니다.. 의도치 않게 승용님께서 언급해주신 B. 기능을 직접 사용해보며 문제가 없는지 확인하기의 중요성을 다시 한번 확인하게 된 좋은 경험이 된 것 같습니다!

해당 부분의 경우 #157 (comment) 에서 언급해주신 부분을 수정하여 정상 작동함을 확인했습니다..!

ResultView.printWinners(winners);
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package io.suhan.racingcar;
package io.suhan.racingcar.domain;
Copy link

Choose a reason for hiding this comment

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

현재 프로젝트 구조가 Domain과 View로 구분되어 있는데, MVC 패턴의 경우 Model-View-Controller로 구성되어 있는 것으로 알고있습니다. 그렇다면 지금 Domain 구조의 경우 Model과 Controller가 합쳐있는 구조로 이해하면 되는 것일까요? 일단 구현 자체는 Model과 Controller가 혼합된 구조로 진행했습니다.

이 질문은 여기서 대화해보면 좋을 것 같습니다.

일단 구현 자체는 Model과 Controller가 혼합된 구조로 진행했습니다.

보기에는 수한님 나름대로 Model과 Controller를 정의하고 계신 것 같네요. 그렇지 않으면 혼합된 구조라는 사실조차 인지할 수 없을테니까요. 수한님은 Model과 Controller를 어떻게 정의하고 계신가요? 이걸 알려주시면 더 대화를 나눠볼 수 있을 것 같네요.

제 생각을 짧게 남겨보자면, Domain과 Controller는 포함 관계가 성립하지 않는 서로 다른 영역이라고 생각해요. 저는 Controller를 일종의 Entrypoint라고 생각해서, Main.java가 Controller의 역할을 하고 있는 게 아닐까? 하는 생각이 드네요.

Copy link
Author

Choose a reason for hiding this comment

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

보기에는 수한님 나름대로 Model과 Controller를 정의하고 계신 것 같네요.

네 맞습니다. 제 생각엔 Model의 경우 객체?의 정의만을 담고 있는 일종의 설계도 같은 것이라고 생각합니다. 그리고 Controller의 경우 Model을 기반으로 실제 비즈니스 로직이 구현되는 곳이라고 이해하고 있습니다. 그리하여 Car.java를 예로 들었을 때, Car의 정의에 필요한 필드(name, position)만 존재하는 것이 아니라, 실제 비즈니스 로직(move(), incrementPosition())이 혼재되어 있어 혼합된 구조라고 판단하게 되었습니다.

그래서 만약에 여기서 더 나누게 된다면 앞서 언급한 비즈니스 로직들만 가지고 있는 CarController를 추가로 만들어 Controller과 Model의 역할을 확실히 구분시키는 것이 MVC 패턴에 좀 더 부합하는 구조가 아닌가 하는 생각을 하고 있습니다..!


import io.suhan.racingcar.generator.NumberGenerator;
import io.suhan.racingcar.generator.RandomNumberGenerator;

public class Car {
private static final int CAR_MOVE_THRESHOLD = 4;
public static final int CAR_MOVE_THRESHOLD = 4;
public static final int CAR_NAME_MAXIMUM_LENGTH = 5;

private final String name;
private final NumberGenerator generator;
Expand All @@ -17,10 +18,14 @@ private Car(String name, NumberGenerator generator) {
}

public static Car of(String name) {
return new Car(name, new RandomNumberGenerator());
return Car.of(name, new RandomNumberGenerator());
}

public static Car of(String name, NumberGenerator generator) {
if (name.length() > CAR_NAME_MAXIMUM_LENGTH) {
throw new IllegalArgumentException("자동차의 이름은 " + CAR_NAME_MAXIMUM_LENGTH + "자 이하만 가능합니다.");
}

return new Car(name, generator);
}

Expand All @@ -43,4 +48,11 @@ public String getName() {
public int getPosition() {
return position;
}

public Car copy() {
Car copy = Car.of(this.name, this.generator);
copy.position = this.position;

return copy;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package io.suhan.racingcar;
package io.suhan.racingcar.domain;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -28,6 +28,22 @@ public void moveCars() {
}
}

public void registerCars(List<String> names) {
for (String name : names) {
Car car = Car.of(name);
cars.add(car);
}
}

public List<Car> getCarsWithBestPosition() {
int bestPosition = getBestPosition();

return getRegisteredCars()
.stream()
.filter((car) -> car.getPosition() == bestPosition)
.toList();
}

public int getBestPosition() {
return cars
.stream()
Expand Down
48 changes: 48 additions & 0 deletions src/main/java/io/suhan/racingcar/domain/Game.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package io.suhan.racingcar.domain;

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

public class Game {
private static final int GAME_ROUNDS_MINIMUM = 1;

private final CarRegistry carRegistry;
private final int rounds;

private Game(int rounds) {
this.carRegistry = CarRegistry.of();
this.rounds = rounds;
}

public static Game of() {
return Game.of(1);
}

public static Game of(int rounds) {
if (rounds < GAME_ROUNDS_MINIMUM) {
throw new IllegalArgumentException("시도 횟수는 " + GAME_ROUNDS_MINIMUM + " 이상이어야 합니다.");
}

return new Game(rounds);
}

public List<RoundResult> execute() {
List<RoundResult> results = new ArrayList<>();

for (int i = 0; i < rounds; i++) {
carRegistry.moveCars();
RoundResult result = RoundResult.of(carRegistry.getRegisteredCars());
results.add(result);
}

return results;
}

public List<Car> getWinners() {
return carRegistry.getCarsWithBestPosition();
}

public CarRegistry getCarRegistry() {
return carRegistry;
}
}
22 changes: 22 additions & 0 deletions src/main/java/io/suhan/racingcar/domain/RoundResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.suhan.racingcar.domain;

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

public class RoundResult {
private final List<Car> cars;

private RoundResult(List<Car> cars) {
this.cars = cars;
}

public static RoundResult of(List<Car> cars) {
List<Car> copied = cars.stream().map(Car::copy).collect(Collectors.toList());

return new RoundResult(copied);
}

public List<Car> getCars() {
return cars;
}
}
22 changes: 22 additions & 0 deletions src/main/java/io/suhan/racingcar/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.suhan.racingcar.view;

import java.util.Arrays;
import java.util.List;
import java.util.Scanner;

public class InputView {
private static final Scanner scanner = new Scanner(System.in);

public static List<String> getCarNames() {
System.out.println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).");
String input = scanner.next();

return Arrays.stream(input.split(",")).toList();
}

public static int getTrialsCount() {
System.out.println("시도할 회수는 몇회인가요?");

return scanner.nextInt();
}
}
30 changes: 30 additions & 0 deletions src/main/java/io/suhan/racingcar/view/ResultView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.suhan.racingcar.view;

import io.suhan.racingcar.domain.Car;
import io.suhan.racingcar.domain.RoundResult;
import java.util.List;
import java.util.stream.Collectors;

public class ResultView {
public static void printExecutionResult(List<RoundResult> results) {
System.out.println("\n실행 결과");
results.forEach((result) -> printRoundResult(result.getCars()));
}

private static void printRoundResult(List<Car> cars) {
cars.forEach(ResultView::printCurrentPosition);
System.out.println(); // newline
}

public static void printWinners(List<Car> winners) {
System.out.println(winners.stream().map(Car::getName).collect(Collectors.joining(", ")) + "가 최종 우승했습니다.");
}

private static void printCurrentPosition(Car car) {
System.out.println(car.getName() + " : " + buildProgressBar(car.getPosition()));
}

private static String buildProgressBar(int value) {
return "-".repeat(Math.max(0, value));
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.suhan.racingcar;
package io.suhan.racingcar.domain;

import static io.suhan.racingcar.domain.Car.CAR_MOVE_THRESHOLD;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import io.suhan.racingcar.generator.FixedNumberGenerator;
import org.junit.jupiter.api.DisplayNameGeneration;
Expand All @@ -22,7 +24,7 @@ public class CarTest {

@Test
void 생성된_값이_4_이상일_경우_자동차가_움직일_수_있다() {
FixedNumberGenerator generator = new FixedNumberGenerator(4);
FixedNumberGenerator generator = new FixedNumberGenerator(CAR_MOVE_THRESHOLD);
Car car = Car.of("neo", generator);

car.move();
Expand All @@ -34,7 +36,7 @@ public class CarTest {

@Test
void 생성된_값이_3_이하일_경우_자동차가_멈춘다() {
FixedNumberGenerator generator = new FixedNumberGenerator(3);
FixedNumberGenerator generator = new FixedNumberGenerator(CAR_MOVE_THRESHOLD - 1);
Car car = Car.of("neo", generator);

car.move();
Expand All @@ -43,4 +45,9 @@ public class CarTest {

assertEquals(0, position);
}

@Test
void 자동차의_이름은_5자_이하만_가능하다() {
assertThrows(IllegalArgumentException.class, () -> Car.of("123456"));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package io.suhan.racingcar;
package io.suhan.racingcar.domain;

import static io.suhan.racingcar.domain.Car.CAR_MOVE_THRESHOLD;
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import io.suhan.racingcar.generator.FixedNumberGenerator;
import java.util.List;
Expand All @@ -17,11 +19,7 @@ public class GameTest {

Game game = Game.of();

for (String name : names) {
Car car = Car.of(name);

game.getCarRegistry().register(car);
}
game.getCarRegistry().registerCars(names);

List<String> registeredNames = game.getCarRegistry().getRegisteredCars().stream().map(Car::getName).toList();

Expand All @@ -33,8 +31,8 @@ public class GameTest {
// given
Game game = Game.of(5);

FixedNumberGenerator forwardGenerator = new FixedNumberGenerator(4);
FixedNumberGenerator stopGenerator = new FixedNumberGenerator(3);
FixedNumberGenerator forwardGenerator = new FixedNumberGenerator(CAR_MOVE_THRESHOLD);
FixedNumberGenerator stopGenerator = new FixedNumberGenerator(CAR_MOVE_THRESHOLD - 1);

Car neo = Car.of("neo", forwardGenerator);
Car brie = Car.of("brie", stopGenerator);
Expand All @@ -47,10 +45,15 @@ public class GameTest {
List<String> expectedNames = List.of("neo", "brown");

// when
game.start();
game.execute();
List<String> winnerNames = game.getWinners().stream().map(Car::getName).toList();

// then
assertIterableEquals(expectedNames, winnerNames);
}

@Test
void 시도_횟수는_1_이상만_가능하다() {
assertThrows(IllegalArgumentException.class, () -> Game.of(0));
}
}