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

[BE] 레이싱게임_고준보 미션 연습 #520

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
21c5320
docs: 구현할 기능 목록 정리
Ridealist Nov 30, 2022
b641c7a
feat: 전체 클래스 구조 밑그림 및 패키지 분리
Ridealist Nov 30, 2022
bb6e722
feat: 전체 프로그램 skeleton code 구현
Ridealist Nov 30, 2022
493ccd0
feat(NumberGenerator): 랜덤 숫자 생성 인터페이스 및 구현 클래스 구현
Ridealist Nov 30, 2022
c4ca5c0
feat(Car): 랜덤 숫자에 따른 이동, 정지 메소드 구현
Ridealist Nov 30, 2022
cf186f5
feat(Car): getter 메소드 구현
Ridealist Nov 30, 2022
5a4a078
test(CarTest): 랜덤 숫자에 따른 이동 메서드 단위 테스트 구현
Ridealist Nov 30, 2022
7aace4b
refactor: domain 내에 car 패키지 추가 생성, 클래스 분리
Ridealist Nov 30, 2022
c1407e9
feat(Player): Car 객체를 담아 전체 참가자를 관리하는 클래스 구현
Ridealist Nov 30, 2022
f2c0781
feat(RacingGame): RacingGame 관리하는 객체 구현
Ridealist Nov 30, 2022
688647f
test(CarTest): 패키지 변화 적용 및 TODO 작성
Ridealist Nov 30, 2022
ff0fafa
feat(InputValidator): 입력값 유효성 검증 클래스 구현
Ridealist Nov 30, 2022
0c00aad
feat(InputView): 사용자 입력값 요청 클래스 구현
Ridealist Nov 30, 2022
91b6063
feat(OutputView): 콘솔 출력 로직 클래스 구현
Ridealist Nov 30, 2022
3ef0908
feat(GameResult): 게임 상황 및 최종 결과 반환 클래스 구현
Ridealist Nov 30, 2022
2a30bbc
fix(InputView): player 이름에 빈칸 출력되는 문제 해결
Ridealist Nov 30, 2022
aee2952
chore: exception 메세지 내용 수정
Ridealist Nov 30, 2022
a4af3a4
chore: exception 메세지 내용 수정, 빈칸 추가
Ridealist Nov 30, 2022
3bbd941
feat(GameController): controller 클래스 구현
Ridealist Nov 30, 2022
95808b3
feat(Application): 전체 기능 구현 완료
Ridealist Nov 30, 2022
68e5ee5
docs: 구현할 기능 목록 전체 체크 완료
Ridealist Nov 30, 2022
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
38 changes: 38 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# 구현할 기능 목록

## 0. 전체 예외 처리
- [x] 사용자가 잘못된 값을 입력할 경우 `IllegalArgumentException`를 발생시키고, "[ERROR]"로 시작하는 에러 메시지를 출력 후 그 부분부터 입력을 다시 받는다.
- [x] 아래의 프로그래밍 실행 결과 예시와 동일하게 입력과 출력이 이루어져야 한다.


## 1. 경주할 자동차
- [x] 각 자동차에 이름을 부여할 수 있다. 전진하는 자동차를 출력할 때 자동차 이름을 같이 출력한다.
### 입력
- [x] 자동차 이름들 입력
### 예외처리
- [x] 자동차 이름은 쉼표(,)를 기준으로 구분
- [x] 이름은 5자 이하만 가능
### 단위테스트
- [x]

## 2. 경주 방법
### 입력
- [x] 시도할 횟수 입력
### 예외처리
- [x] 숫자여야 함
### 단위테스트
- [x]

## 3. 실행 과정
- [x] 주어진 횟수 동안 n대의 자동차는 전진 또는 멈출 수 있음
- [x] 전진하는 조건은 0에서 9 사이에서 무작위 값을 구한 후 무작위 값이 4 이상일 경우
### 출력
- [x] 각 차수별 실행 결과


