-
Notifications
You must be signed in to change notification settings - Fork 5
[미션1] 구현 완료했습니다. 리뷰 요청드립니다. #7
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
base: kikat
Are you sure you want to change the base?
Changes from all commits
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 +1,44 @@ | ||
| ECONOVATION HEROES 2기 2기 Java Racing Game Practice | ||
|
|
||
| #####step1 | ||
| - 접근제어자 접근 범위를 생각하고 사용하기 | ||
| - CamelCase 따르기 | ||
| - 스터디 규칙 : 모든 변수명은 줄여쓰지 않는다 | ||
| - 불필요한 공백 포함하지 않기 | ||
| - Java 는 메모리를 직접 접근하여 해제하지 않고 Garbage Collector 를 사용하여 | ||
| 메모리 해제함. | ||
|
|
||
| #####step2 | ||
| - primitive 타입과 wrapper 타입의 차이점 | ||
| - 게임의 룰은 어떤 클래스에 적용할 것인가? | ||
| - code formatting 습관 들이기 | ||
| - magic number 상수로 추출(상수는 네이밍은 대문자로 하는 것이 룰) | ||
| - 의존성 측면 고려하고 코드짜기 | ||
|
|
||
| ######refactoring: | ||
| - InputView 클래스 상수 타입 변환 | ||
| - Race 클래스 상수 네이밍 대문자 처리 | ||
| - CamelCase에 따라 네이밍 고치기 | ||
| - 게임의 룰을 관리할 Rule 클래스 생성 | ||
| - setter 사용 지양 | ||
| - 불필요한 코드 주석 및 공백 제거 | ||
| - stream api 적용 | ||
| - 게임 시작을 위한 input 과 관련된 print 는 inputview로 옮기기 | ||
| - View 에 대한 책임을 Main(RacingGame) 으로 위임 | ||
| - InputView static 메소드로 구성하여 객체 생성 비용을 절감하기 | ||
| (추가) | ||
| - 메서드 체이닝을 적용해서 Arrays.stream(trimmedInputCarName).forEach(~~) | ||
| 로 이어 붙이기 | ||
| - util 목적으로 사용하실 클래스 객체 생성 비용 줄이기 | ||
| - 의미없는 메모리 낭비 없애기 | ||
| - 가독성에 유리하게 선언과 할당 같은 곳에서 해주기 | ||
| - 테스트 코드 | ||
| ###### 해결할 문제 | ||
|
|
||
| 테스트 코드를 작성하다보니 public으로만 테스트가 안된다는 점에서 코드가 잘 짜여지지 않다고 느꼈다. | ||
| 테스트 코드를 먼저 작성하고 그에 맞추어 코드를 작성해야할까? | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| public class Car { | ||
| private String name; | ||
| private int position; | ||
|
|
||
| Car(String name) { | ||
| this.name = name; | ||
| this.position = 0; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| public int getPosition() { | ||
| return position; | ||
| } | ||
|
|
||
| public void goForward() { | ||
| this.position++; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.Scanner; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public class InputView { | ||
| private static final int MAX_LENGTH = 6; | ||
| private static final Scanner scanner = new Scanner(System.in); | ||
|
|
||
| public static List<Car> inputCarName() { | ||
| String name = scanner.nextLine(); | ||
| String[] splittedInputCarName = splitInputCarName(name); | ||
| String[] trimmedInputCarName = trimInputCarName(splittedInputCarName); | ||
|
|
||
| try { | ||
| processStreamAboutCheckName(trimmedInputCarName); | ||
| } catch(IllegalArgumentException e) { | ||
| System.out.println(e.getMessage()); | ||
| inputCarName(); | ||
| } | ||
|
|
||
| List<Car> cars = new ArrayList<>(); | ||
| for(int i = 0; i< splittedInputCarName.length; i++) { | ||
| cars.add(new Car(trimmedInputCarName[i])); | ||
| } | ||
| return cars; | ||
|
hyukjin-lee marked this conversation as resolved.
|
||
| } | ||
|
|
||
| private static String[] splitInputCarName(String name) { | ||
| return name.split(","); | ||
| } | ||
|
|
||
| private static String[] trimInputCarName(String[] name) { | ||
| for(int i = 0; i<name.length; i++) { | ||
| name[i] = name[i].trim(); | ||
| } | ||
| return name; | ||
| } | ||
|
|
||
| private static void checkNameLength(String name) { | ||
| if(name.length() >= MAX_LENGTH) { | ||
| throw new IllegalArgumentException("이름은 5자 이하만 가능합니다."); | ||
| } | ||
| } | ||
|
|
||
| private static void processStreamAboutCheckName(String[] trimmedInputCarName) { | ||
| Stream<String> stream = Arrays.stream(trimmedInputCarName); | ||
| stream.forEach(e -> checkNameLength(e)); | ||
|
Contributor
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. 메서드 체이닝을 적용해서 Arrays.stream(trimmedInputCarName).forEach(~~) |
||
| } | ||
|
|
||
| public static int inputTrial() { | ||
| return scanner.nextInt(); | ||
| } | ||
|
|
||
| public static void inputCarNamesMessage() { | ||
| System.out.println("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"); | ||
| } | ||
|
|
||
| public static void inputCountMessage() { | ||
| System.out.println("시도할 횟수는 몇회인가요?"); | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import java.util.List; | ||
|
|
||
| public class OutputView { | ||
| private static final String DRIVE = "-"; | ||
|
|
||
| public void resultMessage() { | ||
| System.out.println("\n실행 결과"); | ||
| } | ||
|
Contributor
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 과 관련된 print 는 inputview 에 있는게 어떨까요? |
||
|
|
||
| public void oneTrialMessage(List<Car> cars) { | ||
| for(Car car: cars) { | ||
| System.out.println(car.getName() + ": " + raceOneTrial(car)); | ||
| } | ||
| } | ||
|
|
||
| private StringBuilder raceOneTrial(Car car) { | ||
| StringBuilder goSignal = new StringBuilder(); | ||
| for(int i = 0; i<car.getPosition(); i++) { | ||
| goSignal.append(DRIVE); | ||
| } | ||
| return goSignal; | ||
| } | ||
|
|
||
| public void getWinnerMessage(List<String> winnernames) { | ||
| System.out.println(String.join(",", winnernames + "가 최종 우승했습니다.")); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class RacingGame { | ||
|
Contributor
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 List<String> winnernames = new ArrayList<>(); | ||
|
|
||
| public void race(List<Car> cars) { | ||
| for(Car car: cars) { | ||
| goOrStay(car); | ||
| } | ||
| } | ||
|
|
||
| private void goOrStay(Car car) { | ||
| if(Rule.isGoForward()) { | ||
| car.goForward(); | ||
| } | ||
| } | ||
|
|
||
| public List<String> getWinner(List<Car> cars) { | ||
| int positionOfWinner = findPositionOfWinner(cars); | ||
| cars = cars.stream().filter(car -> (car.getPosition() == positionOfWinner)).collect(Collectors.toList()); | ||
| for (Car car : cars) { | ||
| this.winnernames.add(car.getName()); | ||
| } | ||
| return winnernames; | ||
| } | ||
|
|
||
| private int findPositionOfWinner(List<Car> cars) { | ||
| List<Integer> position = new ArrayList<>(); | ||
| int positionOfWinner; | ||
| for(Car car: cars) { | ||
| position.add(car.getPosition()); | ||
| } | ||
| positionOfWinner = Collections.max(position); | ||
| return positionOfWinner; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class RacingMain { | ||
| public static void main(String[] args) { | ||
| OutputView outputView = new OutputView(); | ||
| RacingGame racingGame = new RacingGame(); | ||
| List<Car> cars = new ArrayList<>(); | ||
| int trial; | ||
|
|
||
| InputView.inputCarNamesMessage(); | ||
| cars = InputView.inputCarName(); | ||
| InputView.inputCountMessage(); | ||
| trial = InputView.inputTrial(); | ||
|
|
||
| for(int i = 0; i < trial; i++) { | ||
| outputView.resultMessage(); | ||
| racingGame.race(cars); | ||
| outputView.oneTrialMessage(cars); | ||
| } | ||
|
|
||
| outputView.getWinnerMessage(racingGame.getWinner(cars)); | ||
| } | ||
| } | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import java.util.Random; | ||
|
|
||
| public class Rule { | ||
| private static final int BOUNDARYNUMBER = 10; | ||
| private static final int GOFORWARDCONDITION = 4; | ||
|
|
||
| public static boolean isGoForward() { | ||
| Random random = new Random(); | ||
| if(random.nextInt(BOUNDARYNUMBER) >= GOFORWARDCONDITION) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import org.junit.After; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| import java.lang.reflect.InvocationTargetException; | ||
| import java.lang.reflect.Method; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import static org.hamcrest.CoreMatchers.*; | ||
| import static org.junit.Assert.assertThat; | ||
|
|
||
| public class InputViewTest { | ||
| List<Car> cars; | ||
| @Before | ||
| public void setUp() throws Exception { | ||
| cars = new ArrayList<>(); | ||
| cars.add(new Car("애플")); | ||
| cars.add(new Car("삼성")); | ||
| cars.add(new Car("전남대")); | ||
| } | ||
|
|
||
| @Test | ||
| public void inputCarName() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { | ||
| String carName = "애플, 삼성, 전남대"; | ||
| InputView inputView = new InputView(); | ||
| try { | ||
| Method methodsplit = inputView.getClass().getDeclaredMethod("splitInputCarName", String.class); | ||
| methodsplit.setAccessible(true); | ||
| Method methodtrim = inputView.getClass().getDeclaredMethod("trimInputCarName", String[].class); | ||
| methodtrim.setAccessible(true); | ||
| String[] name = (String[]) methodsplit.invoke(inputView, carName); | ||
| String[] name1 = (String[]) methodtrim.invoke(inputView, (Object) name); | ||
| assertThat("애플", is(name1[0])); | ||
| assertThat("삼성", is(name1[1])); | ||
| assertThat("전남대", is(name1[2])); | ||
| } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void inputTrial() { | ||
|
|
||
| } | ||
|
|
||
| @After | ||
| public void tearDown() throws Exception { | ||
| cars = null; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import org.junit.Test; | ||
|
|
||
| import static org.junit.Assert.*; | ||
|
|
||
| public class RaceTest { | ||
| @Test | ||
| public void getWinner() { | ||
|
|
||
| } | ||
|
|
||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import org.junit.After; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| import java.lang.reflect.InvocationTargetException; | ||
| import java.lang.reflect.Method; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import static org.hamcrest.CoreMatchers.is; | ||
| import static org.junit.Assert.assertThat; | ||
|
|
||
| public class RacingGameTest { | ||
|
Contributor
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. view 를 제외한 모든 클래스에 대해 테스트를 진행해보시면 어떨까요? |
||
| List<Car> cars; | ||
| RacingGame racingGame; | ||
| @Before | ||
| public void setUp() throws Exception { | ||
| cars = new ArrayList<>(); | ||
| cars.add(new Car("애플")); | ||
| cars.add(new Car("삼성")); | ||
| cars.add(new Car("전남대")); | ||
| racingGame = new RacingGame(); | ||
| } | ||
|
|
||
| @Test | ||
| public void race() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { | ||
| Method goOrStay = racingGame.getClass().getDeclaredMethod("goOrStay", Car.class); | ||
| goOrStay.setAccessible(true); | ||
| for(Car car: cars) { | ||
| goOrStay.invoke(racingGame, car); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void getWinner() { | ||
| cars.get(0).goForward(); | ||
| cars.get(1).goForward(); | ||
| List<String> expectedWinnerNames = new ArrayList<>(); | ||
| expectedWinnerNames.add("애플"); | ||
| expectedWinnerNames.add("삼성"); | ||
| List<String> actualWinnerNames = racingGame.getWinner(cars); | ||
| assertThat(actualWinnerNames,is(expectedWinnerNames)); | ||
| } | ||
|
|
||
| @After | ||
| public void tearDown() throws Exception { | ||
| cars = null; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import org.junit.Test; | ||
|
|
||
| import static org.hamcrest.CoreMatchers.either; | ||
| import static org.hamcrest.CoreMatchers.is; | ||
| import static org.junit.Assert.*; | ||
|
|
||
| public class RuleTest { | ||
| @Test | ||
| public void isGoForward() { | ||
| assertThat(Rule.isGoForward(),either(is(true)).or(is(false)) ); | ||
| } | ||
| } |
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.
랜덤을 통해 특정 조건에서 전진하는 게임의 룰을 자동차라는 모델 클래스가 알고 있을 필요가 있을까요?
예를 들어 게임의 모드가 추가되어서 랜덤 룰이 아닌 다른 게임 룰이 생긴다고 했을 때,
모델이 게임의 룰에 관계없이 pure 하게 짜여져 있으면 모델을 수정하지 않아도 되지 않을까요?
moveForward 가 단순히 car 의 position 을 ++ 하는 로직만 가지고 있으면 어떤지 의견드려봅니다 ~
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.
게임의 룰을 자동차 모델 클래스가 가지고 있을 필요는 없다고 생각했는데 보다 더 좋은 방법을 아직 생각해보지 못한 것 같습니다. 원래는 Rule 클래스를 따로 만들어 구현하려고 했으나 아직 방법을 고민 중에 있습니다! 고민해서 반영할 수 있도록 노력해보겠습니다