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

[2단계 - 자동차 경주 리팩터링] 테오(최우성) 미션 제출합니다. #590

Merged
merged 16 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f48daf2
refactor: 우승자 반환 시 자동차가 없는 경우 디테일한 예외를 던지도록 변경
woosung1223 Feb 11, 2023
89cb13a
refactor: 상수 필드를 모두 static final로 변경
woosung1223 Feb 12, 2023
20f96b4
refactor: 예외메세지 출력을 View의 책임으로 변경
woosung1223 Feb 12, 2023
cd556c7
refactor: OutputView을 ConsoleView로 리네이밍
woosung1223 Feb 12, 2023
b505edf
refactor: 시도 횟수 검증 메소드 네이밍을 이해하기 쉽도록 수정
woosung1223 Feb 12, 2023
cb18855
refactor: 자동차 비교 시 getter를 사용하지 않도록 수정
woosung1223 Feb 12, 2023
fc80f72
refactor: 자동차 이름 VO 생성
woosung1223 Feb 12, 2023
a105917
refactor: 자동차 이름 객체 getter명 변경
woosung1223 Feb 12, 2023
1e1d002
refactor: 자동차 위치 정보를 관리하는 VO 생성
woosung1223 Feb 12, 2023
818ea37
refactor: 자동차 위치를 외부에서 주입받도록 수정
woosung1223 Feb 12, 2023
104dde3
refactor: 시도 횟수를 객체로 다루도록 수정
woosung1223 Feb 12, 2023
42373a9
refactor: 우승자 테스트 로직에 가독성 부여
woosung1223 Feb 12, 2023
75a4012
docs: 변경사항 및 고민사항 기재
woosung1223 Feb 12, 2023
4d5ac66
refactor: 코인을 사용할 때 유효성 검증 추가
woosung1223 Feb 13, 2023
cee3f17
refactor: getter 명을 이해하기 쉽도록 변경
woosung1223 Feb 13, 2023
df57ff2
refactor: 커스텀 예외를 사용해 도메인에서 뷰 의존성 제거
woosung1223 Feb 14, 2023
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
28 changes: 28 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,31 @@
- [x] 문구들을 어디서 출력할지.
- [x] OutputView에서 printGameResultMessage 메소드는 적절한지?
- [x] 유효성 검증을 도메인에서 할지, 입력받을 때 할지.


---
#### 2단계 변경사항
1. 상수 필드를 모두 static final로 변경.
2. 우승자 검출을 위한 자동차 비교 시 getter를 전혀 사용하지 않도록 수정.
3. 도메인과 컨트롤러 영역에서 View에 대해 전혀 모르도록 예외 메세지를 출력하는 부분까지 OutputView에게 위임함. <br>
4. 자동차 이름을 관리하는 VO를 생성했음. 자동차 이름에 대한 제약조건도 여럿 존재하고, <br>
이것을 자동차 객체가 다루기보다는 `carName` 객체가 다루는 것이 보다 적절한 책임 분배라고 생각했다. <br>
비슷한 이유로 `Position` VO를 분리함.
5. 시도 횟수를 다루는 객체를 둬서 프로덕션 및 테스트 코드를 간결하게 변경.


#### 2단계 고민사항
1. 어느정도까지 확장성있게 설계하는게 옳을까? 얼마나 확장해야 오버프로그래밍이 되지 않을까? <br>
예를 들어서, OutputView가 지금은 `한국어로 콘솔에 출력`하는 것이지만, `영어로 콘솔에 출력` 된다거나, 콘솔이 아닌 다른 곳에 출력이 될 수도 있을 것이다. <br>
그러면 OutputView 위에 부모 클래스나 Interface를 정의하는 것이 낫지 않을까? 하지만 현재 문제 해결 상황에서는 전혀 필요하지 않다. <br>
오버 프로그래밍과 유연함 사이의 적정선은 어디인가? <br> <br>