## 4. 실행 결과
- [x] 자동차 경주 게임을 완료한 후 누가 우승했는지를 알려준다. 우승자는 한 명 이상일 수 있다.
- [x] 우승자가 여러 명일 경우 쉼표(,)를 이용하여 구분한다.
### 출력
- [x] 단독 우승자 안내 문구
- [x] 공동 우승자 안내 문구
5 changes: 4 additions & 1 deletion src/main/java/racingcar/Application.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package racingcar;

import racingcar.controller.GameController;

public class Application {
public static void main(String[] args) {
// TODO 구현 진행
GameController gameController = new GameController();
gameController.start();
}
}
12 changes: 0 additions & 12 deletions src/main/java/racingcar/Car.java

This file was deleted.

51 changes: 51 additions & 0 deletions src/main/java/racingcar/controller/GameController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package racingcar.controller;

import java.util.List;
import java.util.function.Supplier;
import racingcar.domain.GameResult;
import racingcar.domain.Player;
import racingcar.domain.RacingGame;
import racingcar.domain.car.NumberGenerator;
import racingcar.domain.car.RandomNumberGenerator;
import racingcar.view.InputView;
import racingcar.view.OutputView;

public class GameController {

private RacingGame racingGame;
private GameResult gameResult;

public GameController() {
setUp();
}

private void setUp() {
InputView inputView = new InputView();
List<String> playerNames = repeat(inputView::readPlayerName);
Player player = new Player(playerNames);
this.gameResult = new GameResult(player);
int gameRound = repeat(inputView::readGameRound);
this.racingGame = new RacingGame(player, gameRound);
Comment on lines +23 to +28
Copy link

Choose a reason for hiding this comment

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

setUp() 메서드 내부의 기능들도 분리해보시는 것이 어떨까요?

setUp() 메서드는 결국 RacingGame 생성을 최종 목표로 하고 있네요.
그렇다면 RacingGame 이 생성되는 과정들을 적절하게 묶어서 메서드로 분리해보시는 것이
가독성 측면에서 더 좋을 것 같습니다!

}

public void start() {
OutputView outputView = new OutputView();
outputView.printResultMessage();
do {
NumberGenerator numberGenerator = new RandomNumberGenerator();
racingGame.play(numberGenerator);
outputView.printRoundResult(gameResult);
} while (racingGame.isGameContinue());
outputView.printFinalWinner(gameResult);
}

private <T> T repeat(Supplier<T> inputReader) {
try {
return inputReader.get();
} catch (IllegalArgumentException e) {
OutputView outputView = new OutputView();
outputView.printErrorMessage(e.getMessage());
return repeat(inputReader);
}
}
}
41 changes: 41 additions & 0 deletions src/main/java/racingcar/domain/GameResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package racingcar.domain;

import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import racingcar.domain.car.Car;

public class GameResult {

private final Player player;
private Map<String, Integer> playerPosition = new LinkedHashMap<>();
Copy link

Choose a reason for hiding this comment

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

일급 컬렉션을 사용하는 것도 좋다고 생각해요!

추가로 playerPosition 상태를 활용하신 이유가 궁금합니다!

게임이 진행될 때마다 Car 객체 내부에서 nameposition 값을 불러와서 저장하는 용도로 사용하고 계신 것 같아요. 마치 repository 처럼요! 그런데 굳이 실시간으로 변하는 position 값을 따로 저장할 필요가 있을 지 고민해보시는 게 좋을 것 같아요! 이미 Car 객체가 최신화 된 position 값을 저장하고 있기 때문에 이를 활용하시는 게 더 합리적일 것 같습니다!

게임 결과를 편하게 출력하기 위한 용도로 playerPosition 을 사용하신 건 아닐까? 라는 생각도 드네요. 그럼에도 GameResult 보다는 RacingGame 이나, Player 객체에서 라운드 마다 게임 결과를 반환하는 책임을 수행하는 게 더 적절해 보여요!

GameResultRacingGame 두 객체 모두 Player에 의존하고 있어요. 이런 경우에 해당 객체에 적절한 책임이 부여됐는지 한번 고민해보시면 좋을 것 같습니다!


public GameResult(Player player) {
this.player = player;
}

public Map<String, Integer> getPlayerPosition() {
updatePosition();
return playerPosition;
}

private void updatePosition() {
List<Car> racingCars = player.getRacingCars();
for (Car car : racingCars) {
playerPosition.put(car.getName(), car.getPosition());
}
}

public List<String> getFinalWinner() {
List<String> finalWinner = new ArrayList<>();
int maxValue = Collections.max(playerPosition.values());
for (Map.Entry<String, Integer> m : playerPosition.entrySet()) {
if (m.getValue() == maxValue) {
finalWinner.add(m.getKey());
}
}
return finalWinner;
}
}
34 changes: 34 additions & 0 deletions src/main/java/racingcar/domain/Player.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package racingcar.domain;

