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

자동차경주/제훈/step1 #4

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
36 changes: 31 additions & 5 deletions src/main/java/study/racingcar/controller/CarController.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,38 @@
package study.racingcar.controller;

import static study.racingcar.model.Car.raceStart;
import static study.racingcar.view.InputView.getAttemptNumber;
import static study.racingcar.view.InputView.getCarNumber;
import study.racingcar.view.InputView;
import study.racingcar.view.ResultView;

public class CarController {
import java.util.ArrayList;
import java.util.List;

import static study.racingcar.model.Car.requestCarsToMove;

public class CarController {
Copy link
Collaborator

Choose a reason for hiding this comment

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

'CarController' 에서 로직을 직접 구현해서 사용하고 있네요!
현재의 구조에서는 Car객체와 CarController가 서로 바뀐듯처럼 보이네요 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

CarController 라는 네이밍은 왜 나온것일까요?
게임기 컨트롤러 처럼 도메인 객체에게 어떠한 일을 하도록 시키는 녀석이 아닐까요?
현재의 구조에서는 Car 객체가 CarController에게 자동차 경주를 하도록 시키는 것으로 보이네요~

Copy link
Collaborator Author

@JoJeHuni JoJeHuni Jul 19, 2023

Choose a reason for hiding this comment

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

MVC 패턴이 컨트롤러 로직 실행 -> 비즈니스 로직 실행 후 결과를 받고 -> 결과 데이터를 모델에 저장 -> 컨트롤러에서 뷰 로직으로 권한을 넘겨준 뒤 뷰는 모델에서 데이터를 참조해 응답하는 것이라는걸 정리해놓고 활용할 때는 반대로 이행하고 있었네요.. 감사합니다!

public CarController() {
raceStart(getCarNumber(), getAttemptNumber());
raceStart(InputView.getCarNumber(), InputView.getAttemptNumber());
}

public static List<Integer> raceStart(int carNum, int attemptNum) {
List<Integer> startLine = addCar(carNum);
Copy link
Collaborator

Choose a reason for hiding this comment

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

분명히 Car객체는 존재하는데도 불구하고
출발했을때의 위치, 경주가 끝났을때의 위치를 각각 만들어서 구현하신 이유가 무엇일까요? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

테스트 코드에서 raceStart 메서드의 반환값으로 assertEquals 를 사용하려고 만들었던 것 같습니다. 다시 생각해보겠습니다!

List<Integer> finalPositions = new ArrayList<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

레이스가 시작하기전에 경주에 참여할 자동차 객체들을 생성하고
반복문을 통해서 자동차에게 앞으로 전진할지 말지를 요청해보는 식으로 만들어보면 어떨까요~?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

raceStart 에서 addCar 메서드를 불러오는 것이 아닌 그 전에 미리 객체들을 생성하고 요청해보자는 조언 감사합니다! 수정하겠습니다!


for (int i = 0; i < attemptNum; i++) {
List<Integer> racingCar = requestCarsToMove(carNum, startLine);
ResultView.printRaceResult(racingCar);
System.out.println("");
Copy link
Collaborator

Choose a reason for hiding this comment

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

출력은 ResultView에게 위임해보아요~

finalPositions = new ArrayList<>(racingCar);
}
return finalPositions;
}

private static List<Integer> addCar(int carNum) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

addCar() 메서드 명만을 봤을때 Car 객체들을 생성해줄 것같은데 Integer들을 생성해주고 있네요 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Car 객체를 생성해준다는 것을 놓치고 있었네요 바로 수정하겠습니다. 감사합니다!!

List<Integer> positions = new ArrayList<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

position을 자동차 객체의 맴버변수로 바꿔서 로직을 만들어보면 어떨까요~?
자동차의 위치는 존재하는데 자동차는 존재하지 않는 이상한 상황이 벌어지고 있네요 😨

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Integer로 받는 것이 아닌 Car 객체의 ArrayList 를 만들어보겠습니다!


for (int i = 0; i < carNum; i++) {
positions.add(i,0);
}

return positions;
}
}
32 changes: 4 additions & 28 deletions src/main/java/study/racingcar/model/Car.java
Original file line number Diff line number Diff line change
@@ -1,48 +1,24 @@
package study.racingcar.model;

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

import static study.racingcar.view.ResultView.printRaceResult;

