-
Notifications
You must be signed in to change notification settings - Fork 1
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
자동차 경주 게임 PR #1
base: develop
Are you sure you want to change the base?
자동차 경주 게임 PR #1
Changes from all commits
97202cd
1da6266
c502a69
6b068d3
f21aefc
57472e2
6567a14
6533332
aaf7f35
9687f38
6221dff
866f1bb
30f7917
870ab11
b923bec
b9400bc
2f80191
83f4dca
139df34
3dcef78
626c531
588c5da
2ca4abb
bf114f9
a25c6a9
6455b38
8cb2640
38a82c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,38 @@ | ||
## [NEXTSTEP 플레이그라운드의 미션 진행 과정](https://github.com/next-step/nextstep-docs/blob/master/playground/README.md) | ||
# 자동차 경주 게임 | ||
|
||
--- | ||
## 학습 효과를 높이기 위해 추천하는 미션 진행 방법 | ||
## 규칙 | ||
|
||
--- | ||
1. 피드백 강의 전까지 미션 진행 | ||
> 피드백 강의 전까지 혼자 힘으로 미션 진행. 미션을 진행하면서 하나의 작업이 끝날 때 마다 add, commit | ||
> 예를 들어 다음 숫자 야구 게임의 경우 0, 1, 2단계까지 구현을 완료한 후 push | ||
- indent(인덴트, 들여쓰기) depth를 3이 넘지 않도록 구현한다. 2까지만 허용 | ||
- else 예약어를 쓰지 않는다 | ||
- 3항 연산자를 쓰지 않는다. | ||
- 함수(또는 메소드)가 한 가지 일만 하도록 최대한 작게 만들어라. | ||
- **모든 기능을 TDD로 구현해 단위 테스트가 존재해야 한다.** | ||
- **모든 원시 값과 문자열을 포장한다.** | ||
- **일급 컬렉션을 쓴다.** | ||
- **Getter를 쓰지 않는다.** | ||
|
||
![mission baseball](https://raw.githubusercontent.com/next-step/nextstep-docs/master/playground/images/mission_baseball.png) | ||
## 기능 요구사항 | ||
|
||
--- | ||
2. 피드백 앞 단계까지 미션 구현을 완료한 후 피드백 강의를 학습한다. | ||
- 각 자동차에 이름을 부여할 수 있다. 자동차 이름은 5자를 초과할 수 없다. | ||
- 전진하는 자동차를 출력할 때 자동차 이름을 같이 출력한다. | ||
- 자동차 이름은 쉼표(,)를 기준으로 구분한다. | ||
- 전진하는 조건은 0에서 9 사이에서 random 값을 구한 후 random 값이 4이상일 경우이다. | ||
- 자동차 경주 게임을 완료한 후 누가 우승했는지를 알려준다. 우승자는 한명 이상일 수 있다. | ||
|
||
--- | ||
3. Git 브랜치를 master 또는 main으로 변경한 후 피드백을 반영하기 위한 새로운 브랜치를 생성한 후 처음부터 다시 미션 구현을 도전한다. | ||
|
||
``` | ||
git branch -a // 모든 로컬 브랜치 확인 | ||
git checkout master // 기본 브랜치가 master인 경우 | ||
git checkout main // 기본 브랜치가 main인 경우 | ||
## 구현 사항 | ||
|
||
git checkout -b 브랜치이름 | ||
ex) git checkout -b apply-feedback | ||
``` | ||
--- | ||
- [ ] 자동차 이름들을 입력한다. | ||
- [x] "자동차 이름"은 5자를 초과 할 수 없다. | ||
- [ ] "시도 할 횟수"를 입력한다. | ||
- [x] 시도 횟수는 음수 일 수 없다. | ||
- [x] 경기를 시작한다. | ||
- [x] "시도 횟수" 만큼 자동차들을 움직인다. | ||
- [x] 모든 자동차를 움직인다. | ||
- [x] 자동차를 움직인다. | ||
- [x] 0 이상 9 이하의 랜덤 값을 받는다. | ||
- [x] 4 이상일 경우 한 칸 전진한다. | ||
- [ ] 우승자를 출력한다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
public abstract class AbstractRandomNumber { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AbstractRandomNumber보다 기본 자료형을 사용하는게 더 간단하지 않을까요? 🧐 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분을 클래스로 만든 이유는 RandomNumber의 종류가 더 확장될 수 있을 것 같아서 입니다. 이 부분은 좀 너무 많이 나간 확장일까요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AbstractRandomNumber와 AbstractRandomNumberFactory의 역할이 계속해서 헷갈리네요. |
||
private final int number; | ||
|
||
public AbstractRandomNumber(int number) { | ||
this.number = number; | ||
} | ||
|
||
public boolean isMoreThan(int comparisonNumber) { | ||
return comparisonNumber <= number; | ||
} | ||
|
||
public boolean isLessThan(int comparisonNumber) { | ||
return comparisonNumber >= number; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
public abstract class AbstractRandomNumberFactory { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. abstract class보단 interface가 더 어울릴것 같아요 😇 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 동의합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분의 경우 기능 명세만 되어있다보니 인터페이스가 어울리겠군여 ㅋㅋ |
||
abstract AbstractRandomNumber produce(); | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import java.util.Objects; | ||
|
||
public class Car { | ||
|
||
private final Name name; | ||
private Position position; | ||
private final MoveStrategy strategy; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Car 객체가 불변이기 때문에 Position 객체는 불변성을 확보하지 못한것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 말씀해주신 내용에 대해서 제가 이해한 내용을 말씀드려보자면 정리현재 코드 상황을 정리해 보자면 Car는 불변객체이고 Position은 불변성을 확보하지 못한 상태입니다. 여기서 저는 Position 값의 변경은 Position 본인이 책임져야 할 부분이라고 생각해서 하지만 재원님의 말씀을 듣고보니 즉 수정이 필요한 필드마다 불변성을 없애게 될 테니, 확장에 있어서 지속적인 불변성을 지키기가 어렵겠군요! 그래서 어차피 완전한 불변성을 지키기 어려운 경우라면 질문그렇다면 Car를 가변객체로 만들 경우 아래와 같이 Position의 인스턴스를 Car에서 만들어서 갈아끼우게 될텐데 public void moveToOneStep(){
this.position = new Position(1);
} 이때, Position의 내부구조가 Car에 노출되다보니 결국 결합도가 올라간다는 단점이 있을 것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move 메서드에서도 동시성 이슈가 발생할거라고 판단되는데 Car는 이미 가변 객체 일수도 있어요🤔 public void moveToOneStep() { There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
오! 이렇게 보니 딱 이해가 됩니다. 그냥 생성자를 가져다 쓰는 것만 생각했는데 public void moveToOneStep() {
this.position = position.moveOneStep();
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. private final Name name; 가변 객체라도 불변을 지향하는게 좋아보여요 😃 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Car를 가변객체로 바꿨다고 해도 |
||
public Car(String name, int position, MoveStrategy strategy) { | ||
this(name, strategy); | ||
this.position = new Position(position); | ||
} | ||
|
||
public Car(String name, MoveStrategy strategy) { | ||
validateMoveStrategy(strategy); | ||
this.name = new Name(name); | ||
this.position = new Position(); | ||
this.strategy = strategy; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메인 생성자를 하나로 두고, 나머지 생성자에서는 this 키워드로 전달하는게 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중복되는 코드가 있으니 생성자를 하나로 통일하라는 말씀으로 이해되는데
이 말이 잘 이해가 안됩니다 ㅜㅜ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 실제 생성자는 하나로 두고 this 키워드를 사용해서 생성자를 오버로딩 하면 좋을것 같아요 👍 |
||
private void validateMoveStrategy(MoveStrategy strategy) { | ||
if (Objects.isNull(strategy)) { | ||
throw new IllegalArgumentException("MoveStrategy must be not null"); | ||
} | ||
} | ||
|
||
public void move() { | ||
this.position = strategy.move(this.position); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
Car car = (Car) o; | ||
return Objects.equals(name, car.name) && Objects.equals(position, car.position); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(name, position); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Objects; | ||
|
||
public class Cars { | ||
|
||
private final List<Car> cars = new ArrayList<>(); | ||
|
||
public Cars(List<Car> cars) { | ||
validate(cars); | ||
this.cars.addAll(cars); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cars를 직접 대입하지 않고 요소들의 참조를 각각 추가 해주셨네요 이유가 있을까요? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분의 경우 원래 생성자의 인자로 피드백 감사합니다!! |
||
} | ||
|
||
private void validate(List<Car> cars) { | ||
if (Objects.isNull(cars)) { | ||
throw new IllegalArgumentException("cars는 Null일 수 없습니다."); | ||
} | ||
if (cars.isEmpty()) { | ||
throw new IllegalArgumentException("cars는 Enpty 일 수 없습니다."); | ||
Comment on lines
+15
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넘어오는 데이터에 대한 방어로직일까요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네! 맞습니다! |
||
} | ||
} | ||
|
||
public void move() { | ||
cars.forEach(Car::move); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
Cars cars1 = (Cars) o; | ||
return Objects.equals(cars, cars1.cars); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(cars); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
public class MoveByRandomNumberStrategy implements MoveStrategy{ | ||
|
||
public static final int MOVABLE_CAR_MIN_NUMBER = 4; | ||
private final AbstractRandomNumberFactory factory; | ||
|
||
public MoveByRandomNumberStrategy(AbstractRandomNumberFactory factory) { | ||
this.factory = factory; | ||
} | ||
|
||
@Override | ||
public Position move(Position position) { | ||
AbstractRandomNumber randomNumber = factory.produce(); | ||
if (randomNumber.isMoreThan(MOVABLE_CAR_MIN_NUMBER)) { | ||
return position.moveOneStep(); | ||
} | ||
return position; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
public interface MoveStrategy { | ||
Position move(Position position); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import java.util.Objects; | ||
|
||
public class Name { | ||
|
||
public static final int MAX_NAME_LENGTH = 5; | ||
private static final String NAME_LENGTH_OVER_MESSAGE = "자동차 이름은 5자를 초과할 수 없다."; | ||
|
||
private final String name; | ||
|
||
public Name(String name) { | ||
validate(name); | ||
this.name = name; | ||
} | ||
|
||
private void validate(String name) { | ||
if (Objects.isNull(name) || name.length() > MAX_NAME_LENGTH) { | ||
throw new IllegalArgumentException(NAME_LENGTH_OVER_MESSAGE); | ||
} | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
Name name1 = (Name) o; | ||
return Objects.equals(name, name1.name); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(name); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import java.util.Objects; | ||
|
||
public class Position { | ||
|
||
private static final int DEFAULT_STEP_COUNT = 0; | ||
|
||
private final int step; | ||
|
||
public Position() { | ||
this.step = DEFAULT_STEP_COUNT; | ||
} | ||
|
||
public Position(int step) { | ||
validate(step); | ||
this.step = step; | ||
} | ||
|
||
private void validate(int step) { | ||
if (step < DEFAULT_STEP_COUNT) { | ||
throw new IllegalArgumentException("step에 음수를 입력할 수 없습니다."); | ||
} | ||
} | ||
|
||
public Position moveOneStep() { | ||
return new Position(step + 1); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
Position position1 = (Position) o; | ||
return step == position1.step; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(DEFAULT_STEP_COUNT, step); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
public class Race { | ||
|
||
private TryCount tryCount; | ||
|
||
private final Cars cars; | ||
|
||
public Race(int tryCount, Cars cars) { | ||
this.tryCount = new TryCount(tryCount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tryCount를 받아서 Race가 생성되는데, 어디서 쓰이는 걸까요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 TryCount는 요구사항이 입력한 "시도 횟수" 만큼 자동차를 전진시켜야 하는데 하지만 이번 프로젝트에서는 getter를 사용하지 않다 보니 while (tryCount.isOverThan(currentTryCount)){
this.cars.move(factory);
currentTryCount++;
} |
||
this.cars = cars; | ||
} | ||
|
||
public void start() { | ||
while (!this.tryCount.isComplete()) { | ||
cars.move(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 부정보단 긍정이 더 잘 읽힌다고 합니다 😎 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아! 예전에 피드백 주셨던 내용이군요! |
||
this.tryCount = tryCount.getNextTryCount(); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
public class RangeableRandomNumber extends AbstractRandomNumber { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 테스트 코드에서만 사용하고 있는 클래스인데, 테스트 패키지로 옮겨보면 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사실 Input, Output을 안만든상태라 그렇습니다.. ㅎㅎ |
||
private static final int DEFAULT_MAX_RANDOM_NUMBER = RangeableRandomNumberFactory.DEFAULT_MAX_RANDOM_NUMBER; | ||
private static final int DEFAULT_MIN_RANDOM_NUMBER = RangeableRandomNumberFactory.DEFAULT_MIN_RANDOM_NUMBER; | ||
|
||
private final int max; | ||
|
||
private final int min; | ||
|
||
public RangeableRandomNumber(int number, int max, int min) { | ||
super(number); | ||
validateNumber(number); | ||
this.max = max; | ||
this.min = min; | ||
} | ||
|
||
public RangeableRandomNumber(int number) { | ||
super(number); | ||
this.max = DEFAULT_MAX_RANDOM_NUMBER; | ||
this.min = DEFAULT_MIN_RANDOM_NUMBER; | ||
validateNumber(number); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
😇 |
||
private void validateNumber(int number) { | ||
if (number < min || number > max) { | ||
throw new IllegalArgumentException("RangeableRandomNumber는 " + min + " 보다 크고 " + max + "보다 작아야 합니다."); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import java.util.Random; | ||
|
||
public class RangeableRandomNumberFactory extends AbstractRandomNumberFactory { | ||
|
||
public static final int DEFAULT_MAX_RANDOM_NUMBER = 9; | ||
public static final int DEFAULT_MIN_RANDOM_NUMBER = 0; | ||
|
||
private final Random random; | ||
private final int max; | ||
private final int min; | ||
|
||
public RangeableRandomNumberFactory(int max, int min) { | ||
random = new Random(); | ||
this.max = max; | ||
this.min = min; | ||
} | ||
|
||
public RangeableRandomNumberFactory() { | ||
random = new Random(); | ||
this.max = DEFAULT_MAX_RANDOM_NUMBER; | ||
this.min = DEFAULT_MIN_RANDOM_NUMBER; | ||
} | ||
|
||
@Override | ||
public AbstractRandomNumber produce() { | ||
return new RangeableRandomNumber(random.nextInt(max+min) + min, max, min); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인자로 들어가는 값들이 어떤 역할을 하는지 명확하게 명시하는게 좋을 것 같아요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. max, min 보다는 |
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 클래스를 만드신 이유는 RandomNumber를 생성하는 로직이 앞으로 더 추가될 수 있기 때문인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 그렇습니다!
자동차의 Move 기능을 전략패턴으로 사용한 만큼
그 전략이 조건이 되는 RandomNumber 또한 다양한 조건들로 확장이 가능하도록 설계해봤습니다.