import java.util.ArrayList;
import java.util.List;
import racingcar.domain.car.Car;
import racingcar.domain.car.NumberGenerator;

public class Player {
Copy link

Choose a reason for hiding this comment

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

Player 내부에서 복수의 Car 객체들을 다루고 있네요!
Player 보단 Players 처럼 복수형으로 이름을 지어주시는 게 더 타당해보입니다!

Copy link
Author

@Ridealist Ridealist Dec 4, 2022

Choose a reason for hiding this comment

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

네 동의합니다. 일반적으로 클래스명에 단수를 써야한다는(?) 저만의 강박이 있었는데, 한준님 피드백 덕분에 제 고정관념을 깨게 되었습니다. stackoverflow에서도 제 생각과 비슷한 고민을 한 분이 있었네요...ㅎㅎ 감사합니다!
https://stackoverflow.com/questions/11673750/is-it-good-practice-for-java-class-names-to-be-plural


private final List<Car> racingCars;

public Player(List<String> playerNames) {
this.racingCars = setRacingCars(playerNames);
}

private List<Car> setRacingCars(List<String> playerNames) {
List<Car> racingCars = new ArrayList<>();
for (String name : playerNames) {
Car car = new Car(name);
racingCars.add(car);
}
return racingCars;
}

public void playOneRound(NumberGenerator numberGenerator) {
for (Car car : racingCars) {
car.moveOrStop(numberGenerator);
}
}

public List<Car> getRacingCars() {
return racingCars;
}
Comment on lines +31 to +33
Copy link

Choose a reason for hiding this comment

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

getter()로 리스트를 그대로 반환하는 것인 정말 위험하다고 생각해요!
리스트를 반환하더라도 실제로 반환되는 것은 해당 리스트의 참조 값입니다.

만약 다음과 같은 코드가 작성되면 외부에서 Player 내부의 상태가 쉽게 바뀔 수 있어요.

List<Car> cars = player.getRacingCars();
cars = new ArrayList<Car>();

따라서 리스트를 반환할 때는 불변 객체로 변환해주시는 게 안전해요!
Collections.unmodifiableList() 를 활용해보시는 것을 추천드립니다!

Copy link
Author

@Ridealist Ridealist Dec 4, 2022

Choose a reason for hiding this comment

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

한준님의 코드에서 이 부분의 의도가 궁금했는데, 이런 깊은 뜻이 있었군요! 조언 정말 감사합니다:)

한준님 덕분에 더 공부해봤는데, List 필드에 값을 지정할 때 아예 unmodifiableList를 적용해주면 어떨까 싶어
자료들 찾아봤고, 아래 자료 읽어보면 좋을 것 같아 공유드립니다:)
남겨주신 리뷰로 많은 공부 했습니다 감사합니다!

Copy link
Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

좋은 자료 감사합니다!! 일급 컬렉션을 반환할 때 뿐만 아니라 생성할 떄도 불변성을 고려해줘야 하는군요...!
덕분에 방어적 복사에 대해서도 알 수 있었어요! 정말 감사합니다!!!! 👍👍 👍 👍 👍

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

import racingcar.domain.car.NumberGenerator;