2. VO의 경우 메소드 및 프로퍼티의 네이밍은 어떻게 해야할까? <br>
CarName의 경우, 프로퍼티 및 getter의 이름은 어떻게 해야 좋을지 고민. <br>
프로퍼티 이름은 `carName`으로 했는데, 클래스명과 중복이 된다는 점에서 애매하다는 느낌을 받았음. <br>
또한 비슷한 이유에서 getter 이름을 `getCarName`이라고 하지 않고 `get` 이라고 정의했는데(carName.get() 처럼 사용될 수 있도록) <br>
괜찮은 방법인지도 고민! <br> <br>

3. 현재 입력을 위한 출력이 InputView에서 수행되고 있는데, <br>
View가 콘솔창이 아니게 된다거나 하는 경우에는 InputView까지 수정이 되어야 한다. <br>
그러면 InputView에서도 출력이 필요할 때 OutputView를 호출해야 하지 않을까? <br>
4 changes: 2 additions & 2 deletions src/main/java/Application.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import controller.RacingController;
import view.input.InputView;
import view.output.OutputView;
import view.output.ConsoleView;

import java.util.Scanner;

public class Application {
public static void main(String[] args) {
Scanner scanner = new Scanner(System.in);
new RacingController(new InputView(scanner), new OutputView()).start();
new RacingController(new InputView(scanner), new ConsoleView()).start();
}
}
48 changes: 24 additions & 24 deletions src/main/java/controller/RacingController.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,64 +3,64 @@
import domain.RacingGame;
import domain.RandomNumberGenerator;
import view.input.InputView;
import view.output.OutputView;
import view.output.ConsoleView;

import java.util.List;

