Skip to content
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
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
# 자동차 경주 게임

## 기능 목록
* 게임 참가자의 이름을 입력받는다. 이름의 구분자는 쉼표(,)이고, 이름이 5자가 넘어가면 NameLengthExceededException을 발생시키고 이름을 다시 입력받도록 한다.
* 자동차별로 random값을 만들고, 그 숫자가 4 이상이면 전진한다.
* 전진칸수가 최대값인 자동차가 우승자이다. 공동우승이 가능하다.

## 진행 방법
* 자동차 경주 게임 요구사항을 파악한다.
* 요구사항에 대한 구현을 완료한 후 자신의 github 아이디에 해당하는 브랜치에 Pull Request(이하 PR)를 통해 코드 리뷰 요청을 한다.
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ repositories {
dependencies {
testImplementation 'org.assertj:assertj-core:3.22.0'
testImplementation 'org.junit.jupiter:junit-jupiter:5.8.2'
testImplementation 'org.mockito:mockito-core:4.8.1'
}

java {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/exception/NegativeNumberException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package exception;

public class NegativeNumberException extends RuntimeException {
public NegativeNumberException() { super("negative number is not allowed."); }
}
62 changes: 62 additions & 0 deletions src/main/java/racingCar/CarRacingGame.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package racingCar;

public class CarRacingGame {
private final InputView inputView;
private final ResultView resultView;
private final NumberVerifier numberVerifier;

public CarRacingGame(InputView inputView, ResultView resultView, NumberVerifier numberVerifier) {
this.inputView = inputView;
this.resultView = resultView;
this.numberVerifier = numberVerifier;
}

public static void main(String[] args) {
InputView inputView = new InputView();
ResultView resultView = new ResultView();
NumberVerifier numberVerifier = new NumberVerifier();
CarRacingGame game = new CarRacingGame(inputView, resultView, numberVerifier);
game.startGame();
}

private void startGame() {
RacingCar[] cars = getParticipant();

play(cars, inputView.requestPlayCount());
}

private RacingCar[] getParticipant() {
while (true) {

Choose a reason for hiding this comment

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

while문을 쓸 필요가 있는걸까요 ?

int participantCount = inputView.requestParticipant();
try {
validateNum(participantCount);
return createRacingCars(participantCount);
} catch (NumberFormatException e) {
System.out.println(e.getMessage());
throw e;
}
}
}

void validateNum(int num) {

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.

테스트코드를 위해 default 접근지정자를 사용했습니다.
private으로 두고 테스트하는 방법들을 찾아보았는데

  1. 리플렉션 사용하는 것은 캡슐화를 깨트리는 방식이라 제외했고
  2. 해당 private method를 사용하는 public method를 테스트하는 방식은 메소드별로 단위 테스트를 작성하고자 하는 방향성과 멀어지는 것 같았습니다.
    대안으로 default 생성자를 사용하고 테스트코드에서 사용하는 방식을 사용했는데요, 다른 대안이 있다면 말씀해주세요:)

Choose a reason for hiding this comment

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

테스트코드를 위해 접근지정자를 변경하는 등의 기존 코드에 영향을 주는건 바람직하지 못합니다!
그리고 기본적으로 테스트코드의 대상은 public이라 private 메서드를 직접 테스트하지는 않으셔도 돼요!

해당 private method를 사용하는 public method를 테스트하는 방식은 메소드별로 단위 테스트를 작성하고자 하는 방향성과 멀어지는 것 같았습니다.

이건 아마 .. 모든 로직에 단위 테스트를 구현한다 라는 요구사항 때문에 말씀하신 것 같은데, 여기서 말하는 모든 로직은 public 메서드를 뜻합니다! 기본적으로는 public 메서드를 통해 내부에서 사용되는 private 메서드가 같이 검증되어야 하고, 만약 이게 어렵거나 private 메서드를 직접 테스트해야 하는 상황이 있다면 그건 클래스 설계가 잘못되지 않았는지 의심해봐야 해요

numberVerifier.verify(num);
}

RacingCar[] createRacingCars(int count) {

Choose a reason for hiding this comment

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

이건 그냥 빈 배열을 만들어주는 것 같아요?

추가로, 특별한 이유가 없다면 배열보단 List 등의 Collection을 사용하는 걸 추천드려요

Copy link
Author

Choose a reason for hiding this comment

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

리스트는 배열보다 대략 3배의 메모리를 더 사용한다고 알고 있습니다!
요구사항으로 볼 때 한 번 입력받은 뒤로 참여자가 추가되는 케이스는 없는 것으로 보여 사이즈를 고정해도 문제가 없을 것 같아 배열을 사용했습니다~!

Choose a reason for hiding this comment

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

말씀드리기전에 먼저 .. 리스트가 3배의 메모리를 사용하는 근거가 혹시 뭘까요 ? ArrayList면 기본 배열 사이즈때문에 조금 쓸 수 있어도 사이즈 지정을 해주면 부가정보 조금을 제외하면 크게 차이 없을 것 같고 .. LinkedList면 노드 크기와 타입에 따라 가변적이라 계산이 나오기가 어려울 것 같은데.. 궁금하네요!

말씀주신대로 문제는 없고 배열도 배열의 장점이 있어 필요한 경우가 있는데요. 대부분의 경우 List가 가지는 조작의 편의성과 가독성 그리고 확장성 때문에 배열보다는 List를 사용하는게 효율적이라고 생각해서 배열을 지양하시는게 어떨까 하고 드린 의견이었구요!

이건 대답을 안주셨는데, 44번 라인 빈 배열만 만들고 속이 안채워져있는 것 같아요~ 지금 프로그램 실행하면 NPE가 나지 않나요 ?

return new RacingCar[count];
}

private void play(RacingCar[] cars, int count) {
resultView.printResultTitle();
for (int i=0 ; i<count ; i++) {
moveCar(cars);
resultView.printPlay(cars);
}
}

private void moveCar(RacingCar[] cars) {
for (RacingCar car : cars) {
car.progress();
}
}
}
16 changes: 16 additions & 0 deletions src/main/java/racingCar/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package racingCar;

import java.util.Scanner;

public class InputView {
private static Scanner scanner = new Scanner(System.in);
int requestParticipant() {

Choose a reason for hiding this comment

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

여기도 접근지정자가 없네요 ~

System.out.println("자동차 대수는 몇 대 인가요?");
return Integer.parseInt(scanner.nextLine());
}

int requestPlayCount() {
System.out.println("시도할 회수는 몇 회 인가요?");
return Integer.parseInt(scanner.nextLine());
}
}
11 changes: 11 additions & 0 deletions src/main/java/racingCar/NumberVerifier.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package racingCar;

import exception.NegativeNumberException;

public class NumberVerifier {
void verify(int num) {
if (num <= 0) {
throw new NegativeNumberException();
}
}
}
38 changes: 38 additions & 0 deletions src/main/java/racingCar/RacingCar.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package racingCar;

import java.util.Random;

public class RacingCar {
private int pos;

Choose a reason for hiding this comment

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

변수명에는 줄임말을 지양해주세요 ~


public RacingCar(int pos) {
this.pos = pos;
}

public int getPos() {
return pos;
}

public void setPos(int pos) {

Choose a reason for hiding this comment

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

getter/setter는 최대한 지양 해 주세요 ~ 특히 setter는 아예 안쓰시는 걸 추천드립니다!

this.pos = pos;
}

@Override
public String toString() {
return "RacingCar{" + "pos=" + pos + '}';
}

int makeRandomNum() {

Choose a reason for hiding this comment

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

이 역할을 RacingCar가 가지고 있어야 하는게 맞을까요 ?

Copy link
Author

Choose a reason for hiding this comment

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

별도 클래스로 역할 구분했습니다~

return new Random().nextInt(10);

Choose a reason for hiding this comment

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

상수로 빼주시면 좋을 것 같아요 ~

}

void progress() {
if (makeRandomNum() >= 4){
incPos();
}
}

protected void incPos() {

Choose a reason for hiding this comment

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

요게 있는데 setter는 왜 있는걸까요 ?

그리고 이건 왜 protected인가요 ?

++this.pos;
}
}
14 changes: 14 additions & 0 deletions src/main/java/racingCar/ResultView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package racingCar;

public class ResultView {
void printResultTitle() {
System.out.println("실행 결과");
}

void printPlay(RacingCar[] cars) {
for (RacingCar car : cars) {
System.out.println("-".repeat(car.getPos()));
}
System.out.println();
}
}
40 changes: 40 additions & 0 deletions src/test/java/racingCar/CarRacingGameTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package racingCar;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;

class CarRacingGameTest {
private CarRacingGame carRacingGame;
private RacingCar[] cars;

@BeforeEach
void setUp() {
InputView mockInputView = mock(InputView.class);

Choose a reason for hiding this comment

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

mocking 한 객체를 아래에서 사용하지 않는것으로 보이는데요, mocking 한 이유가 있을까요 ?

Copy link
Author

Choose a reason for hiding this comment

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

별다른 이유는 없어서 실제 객체를 사용하도록 변경했습니다~

ResultView mockResultView = mock(ResultView.class);
NumberVerifier numberVerifier = mock(NumberVerifier.class);
carRacingGame = new CarRacingGame(mockInputView, mockResultView, numberVerifier);
cars = new RacingCar[3];
}

@Test
void testValidateNames_validNames() {
int TEST_COUNT = 3;
assertDoesNotThrow(() -> carRacingGame.validateNum(TEST_COUNT));

Choose a reason for hiding this comment

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

테스트는 기본적으로 public 메서드에 대해서만 진행하고, 테스트를 위해 접근지정자 레벨을 조정하는 건 지양해야 합니다~

Copy link
Author

Choose a reason for hiding this comment

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

위에서 말씀드렸듯이.. 접근 지정자 조정보다 나은 해결책을 잘 모르겠습니다..

}

@Test
void testValidateNames_invalidNames() {
int TEST_COUNT = 3;
assertThrows(NumberFormatException.class, () -> carRacingGame.validateNum(TEST_COUNT));
}

@Test
void testCreateRacingCars() {
int TEST_COUNT = 3;
RacingCar[] createdCars = carRacingGame.createRacingCars(TEST_COUNT);
assertEquals(3, createdCars.length);
}
}
37 changes: 37 additions & 0 deletions src/test/java/racingCar/RacingCarTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package racingCar;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.*;

class RacingCarTest {

private RacingCar car;

@BeforeEach
void setUp() {
car = spy(new RacingCar(0));
}

@Test
void makeRandomNum() {
int random = car.makeRandomNum();
assertTrue(random >= 0 && random <= 9);

Choose a reason for hiding this comment

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

이건 현재는 무조건 통과하는 테스트이고, 혹시 범위가 0~20 정도로 바뀌게 되면 이 테스트는 성공하기도, 실패하기도 하는 테스트라 검증을 못하고 넘어갈 수도 있어서 적절치 못한 테스트에요

랜덤한 값을 생성하고 그 값이 4 이상인 경우 true 를 리턴해주는걸 하나의 전략으로 정의하고 추상화해서 RacingCar가 인터페이스를 통해 주입받을 수 있게 하면 mocking을 통해 해당 인터페이스의 메서드가 불린지만 확인하는걸로 테스트를 만들어 볼 수 있을 것 같아요!

Choose a reason for hiding this comment

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

이건 현재는 무조건 통과하는 테스트이고, 혹시 범위가 0~20 정도로 바뀌게 되면 이 테스트는 성공하기도, 실패하기도 하는 테스트라 검증을 못하고 넘어갈 수도 있어서 적절치 못한 테스트에요

랜덤한 값을 생성하고 그 값이 4 이상인 경우 true 를 리턴해주는걸 하나의 전략으로 정의하고 추상화해서 RacingCar가 인터페이스를 통해 주입받을 수 있게 하면 mocking을 통해 해당 인터페이스의 메서드가 불린지만 확인하는걸로 테스트를 만들어 볼 수 있을 것 같아요!

}

@Test
void progress() {
// given
doReturn(4).when(car).makeRandomNum();
int expected = car.getPos()+1;

// when
car.progress();

// then
assertEquals(expected, car.getPos());
}
}