public class Car {

private static final int MAX_BOUNDARY_OF_RANDOM_NUMBER= 10;
public Car(int carNum, List<Integer> positions) {
addCar(carNum, positions);
}

public static void raceStart(int carNum, int attemptNum) {
List<Integer> positions = new ArrayList<>();

Car car = new Car(carNum, positions);

car.addCar(carNum, positions);

for (int i = 0; i < attemptNum; i++) {
car.requestCarsToMove(carNum, positions);
printRaceResult(positions);
}
}

private static void addCar(int carNum, List<Integer> positions) {
for (int i = 0; i < carNum; i++) {
positions.add(i,0);
}
}
public Car() { }

public static void requestCarsToMove(int carNum, List<Integer> positions) {
public static List<Integer> requestCarsToMove(int carNum, List<Integer> positions) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

열심히 만든 Car 객체를 자동차 경주에 참가시켜봅시다 💪

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

한 차례 더 수정을 거치는 중이라 곧 커밋하겠습니다 조언 항상 감사합니다!

for (int i = 0; i < carNum; i++) {
forwardNumber(i, positions);
}
return positions;
}

private static void forwardNumber(int current, List<Integer> positions) {
Random random = new Random();
int forward = random.nextInt(MAX_BOUNDARY_OF_RANDOM_NUMBER); // 0부터 9

if (forward >= 4) { // 전진하는 조건
if (forward >= 4) {
int currentValue = positions.get(current);
int newValue = currentValue + 1;

Expand Down
2 changes: 0 additions & 2 deletions src/main/java/study/racingcar/view/ResultView.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package study.racingcar.view;

import java.util.Collections;
import java.util.List;
import java.util.Random;

public class ResultView {
public static void printRaceResult(List<Integer> positions) {
Expand Down
46 changes: 24 additions & 22 deletions src/test/java/study/racingcar/CarTest.java
Original file line number Diff line number Diff line change
@@ -1,44 +1,46 @@
package study.racingcar;

import org.junit.jupiter.api.Test;
import study.racingcar.model.Car;

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

import static org.junit.jupiter.api.Assertions.assertEquals;
import static study.racingcar.controller.CarController.raceStart;
import static study.racingcar.model.Car.requestCarsToMove;

public class CarTest {

private static final int POSITIONS_INDEX = 0;
private static final int MAX_BOUNDARY_OF_RANDOM_CAR_NUMBER = 5;
private static final int MAX_BOUNDARY_OF_RANDOM_ATTEMPT_NUMBER = 3;
@Test
public void 자동차_대수와_시도_횟수를_1로_정해준_뒤_이동_로직을_확인한다() {
int carNum = 1;
int attemptNum = 1;
List<Integer> positions = new ArrayList<>();

Car car = new Car(carNum, positions);
car.raceStart(carNum, attemptNum);

int carPosition = 0;
public void 자동차_이동_로직을_확인한다() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

테스트가 아직 제대로 작성되지 않았네요!
저번에 리뷰에서 말한것이지만 테스트를 작성했을때 어려웠다면
책임을 나누고 객체들 간의 관계를 정하는 작업을 더 해야한다는 신호가 아닐까요~?

조금씩 좋아지고 있습니다 👍

// given
Random random = new Random();
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서도 자동차 로직을 테스트 하는데 자동차는 없고 자동차의 위치가 둥둥 떠다니고 있네요 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

바로 수정해보겠습니다!

int carNum = random.nextInt(MAX_BOUNDARY_OF_RANDOM_CAR_NUMBER + 1); // 1 ~ 5
int attemptNum = random.nextInt(MAX_BOUNDARY_OF_RANDOM_ATTEMPT_NUMBER + 3); // 3 ~ 5

for (int i = 0; i < positions.size(); i++) {
carPosition = positions.get(i);
}
// when
List<Integer> finalPositions = raceStart(carNum, attemptNum);

assertEquals(carPosition >= 0 && carPosition <= 1, true);
// then
assertEquals(carNum, finalPositions.size());
}

@Test
public void 자동차_대수를_1대로_정해준_뒤_전진_로직을_확인한다() {
int carNum = 1;
public void 자동차_전진_로직을_확인한다() {
// given
Random random = new Random();
int carNum = random.nextInt(MAX_BOUNDARY_OF_RANDOM_CAR_NUMBER + 1); // 1 ~ 5
List<Integer> positions = new ArrayList<>();

Car car = new Car(carNum, positions);
car.requestCarsToMove(carNum, positions);

int carPosition = positions.get(POSITIONS_INDEX);
// when
List<Integer> checkForward = requestCarsToMove(carNum, positions);

assertEquals(carPosition == 0 || carPosition == 1, true);
// then
for (int i = 0 ; i < carNum; i++) {
assertEquals(checkForward.get(i) == 0 || checkForward.get(i) == 1, true);
}
}
}