public class RacingGame {

private static final int GAME_OVER_COUNT = 0;

private final Player player;
private int gameRound;

public RacingGame(Player player, int gameRound) {
this.player = player;
this.gameRound = gameRound;
}

public void play(NumberGenerator numberGenerator) {
player.playOneRound(numberGenerator);
gameRound--;
}

public boolean isGameContinue() {
return (gameRound != GAME_OVER_COUNT);
}
}
32 changes: 32 additions & 0 deletions src/main/java/racingcar/domain/car/Car.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package racingcar.domain.car;

/*
Car 기본 생성자를 추가할 수 없다.
name, position 변수의 접근 제어자인 private을 변경할 수 없다.
가능하면 setPosition(int position) 메소드를 추가하지 않고 구현한다.
*/

public class Car {

private static final int MOVING_CRITERIA = 4;
private final String name;
private int position = 0;

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

public void moveOrStop(NumberGenerator numberGenerator) {
if (numberGenerator.generate() >= MOVING_CRITERIA) {
Copy link

Choose a reason for hiding this comment

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

해당 조건문을 메서드로 분리해서 어떤 역할을 하는 지 이름을 지어주는 것이 좋다고 생각해요!

Copy link
Author

Choose a reason for hiding this comment

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

한준님 말씀에 동의합니다. moveOrStop 이름이 1.이동할 수 있을지 판단하는 역할과 2.자동차를 이동시키는 역할 2가지를 함께 하고 있어, 한가지 역할만 해야하는 메소드 원칙에 위배되었네요... 피드백 감사합니다:)

position++;
}
}

public String getName() {
return name;
}

public int getPosition() {
return position;
}
}
7 changes: 7 additions & 0 deletions src/main/java/racingcar/domain/car/NumberGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package racingcar.domain.car;

@FunctionalInterface
public interface NumberGenerator {

int generate();
}
14 changes: 14 additions & 0 deletions src/main/java/racingcar/domain/car/RandomNumberGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package racingcar.domain.car;

import camp.nextstep.edu.missionutils.Randoms;

public class RandomNumberGenerator implements NumberGenerator {

private static final int LOWER_BOUND = 0;
private static final int UPPER_BOUND = 9;

@Override
public int generate() {
return Randoms.pickNumberInRange(LOWER_BOUND, UPPER_BOUND);
}
}
38 changes: 38 additions & 0 deletions src/main/java/racingcar/utils/InputValidator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package racingcar.utils;

public final class InputValidator {
private static final int MAX_NAME_LENGTH = 5;
private static final String COMMA = ",";

private static final String NOT_IN_COMMA = "자동차 이름을 쉼표(,)로 구분해 적어주세요.";
private static final String INVALID_NAME_LENGTH = "자동차 이름을 5글자 이내로 입력해주세요.";
private static final String NOT_NUMBER = "시도할 횟수를 숫자로 입력해주세요.";
Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

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

InputValidator 에서 거의 모든 예외를 전부 처리하고 있어요.

자동차 이름 길이와 같은 제약 사항은 도메인에서 관리해주는 것이 더 타당하다고 생각합니다!
입력 단계에서 관리해 줄 예외와 도메인 단에서 관리해 줄 예외를 구분해보시는 것을 추천드려요!

Copy link
Author

Choose a reason for hiding this comment

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

한준님 말씀에 동의합니다.

저도 모든 validation을 InputValidator에서 처리하는 구조가 BE 입장에서도 안 좋다고 생각하긴 했습니다:)
현준님 말씀처럼 비즈니스 로직 문제도 그렇고, view는 결국 FE 단인데 FE에서 넘어온 데이터에 대한 유효성 검증 책임이 BE에도 있다고 생각해서요ㅎㅎ 다만 그 Exception 처리를 controller에서 더 쉽게 하기 위해, 어쩔 수 없이 InputValidator에 모든 책임을 맡겼습니다.

현준님 코드 보면서 controller에서 도메인 객체를 생성하는 부분만 메소드화 시켜 exception 처리 메소드에 대입해서 사용하신걸 보고 적절한 처리 방법을 배웠습니다. 감사합니다!


