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

[1단계 - 자동차 경주] BE_필즈(조성우) 미션 제출합니다 #331

Merged
merged 46 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
217eb90
(feat): JUnit 연습
Feb 9, 2022
f8be399
(fix): 요구사항 3 코드 수정
Feb 9, 2022
ae87754
<docs> 기능구현 목록 작성
Feb 10, 2022
94698e6
test: 자동차 이름 유효성 검증
Feb 10, 2022
e557cbc
feat: 문자열 파싱 기능
Feb 10, 2022
7c5e0b5
feat: 자동차 위치 이동
Feb 10, 2022
97c1b8c
feat: 기능 임시 구현
Feb 10, 2022
a772221
feat: 게임 구동부
Feb 10, 2022
a84b13a
feat: 우승자 판정 및 출력
Feb 10, 2022
b628764
test: 우승자를 결정하는 테스트 코드
Feb 10, 2022
733265b
<docs>: 기능 목록 최신화
Feb 10, 2022
fd25aae
refactor: MVC 패턴 적용
Feb 10, 2022
0cddf9c
test: MVC 패턴에 부합하도록 테스트 구조 변경
Feb 10, 2022
803addf
refactor: 테스트 코드 정리
Feb 10, 2022
81d6511
refactor: 코드 정리
Feb 10, 2022
cee8a63
refactor: 테스트 코드 패키지 구조 변경
Feb 10, 2022
7103816
refactor: 패키지 구조 변경
Feb 10, 2022
3cdcae9
docs(readme): 요구사항 포맷 변경
progress0407 Feb 11, 2022
cd13c29
feat(StringCalculator): 기존에 작성한 계산기 수동 병합
progress0407 Feb 11, 2022
71e2ec0
test(StringCalculator): 하나의 메서드에 하나의 assertThat 이 오게끔 변경, 지역변수 final로 변경
progress0407 Feb 11, 2022
b2981b4
refactor: 클래스 이름 변경 (GameController -> CarController)
progress0407 Feb 11, 2022
1221a56
refactor: InputView, OutputView 를 유틸리티 클래스로 전환
progress0407 Feb 11, 2022
5c48862
refactor: CarController 가 너무 많은 책임을 하지 않게끔 MVC 구조 수정
progress0407 Feb 11, 2022
d275f38
refactor: 컨트롤러에서 출력하는 부분 제거, InputView, OutputView 에 대한 책임 재분담
progress0407 Feb 11, 2022
4df38f5
refactor: Car 에서 담당하던 차 상태 출력을 OutputView 로 옮김
progress0407 Feb 11, 2022
9eb188f
refactor: 랜덤한 움직임을 Strategy 로 구조 변경
progress0407 Feb 11, 2022
806b28e
refactor(Car): Cars 일급 컬렉션과 CarName 값 객체 추출, Validator 제거
progress0407 Feb 11, 2022
11cc2ba
test(CarTest): 중첩 구조로 변경 (Nested)
progress0407 Feb 11, 2022
e7e1b15
docs(question): 질문했었던 내용에 대한 마크다운 작성
progress0407 Feb 11, 2022
853d418
refactor(MovingStrategy): 자동차 움직임 생성 Car 정적(static) 생성 방식으로 변경
progress0407 Feb 13, 2022
21693a8
refactor(Cars): 우승자 선출 방식을 컨트롤러에서 Cars 에게 위임
progress0407 Feb 13, 2022
7d98cb3
docs(readme): 리뷰어님 요구사항 추가
progress0407 Feb 13, 2022
ba247d1
refactor(RandomMovingStrategy): Random에서 ThreadLocalRandom으로 변경 (쓰레드에…
progress0407 Feb 13, 2022
e962c14
refactor(OutputView): 우승자 출력시 이름순으로 출력
progress0407 Feb 13, 2022
83a5a83
feat(Cars): 자동차 중복된 이름 생성시 검증하는 로직 추가 및 테스트 작성 (리뷰어님 의견 반영!)
progress0407 Feb 13, 2022
fd36fb6
feat(TryRoundNumber): 시행횟수를 값 객체로 변환
progress0407 Feb 13, 2022
083c2ce
feat: 입력부에서 예외가 났을 경우, 해당 입력부부터 다시 받을 수 있도록 리펙터링. void to void 를 처리할 …
progress0407 Feb 13, 2022
2f00f06
test(Car, Cars): 예외 경우를 테스트할 때 어떤 예외 타입인지, 어떤 에러 메세지를 가지고 예외가 나오는지 기술
progress0407 Feb 13, 2022
92c79cd
refactor: Effectively final 한 모든 지역변수에 final 선언
progress0407 Feb 13, 2022
4e392ec
test: @DisplayName 에 경우에 대한 결과(검증해야할 내용)을 적음
progress0407 Feb 13, 2022
13d34d6
refactor(InputView): Scanner에서 쓰레드 세이프한 BufferedReader로 변경, inputView…
progress0407 Feb 14, 2022
6012ceb
refactor(CarDto): 차 상태표시 역할을 toString 메서드로 CarDto에게 위임
progress0407 Feb 14, 2022
d1a6375
refactor: 외부로 노출할 필요가 없는 메서드는 private 혹은 default로 변경, 메서드의 순서 조정
progress0407 Feb 15, 2022
55e3506
refactor(Car): OCP를 지킬수 있게 빌더패턴으로 전환후 정적 팩토리 메서드 제거
progress0407 Feb 15, 2022
d68246b
refactor: 테스트 코드를 CarTest 에서 CarNameTest 로 따로 분리
progress0407 Feb 15, 2022
3c367fd
refactor(CarDto): 해당 클래스 제거
progress0407 Feb 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 83 additions & 0 deletions QUESTION.MD
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@


# 첫 번째 Pull Request
Copy link

Choose a reason for hiding this comment

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

2. 기억하기 위한 마크다운 파일도 답변 드릴게요 😄
질문 기록을 위한 마크다운 파일 💯 아아주 훌륭합니다! 필즈가 기억하고 정리하는데 좋다면 무엇이 문제일까요!
최상위 문서로 만들어도 괜찮고, 정 그러면 최상위에 study 혹은 question같은 폴더를 만들고 그 안에 md 파일을 올려도 괜찮을 것 같아요 👍

Copy link
Author

Choose a reason for hiding this comment

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

아 !! ㅎㅎㅎ 감사합니다.. 칭찬 너무 잘해주시는거 같아요, 넘 기쁘네요 ㅎㅎㅎ


---

안녕하세요 ! 잘 부탁드립니다 ㅎㅎ 필즈(Philz)입니다 !

이렇게 공식적으로 코드리뷰를 받아보는 것은 처음인데.. 두근두근! 합니다 ㅎㅎ :)