public class RacingController {

private final InputView inputView;
private final OutputView outputView;
private final ConsoleView consoleView;
private RacingGame racingGame;

public RacingController(InputView inputView, OutputView outputView) {
public RacingController(InputView inputView, ConsoleView consoleView) {
this.inputView = inputView;
this.outputView = outputView;
this.consoleView = consoleView;
}

public void start() {
try {
makeRacingGame(readCarNames());
startRacingGame(readGameTry());
makeRacingGame(readCarNames(), readGameTry());
startRacingGame();
makeRacingGameResult();
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
} catch (RuntimeException e) {
consoleView.printExceptionMessage(e.getMessage());
start();
}
}

private void makeRacingGame(List<String> carNames) {
this.racingGame = new RacingGame(carNames, new RandomNumberGenerator());
}

private List<String> readCarNames() {
try {
return inputView.readCarName();
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
consoleView.printExceptionMessage(e.getMessage());
return readCarNames();
}
}

private void startRacingGame(int gameTry) {
outputView.printGameResultMessage();
for (int i = 0; i < gameTry; i++) {
racingGame.start();
outputView.printRacingStatus(racingGame.getCars());
}
}

private int readGameTry() {
try {
return inputView.readGameTry();
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
consoleView.printExceptionMessage(e.getMessage());
return readGameTry();
}
}

private void makeRacingGame(List<String> carNames, int gameTry) {
this.racingGame = new RacingGame(carNames, gameTry, new RandomNumberGenerator());
}

private void startRacingGame() {
consoleView.printGameResultMessage();
while (racingGame.isGameOnGoing()) {
racingGame.start();
consoleView.printRacingStatus(racingGame.getCars());
}
}

private void makeRacingGameResult() {
outputView.printRacingStatus(racingGame.getCars());
outputView.printRacingWinners(racingGame.getWinners());
consoleView.printRacingStatus(racingGame.getCars());
consoleView.printRacingWinners(racingGame.getWinners());
}
}
41 changes: 13 additions & 28 deletions src/main/java/domain/Car.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,32 @@

public class Car {

private final String CAR_NAME_LENGTH_EXCEED = "[ERROR] 자동차 이름의 길이는 1자 이상, 5자 이하여야 합니다.";
private final String CAR_NAME_EMPTY = "[ERROR] 자동차의 이름은 공백이면 안됩니다.";
private final CarName carName;
private final Position position;

private final int MAX_CAR_NAME_LENGTH = 5;

private int position;
private final String carName;

public Car(String carName) {
validateCarName(carName);
this.carName = carName;
position = 0;
}

public void validateCarName(String carName) {
validateCarNameIsNotEmpty(carName);
validateCarNameLength(carName);
public Car(String name, int position) {
this.carName = new CarName(name);
this.position = new Position(position);
}

private void validateCarNameIsNotEmpty(String carName) {
if (carName.isBlank()) {
throw new IllegalArgumentException(CAR_NAME_EMPTY);
}
public void move() {
position.moveForward();
}

private void validateCarNameLength(String carName) {
if (carName.length() > MAX_CAR_NAME_LENGTH) {
throw new IllegalArgumentException(CAR_NAME_LENGTH_EXCEED);
}
public int comparePosition(Car otherCar) {
return this.getPosition() - otherCar.getPosition();
}

public void move() {
position++;
public boolean hasSamePositionWith(Car otherCar) {
return comparePosition(otherCar) == 0;
}

public int getPosition() {
return position;
return position.getPosition();
}

public String getCarName() {
return carName;
return carName.getCarName();
}
}

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

import exception.CarNameBlankException;
import exception.CarNameLengthException;

public class CarName {

private static final int MAX_CAR_NAME_LENGTH = 5;

private final String carName;

public CarName(String carName) {
validateCarName(carName);
this.carName = carName;
}

private void validateCarName(String carName) {
validateCarNameIsNotEmpty(carName);
validateCarNameLength(carName);
}

private void validateCarNameIsNotEmpty(String carName) {
if (carName.isBlank()) {
throw new CarNameBlankException();
}
}

private void validateCarNameLength(String carName) {
if (carName.length() > MAX_CAR_NAME_LENGTH) {
throw new CarNameLengthException();
}
}

public String getCarName() {
return carName;
}
}
20 changes: 20 additions & 0 deletions src/main/java/domain/Coin.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package domain;

public class Coin {
Copy link

Choose a reason for hiding this comment

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

VO라는 용어를 사용하셔서 질문드려요 :)
Coin은 VO라고 생각하고 모델링 하신걸까요?
VO (Value Object) 이외엔 어떠한 유형의 객체 모델이 존재하나요?

Copy link
Member Author

@woosung1223 woosung1223 Feb 13, 2023

Choose a reason for hiding this comment

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

CarName이나 Position은 VO라고 생각하지만, Coin객체는 VO라고 생각하지 않았습니다!

제가 생각하는 VO의 정의는 다음과 같습니다.
본래 객체로 나타내지 않아도 될 값들을 제약조건이나 동등성 비교를 위해 객체로 다루는 것

즉, 어떤 값들을(돈, 나이, 개수 등) 원시형으로 다룰 수도 있겠지만,
일부러 객체로 구성해 의미를 부여하는 것이 Value Object의 목적이라고 생각했습니다.

Coin 객체는 remaining 이 같다고 해도 같은 객체라고 보장할 수 없고,
값을 나타내기 위해 탄생한 객체가 아니기 때문에 VO라고 생각하지 않았습니다.

또한 조사해 본 결과, VO 이외에 DTO, DAO, Entity, PO, BO, SO 등의 객체 모델이 존재하는 것 같습니다..!

Copy link

Choose a reason for hiding this comment

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

값을 나타내기 위해 탄생한 객체가 아니기 때문에 VO라고 생각하지 않았습니다.

(전 VO같아요 🙃)
이 부분에 대해선 따로 더 의견 드리진 않겠습니다 :)
용어에 대한 해석이 제 각각이고, 각자의 해석에 따라 여러 자료들이 재생산되다보니 여러 개념이 뒤섞이게 되는것 같은데요.
이 부분은 고민해보면서 테오 나름의 개념을 만들어 가셨으면 좋겠습니다.
그리고 말씀주신 유형중, VO와 같은 선상에 놓고 비교해야하는 객체는 Entity라고 생각합니다. (PO, BO, SO는 처음 들어보네요. 저도 공부해볼게요)


private int remaining;

public Coin(int remaining) {
this.remaining = remaining;
}

public void use() {
Copy link

Choose a reason for hiding this comment

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

use() 이전에 isLeft() 검사를 해주는 것도 좋을것 같네요.
도메인 모델의 메서드엔 도메인 규칙을 어기지 않도록 신경써서 만들어주는 것이 좋습니다.

if (isLeft()) {
remaining--;
}
}

public boolean isLeft() {
return remaining > 0;
}
}
31 changes: 31 additions & 0 deletions src/main/java/domain/Position.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package domain;

import exception.PositionInvalidException;

public class Position {

private int position;

public Position(int position) {
validatePosition(position);
this.position = position;
}

private void validatePosition(int position) {
validatePositionIsNotNegative(position);
}

private void validatePositionIsNotNegative(int position) {
if (position < 0) {
throw new PositionInvalidException();
}
}

public void moveForward() {
position++;
}

public int getPosition() {
return position;
}
}
23 changes: 16 additions & 7 deletions src/main/java/domain/RacingGame.java
Original file line number Diff line number Diff line change
@@ -1,29 +1,34 @@
package domain;

import exception.NoCarsExistException;

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

public class RacingGame {

private final int MOVABLE_BOUND = 4;
private static final int DEFAULT_START_LINE = 0;
private static final int MOVABLE_BOUND = 4;

private final List<Car> cars;
private final NumberGenerator numberGenerator;
private final Coin gameCoin;

public RacingGame(List<String> splitCarNames, NumberGenerator numberGenerator) {
public RacingGame(List<String> splitCarNames, int gameTry, NumberGenerator numberGenerator) {
cars = splitCarNames.stream()
.map(Car::new)
.map(carName -> new Car(carName, DEFAULT_START_LINE))
.collect(Collectors.toList());

gameCoin = new Coin(gameTry);
this.numberGenerator = numberGenerator;
}

public void start() {
for (Car car : cars) {
moveCar(car);
}
gameCoin.use();
}

private void moveCar(Car car) {
Expand All @@ -33,6 +38,10 @@ private void moveCar(Car car) {
}
}

public boolean isGameOnGoing() {
return gameCoin.isLeft();
}

public List<Car> getCars() {
return Collections.unmodifiableList(cars);
}
Expand All @@ -41,13 +50,13 @@ public List<Car> getWinners() {
Car furthestCar = getFurthestCar();

return cars.stream()
.filter(car -> car.getPosition() == furthestCar.getPosition())
.filter(car -> car.hasSamePositionWith(furthestCar))
.collect(Collectors.toList());
}

private Car getFurthestCar() {
return cars.stream()
.max(Comparator.comparingInt(Car::getPosition))
.orElseThrow(RuntimeException::new);
.max(Car::comparePosition)
.orElseThrow(NoCarsExistException::new);
}
}
2 changes: 1 addition & 1 deletion src/main/java/domain/RandomNumberGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

public class RandomNumberGenerator implements NumberGenerator {

private final int DIGIT_MAX = 10;
private static final int DIGIT_MAX = 10;

public int makeDigit() {
Random random = new Random();
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/exception/CarNameBlankException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package exception;

public class CarNameBlankException extends RuntimeException {

private static final String MESSAGE = "[ERROR] 자동차의 이름은 공백이면 안됩니다.";
Copy link

Choose a reason for hiding this comment

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

이 부분도 결국엔 도메인이 어떤 메세지를 노출시켜야할지 정하는 구조이지 않을까요?
의존의 방향으로 생각하면 현재,
도메인 모델 -> 예외 -> 메세지
인것 같은데요, 예외에 따라 View가 메세지를 처리하도록 변경이 가능해 보입니다 :)

고민해보시고 마땅한 방법이 떠오르지 않으신다면 DM 주세요


public CarNameBlankException() {
super(MESSAGE);
}
}
10 changes: 10 additions & 0 deletions src/main/java/exception/CarNameLengthException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package exception;

public class CarNameLengthException extends RuntimeException {

private static final String MESSAGE = "[ERROR] 자동차 이름의 길이는 1자 이상, 5자 이하여야 합니다.";

public CarNameLengthException() {
super(MESSAGE);
}
}
10 changes: 10 additions & 0 deletions src/main/java/exception/NoCarsExistException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package exception;

public class NoCarsExistException extends RuntimeException {

private final static String MESSAGE = "[ERROR] 자동차가 존재하지 않습니다.";

public NoCarsExistException() {
super(MESSAGE);
}
}
Loading