private InputValidator() {

}

public static void notContainsComma(String input) {
if (input.length() > MAX_NAME_LENGTH && !input.contains(COMMA)) {
throw new IllegalArgumentException(NOT_IN_COMMA);
}
}

public static String[] moreThanLenFive(String input) {
String[] inputList = input.split(COMMA);
for (String str : inputList) {
if (str.trim().length() > MAX_NAME_LENGTH) {
throw new IllegalArgumentException(INVALID_NAME_LENGTH);
}
}
return inputList;
}

public static int notNumber(String input) {
try {
return Integer.parseInt(input);
} catch (NumberFormatException e) {
throw new IllegalArgumentException(NOT_NUMBER);
}
}
}
27 changes: 27 additions & 0 deletions src/main/java/racingcar/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package racingcar.view;

import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import racingcar.utils.InputValidator;
import camp.nextstep.edu.missionutils.Console;


public class InputView {

public List<String> readPlayerName() throws IllegalArgumentException {
System.out.println("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)");
String input = Console.readLine();
InputValidator.notContainsComma(input);
String[] inputList = InputValidator.moreThanLenFive(input);
Comment on lines +15 to +16
Copy link

Choose a reason for hiding this comment

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

메서드 네임에 Comma 와 같은 너무 구체적인 사항이 들어가는 것은 좋지 않다고 생각해요.
만약에 자동차 이름을 구분하는 단위가 , 에서 : 로 변했다면 관련된 메서드 이름까지 바꿔줘야 하는 번거로움이 있습니다.

moreThanLenFive 도 마찬가지로 자동차 이름을 6자까지 허용해준다면? 메서드 네이밍이 변해야겠죠.
이처럼 이름에 자료형이나 구체적인 값을 포함시키는 것은 유지보수에 매우 불리하게 작용합니다!

추가로 이름을 축약시키는 것도 지양해야한다고 알고있어요! 이름이 길어지더라도 Len 보다는 Length 를 그대로 사용하시는 것이 더 이해하기 쉽다고 생각합니다!

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다. 이건 부끄럽네요ㅎㅎ...
예전에 다른 분께도 메소드 네이밍에 대해 지적받은 부분인데 아직 고치지 못하고 있었습니다.
말씀하신 내용 다 동의하고, 지나치게 구체적인 메서드 네이밍이나 축약 표현 지향하도록 노력하겠습니다.
감사합니다!

return Arrays.stream(inputList)
.map(String::trim)
.collect(Collectors.toList());
}

public int readGameRound() throws IllegalArgumentException {
System.out.println("시도할 회수는 몇회인가요?");
String input = Console.readLine();
return InputValidator.notNumber(input);
}
}
40 changes: 40 additions & 0 deletions src/main/java/racingcar/view/OutputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package racingcar.view;

import java.util.List;
import java.util.Map;
import racingcar.domain.GameResult;

public class OutputView {

private static final String RESULT_OF_PLAY = "실행 결과";
private static final String FINAL_WINNER = "최종 우승자";
private static final String COLON = ":";
private static final String BLANK = " ";
private static final String MOVING = "-";

public void printResultMessage() {
System.out.println();
System.out.println(RESULT_OF_PLAY);
}

public void printRoundResult(GameResult gameResult) {
Map<String, Integer> playerPosition = gameResult.getPlayerPosition();
for (String name : playerPosition.keySet()) {
System.out.print(name + BLANK + COLON + BLANK);
for (int i = 0; i < playerPosition.get(name); i++) {
System.out.print(MOVING);
}
System.out.println();
}
System.out.println();
}

public void printFinalWinner(GameResult gameResult) {
List<String> finalWinner = gameResult.getFinalWinner();
System.out.println(FINAL_WINNER + BLANK + COLON + BLANK + String.join(", ", finalWinner));
}

public void printErrorMessage(String message) {
System.out.println("[ERROR]" + BLANK + message);
}
}
Loading