리뷰를 받을때 궁금한 사항들이 생겨.. 몇가지 질문을 같이 드리고자 합니다 ㅠ
(너무 많으면 생략하셔도 됩니다 ㅠ ㅎㅎ..)

---

### 1. 앵귤러 커밋 컨벤션에서.. 스코프

---

![image](https://user-images.githubusercontent.com/66164361/153561334-1f6cc7ef-62b6-41f5-b5d2-5cc653eebbbe.png)


커밋 메시지를 적을 때 `feat(스코프)` 라고 적는데 이 스코프에는 어떤 내용이 들어가는지 확신이 서지 않습니다 . domain이란 개념으로 받아들여도 될까요?
> 예) feat(Item): 구조 수정

### 2-1.`패키지구조` domain 클래스 안에 패키지가 여러개 있어도 되나요?

---

예를들어 자동차의 움직임을 묘사하는 strategy 패턴 같은 곳을 정의 하는 패키지가 이곳에 오는 지 궁금합니다

### 2-2. `패키지 구조` domain 과 controller, service 등이 있을때 어떻게 나뉘는지 궁금합니다

---

> 아래에서 domain 은 station, line 입니다!

1.
![image](https://user-images.githubusercontent.com/66164361/153560471-29b9e4bb-44bf-467d-a313-a1bc50661077.png)

2.
![image](https://user-images.githubusercontent.com/66164361/153560504-aa3b36c7-e333-4401-a9cc-95a4ef8cc11d.png)

### 3. 실무에서 반드시 자바 빈 프로퍼티 규약을 준수 하나요??

---

제 코드에서 Car는 별도로 CarName 값 객체를 멤버로 두었습니다.
Car객체에서 이름을 반환하고자 할때는 `car.getName()` 을 사용하지만
CarName 에서까지 getter를 사용하면, **name.getName()** 이 되어서 의미상 중복(`name`)이 되기에
`name.get()` 으로 시그니쳐를 정하려고 하는데 실무에서는 어떤지 궁금합니다


### 4. 테스트 코드 작성시 `public` 으로 노출하고 싶지 않았던 메서드를 테스트하게 될 때

---

예전에 혼자서 테스트 코드를 작성할 때 한 번 써보았던 방법인데

외부로 최대한 노출 안시키고자 하는 타협점에서 (그렇다고 리플렉션까지는 가지 않기 위해..) 사용해보았던건데..

`CarTest extends Car` 로 상속받은 후에

`protected` 를 써서 테스트 코드를 한 번 작성해본 적이 있었습니다

이경우 단점으로는 **결국 메서드 시그니쳐가 변경**되지만

장점으로 **그래도 완전히 외부로 노출되지는 않음** 이 된다고 생각했었는데.. 실무에서는 주로 어떻게 접근하는지 궁금합니다



### 5. 실무에서도 값 객체를 많이 활용하나요??

---

넥스트 스텝, 우테코를 통해서 VO라는 개념이 엔티티와는 다르단 것을 알았습니다 !

Car에는 CarName이 있는 것 처럼 실무에서도 값 객체를 통한 검증로직 등을 내부로 감싸서 해결하는 등의 방식을 많이 사용하고 있는지 궁금합니다.. ㅎㅎ..


#### 이상입니다 ! 읽어주셔서 감사해요 ㅠ


69 changes: 65 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,68 @@
# java-racingcar
# 문자열 덧셈 계산기

자동차 경주 미션 저장소
---

## 우아한테크코스 코드리뷰
## 기능 구현 목록

- [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)
- 쉼표 `,` 혹은 콜론 `:` 을 구분자로 하는 각 숫자의 합 반환

> 예시

| 입력 | 출력 |
| ------- | ---- |
| "" | 0 |
| "1,2,3" | 6 |
| “1,2:3” | 6 |

- [x] 커스텀 구분자
- `//` 와 `\n` 사이의 문자가 사용자가 지정한 구분자가 된다

> 예시

| 입력 | 출력 |
| ------------ | ---- |
| `//;\n1;2;3` | 6 |

## 예외 처리 목록

- [x] `RuntimeException` 으로 처리할 것
- [x] 숫자이면서 음수
- [x] 숫자가 아닌 것
- 한글
- 영어
- 특수문자
- 공백(스페이스, 탭, \n, \r 등)

# 자동차 경주 미션

---

## 기능 구현 목록

- [x] 주어진 횟수 동안 n대의 자동차는 전진 혹은 멈출 수 있다
- 즉 하나의 자동차는 턴마다 멈추거나 전진한다
- [x] 자동차에는 이름이 있다
- 자동차 출력시에 이름이 표시된다
- [x] 자동차의 이름은 쉼표를 기준으로 구분한다
- 이름은 5자만 가능
- [x] 사용자는 완주 이동 횟수를 입력한다
- [x] 0 ~ 9 의 random 값을 구한 후
- 0 ~ 3 멈춤
- 4 ~ 9 전진
- [x] 우승자를 판별한다
- 우승자는 2명이상 일 수 있다

## 예외 처리 목록

- [x] 자동차의 이름에는 한글, 영어, 숫자, 언더바만 올 수 있다
- 특수문자나 `null`은 허용하지 않는다
- [ ] 완주 이동횟수는 숫자만 입력 가능하다
- 음수를 입력할 수 없다
- 0도 허용하지 않는다
- [ ] 현재 식별자는 이름밖에 없으므로 자동차 선수들이 여러명일 때 중복된 이름을 허용하지 않는다
- > 리뷰어 `heebong`님의 예외사항 아이디어 !
hint : `Set`

## 기술 관련 사항 목록
- [ ] Random과 ThreadLocalRandom 차이를 익히고 적재적소 적용
- > `heebong`님 아이디어!
31 changes: 31 additions & 0 deletions src/main/java/racingcar/RacingCarApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package racingcar;

import racingcar.controller.CarController;
import racingcar.repository.CarRepository;
import racingcar.view.InputView;

public class RacingCarApplication {

Copy link

Choose a reason for hiding this comment

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

빈 줄 발견! 지워주면 좋겠네요 😄
main 함수의 맨 마지막, 빈 줄인 20line도 제거하면 좋겠습니다.

Copy link
Author

Choose a reason for hiding this comment

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

체크 완료 !

private static CarController carController;
private static CarRepository carRepository;
private static InputView inputView;

public static void main(String[] args) {

injectDependency();
playGame();
}

private static void playGame() {
carController.initGame();
carController.playGame();
carController.showWinners();
carController.end();
}

private static void injectDependency() {
inputView = new InputView();
carRepository = new CarRepository();
carController = new CarController(carRepository, inputView);
}
}
98 changes: 98 additions & 0 deletions src/main/java/racingcar/controller/CarController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package racingcar.controller;

import java.util.List;

import racingcar.controller.dto.CarDto;
import racingcar.domain.Car;
import racingcar.domain.Cars;
import racingcar.domain.vo.TryRoundNumber;
import racingcar.repository.CarRepository;
import racingcar.util.Voider;
import racingcar.view.InputView;
import racingcar.view.OutputView;

public class CarController {

private final CarRepository carRepository;
private final InputView inputView;

private int roundNumber;

public CarController(CarRepository carRepository, InputView inputView) {
this.carRepository = carRepository;
this.inputView = inputView;
}

public void initGame() {
initCars();
initRoundNumbers();
}

private void initCars() {
Voider voider = () -> {
String carNames = inputView.inputCarNames();
Cars cars = new Cars(carNames);
List<Car> carList = cars.getCars();
carRepository.addCars(carList);
};
commonInputProcess(voider);
}

private void initRoundNumbers() {
Voider voider = () -> {
String input = inputView.inputRoundNumber();
TryRoundNumber tryRoundNumber = new TryRoundNumber(input);
roundNumber = tryRoundNumber.get();
};
commonInputProcess(voider);
}

private void commonInputProcess(final Voider inputFunction) {
while (true) {
try {
inputFunction.execute();
return;
} catch (Exception exception) {
inputView.printErrorMessage(exception);
}
}
}

public void playGame() {
OutputView.printResultMessage();
for (int i = 0; i < roundNumber; i++) {
playRound();
}
}

public void playRound() {
Copy link

Choose a reason for hiding this comment

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

playGame()에서만 사용돼서 private해도 될 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

체크 완료 감사합니다 !

List<Car> cars = carRepository.findAll();
moveCars(cars);
List<CarDto> carDtos = CarDto.from(cars);
OutputView.showCurrentStatus(carDtos);
}

public List<Car> getWinners() {
Copy link

Choose a reason for hiding this comment

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

getWinners()showWinners()에서만 사용되네요. private으로 바꾸면 좋을 것 같아요. 물론 그럼 테스트코드를 못짤거에요. 다시 생각해봅시다. 이 테스트가 정말 필요한 테스트인가? 그렇다면 다른 객체로 위임해야하는가?

Copy link
Author

Choose a reason for hiding this comment

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

맞아요 테스트 코드 떄문에 default로 해 놓았었던건데... 흠 이부분은 한번 고민해봐야겠네요 advance 하군요 !!

List<Car> findCars = carRepository.findAll();

Copy link

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.

image
이렇게 접혀있는 부분을 누르면 코멘트가 더 나오는데, 혹시 접혀있는 부분을 눌렀는지 질문드려요. 👀 (공교롭게도 저 부분만 반응을 안해주셔서..!)

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다 !! 저게 conversations == 소중이 리뷰들 이란 것을 인지하지 못하고 있었네요 ㅠㅠ 지금 다시 확인중이에요 ㅎ

Cars cars = new Cars(findCars);

return cars.getWinners();
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.

아 ㅎㅎ 일급컬렉션에게 위임한건데 알아봐 주셨군요 ㅎ 이런이런..훗

Copy link
Author

Choose a reason for hiding this comment

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

앗.. 제 말투가 읍읍.. 여튼 땡큐해요 ! ㅎㅎ;

}

public void moveCars(List<Car> cars) {
Copy link

Choose a reason for hiding this comment

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

moveCars()playRound()에서만 사용돼서 private으로 바꾸면 좋을 것 같습니다. 😄
그리고 함수의 구현 순서도 사용되는 순으로 맞추면 좋겠어요.

Copy link
Author

Choose a reason for hiding this comment

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

ㅎㅎ 여기 conversation 을 놓쳤었네요 ㅠㅠ 후에 다시 언급해주셔서 지금은 마침 했어요 ㅎ

for (Car car : cars) {
car.move();
}
}

public void end() {
inputView.terminateScanner();
}

public void showWinners() {
List<Car> winners = getWinners();
List<CarDto> carDtos = CarDto.from(winners);
OutputView.showGameResult(carDtos);
}
}
48 changes: 48 additions & 0 deletions src/main/java/racingcar/controller/dto/CarDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package racingcar.controller.dto;

import java.util.ArrayList;
import java.util.List;
import java.util.function.Function;

import racingcar.domain.Car;
import racingcar.domain.vo.CarName;

public class CarDto {

private final CarName name;
private final int position;

public CarDto(CarName name, int position) {
this.name = name;
this.position = position;
}

public String getName() {
return name.get();
}

public int comparePositionTo(final CarDto other) {
return compareTo((CarDto otherCar) -> this.position - otherCar.position, other);
}

public int compareNameTo(final CarDto other) {
return compareTo((CarDto otherCar) -> this.name.get().compareTo(other.getName()), other);
}

public int compareTo(final Function<CarDto, Integer> function, final CarDto other) {
return function.apply(other);
}

public static List<CarDto> from(List<Car> cars) {
Copy link

Choose a reason for hiding this comment

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

public static CarDto from(Car car) { ... } 구현 코드가 있으면 List<CarDto> from(List<Car> cars)함수에서 사용할 수 있을 것 같아요 😄

Copy link
Author

Choose a reason for hiding this comment

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

아.. ㅎㅎ DTO 클래스 제거 전의 conversation 이었네요 ㅎ.. 근데 막상 해당 버전 체크아웃해서 해보니까.. 조금 애매한 거 같아요 이부분은 ㅠ (메서드로 분할시 코드가 감소 하지 않음 등..)

	public static CarDto from(Car car) {
		return car.toDto();
	}

	public static List<CarDto> from(List<Car> cars) {
		List<CarDto> carDtos = new ArrayList<>();
		for (Car car : cars) {
			carDtos.add(CarDto.from(car));
		}
		return carDtos;
	}

음 생각해보니 car의 생성을 CarDto 클래스에 위임하는 방법도 있긴한데.. 닭이 먼저냐, 달걀이 먼저냐 하는 느낌이 있어서 ㅎㅎ..

List<CarDto> carDtos = new ArrayList<>();
for (Car car : cars) {
carDtos.add(car.toDto());
}
return carDtos;
}

@Override
public String toString() {
return name + " : " + "-".repeat(position);
}
}