-
Notifications
You must be signed in to change notification settings - Fork 452
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
[엘리] 자동차 경주 미션 제출합니다. #106
Changes from 28 commits
0022dd7
ba078de
8a0924e
d305c05
b4956f0
1398f05
643b30f
98e65a3
4f0db60
710a7e6
7622dfa
5133e61
efb2554
2454c99
032e9f9
b2be83a
18cfd3d
f39db65
e86547e
eea92f6
dedc0c3
199cebf
a6a1ee3
22c2e95
e33873e
a90a426
e69ee92
3523bbf
c3835b9
777038d
0b5d5df
45b96f6
912642c
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,5 +1,44 @@ | ||
# java-racingcar | ||
자동차 경주 게임 미션 저장소 | ||
# 문자열 덧셈 계산기 | ||
## 요구사항 | ||
- [x] 쉼표(,) 또는 콜론(:)을 구분자로 가지는 문자열을 전달하는 경우 구분자를 기준으로 분리한 각 숫자의 합을 반환 | ||
- [x] 앞의 기본 구분자(쉼표, 콜론)외에 커스텀 구분자를 지정할 수 있다. 커스텀 구분자는 문자열 앞부분의 “//”와 “\n” 사이에 위치하는 문자를 커스텀 구분자로 사용한다. | ||
- [x] 문자열 계산기에 숫자 이외의 값 또는 음수를 전달하는 경우 RuntimeException 예외를 throw한다. | ||
|
||
## 우아한테크코스 코드리뷰 | ||
* [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md) | ||
|
||
## 프로그래밍 요구사항 | ||
- indent(들여쓰기) depth를 2단계에서 1단계로 줄여라. | ||
- depth의 경우 if문을 사용하는 경우 1단계의 depth가 증가한다. if문 안에 while문을 사용한다면 depth가 2단계가 된다. | ||
- 메소드의 크기가 최대 10라인을 넘지 않도록 구현한다. | ||
- method가 한 가지 일만 하도록 최대한 작게 만들어라. | ||
- else를 사용하지 마라. | ||
|
||
|
||
# 자동차 경주 게임 미션 저장소 | ||
## 요구사항 | ||
- [x] 자동차 이름 입력 받기 | ||
- [x] 쉼표 기준으로 구분 | ||
- 예외처리 | ||
- [x] 공백이 입력되었을 경우 | ||
- [x] 이름이 6자 이상 입력되었을 경우 | ||
- [x] 시도할 횟수 입력 받기 | ||
- 예외처리 | ||
- [x] 숫자가 아닌 경우 | ||
- [x] 양수가 아닌 경우 | ||
- [x] 경주할 자동차 생성 | ||
- [x] 자동차에 이름, 포지션 부여 | ||
- [x] 랜덤 값이 4 이상이면 전진, 3 이하이면 멈춘다 | ||
- [x] 위치값 생성(0~9의 랜덤값) | ||
- [x] 자동차에게 랜덤값 전달 | ||
- [x] 실행 결과 출력 | ||
- [x] 전진 횟수를 "-"로 출력 | ||
- [x] 자동차 개수만큼 출력 | ||
- [x] 시도 횟수만큼 출력 | ||
- [x] 최종 우승자 출력 | ||
|
||
## 프로그래밍 요구사항 | ||
- 모든 로직에 단위 테스트를 구현한다. 단, UI(System.out, System.in) 로직은 제외 | ||
- 자바 코드 컨벤션을 지키면서 프로그래밍한다. | ||
- indent(인덴트, 들여쓰기) depth를 3을 넘지 않도록 구현한다. 2까지만 허용한다. | ||
- else 예약어를 쓰지 않는다. | ||
- 함수(또는 메소드)의 길이가 15라인을 넘어가지 않도록 구현한다. | ||
- 함수(또는 메소드)가 한 가지 일만 잘 하도록 구현한다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package calculator; | ||
|
||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
public class StringCalculator { | ||
public static int calculate(String text) { | ||
if (text == null || text.isEmpty()) { | ||
return 0; | ||
} | ||
if (text.contains("-")) { | ||
throw new RuntimeException("message"); | ||
} | ||
return splitAndSum(text); | ||
} | ||
|
||
private static int splitAndSum(String text) { | ||
|
||
Matcher m = Pattern.compile("//(.)\n(.*)").matcher(text); | ||
if (m.find()) { | ||
String customDelimiter = m.group(1); | ||
String[] tokens = m.group(2).split(customDelimiter); | ||
return sum(tokens); | ||
} | ||
String[] numbers = text.split(",|:"); | ||
return sum(numbers); | ||
} | ||
|
||
private static int sum(String[] numbers) { | ||
int result = 0; | ||
for (String number : numbers) { | ||
result += Integer.parseInt(number); | ||
} | ||
return result; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package racingcar; | ||
|
||
import racingcar.domain.Game; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
Game game = new Game(); | ||
game.run(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package racingcar.domain; | ||
|
||
public class Car implements Comparable<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. 아이고! 51페이지부터가 도움이 될 것 같습니다 :) (전체화면 해야지만 내용이 나오네요.) 테스트할 수 없는 영역을 테스트하기 위해 전략패턴을 사용하여 테스트 가능하도록 만들 수 수 있습니다.
숫자를 생성해주는 추상체를 만듭니다.
와 같은 코드를 작성할 수 있습니다.
이렇게 될 때
와 같이 NumberGenerator 를 조작하여 의도한데로 동작하는지 테스트할 수 있습니다. 이와 별게로 NumberGenerator 와 같은 의미없고 추상적인 의미보다는, 해당 클래스가 수행하는 역할을 고려해 거의 다 온 것 같아 간단한 수도코드와 문서를 첨부합니다 :) 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 static final String POSITION_MARK = "-"; | ||
|
||
private Name name; | ||
private int position; | ||
|
||
public Car(Name name) { | ||
this.name = name; | ||
this.position = 0; | ||
} | ||
|
||
public int movePosition(int moveValue) { | ||
if (moveValue >= 4) { | ||
YebinK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
position++; | ||
} | ||
return position; | ||
} | ||
|
||
public String getCurrentPosition() { | ||
StringBuilder currentPosition = new StringBuilder(name.getName()); | ||
currentPosition.append(" : "); | ||
YebinK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
for (int i = 0; i < position; i++) { | ||
currentPosition.append(POSITION_MARK); | ||
} | ||
return currentPosition.toString(); | ||
} | ||
|
||
public boolean isMaxPosition(Car maxPositionCar) { | ||
YebinK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return this.position >= maxPositionCar.position; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return name.getName(); | ||
} | ||
|
||
@Override | ||
public int compareTo(Car car) { | ||
return this.position - car.position; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package racingcar.domain; | ||
|
||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
|
||
public class Cars { | ||
public static final int RANDOM_VALUE_LIMIT = 10; | ||
|
||
private List<Car> cars; | ||
|
||
public Cars(Names names) { | ||
this.cars = names.getNames() | ||
.stream() | ||
.map(Car::new) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
public int createRandomValue() { | ||
YebinK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return (int) (Math.random() * RANDOM_VALUE_LIMIT); | ||
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. NumberGenerator 인터페이스를 만들고 그걸 상속받는 RandomNumber와 FixedNumber를 만들었습니다. 그리고 FixedNumber는 테스트코드에서만 사용되게 변경했습니다! |
||
} | ||
|
||
public void moveCars() { | ||
for (Car car : cars) { | ||
car.movePosition(createRandomValue()); | ||
} | ||
} | ||
|
||
public String getCurrentResult() { | ||
StringBuilder currentResult = new StringBuilder(); | ||
for (Car car : cars) { | ||
currentResult.append(car.getCurrentPosition()); | ||
currentResult.append("\n"); | ||
} | ||
return currentResult.toString(); | ||
} | ||
|
||
public List<Car> getCars() { | ||
return cars; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package racingcar.domain; | ||
|
||
import racingcar.view.InputView; | ||
import racingcar.view.OutputView; | ||
|
||
public class Game { | ||
public void run() { | ||
Names names = InputView.getNames(); | ||
Trial trial = InputView.getTrial(); | ||
|
||
Cars cars = new Cars(names); | ||
|
||
OutputView.printCurrentResultTitle(); | ||
for (int i = 0; i < trial.getTrial(); i++) { | ||
cars.moveCars(); | ||
OutputView.printCurrentResult(cars.getCurrentResult()); | ||
} | ||
Winners.selectWinners(cars.getCars()); | ||
OutputView.printWinnerResult(); | ||
} | ||
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. 개인적으로 run()안에 두 로직(게임 준비, 게임 진행) 이 있다고 판단하여 8-11번 라인을 prepareGame()이라는 함수로 묶고, 13-19번 라인을 playGame()이라는 함수로 묶고 싶었습니다. 하지만 그럴 경우 trial과 cars라는 인스턴스 변수를 생성해야 하는 문제가 발생했습니다. 인스턴스 변수 생성을 최소화하라는 피드백이 있어서 우선은 run()함수 안에 전체 로직을 다 구현했는데요, 이렇게 하면 로직이 복잡해졌을 때 run()함수가 너무 길어질 것 같아 걱정됩니다. 혹시 현장에서는 어떻게 구현하시는지 궁금합니다! 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. 이 부분은 Trade Off 입니다. 극도의 가독을 가져가면서 인스턴스를 생성하는 비용을 지불할 것인지, 현 수준의 가독을 가져가면서 인스턴스 생성 비용을 절약할 것인지. 이것은 그 상황과 경우에 따라 많이 다를 듯 합니다 :) 인스턴스 생성 비용은 요즘 시대에는 거의 신경쓰지 않아도 됩니다. 성능이 많이 좋아졌거든요. 그렇지만 우리에게는 최적화해야할 의무 또한 있습니다. 저 역시 인스턴스 생성에 관대한 편이지만, 위와 같은 코드에서는 사용하지 않고 효율을 취하고 싶네요 :) 모든 것을 극악의 가독으로 만들 순 없습니다. 현 교육에서 제시하는 컨벤션 정도가 그 선이 되면 좋을 것 같습니다 :) 하지만 때로는 극악까지 가독을 끌어올려보는 것 또한 권장합니다. 메서드와 객체를 분리하여 가독을 올렸을 때, 변경에 대한 유연한 변화가 문제가 될 수 있으며, 잘못된 설계 기반에서 극악의 분리를 하였을 때 일부를 변경하지 못하고 모든 것을 변경해야 하는 시행착오 또한 겪을 수 있을 것 입니다. 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. 그렇군요! 이 부분은 경험치를 더 많이 쌓아야 할 것 같아요. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package racingcar.domain; | ||
|
||
public class Name { | ||
public static final int NAME_LENGTH_LIMIT = 5; | ||
|
||
private String name; | ||
|
||
public Name(String name) throws IllegalArgumentException { | ||
this.name = name; | ||
isCorrectName(); | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
|
||
public void isCorrectName() throws IllegalArgumentException { | ||
isBlankValidation(); | ||
isOverSixLetters(); | ||
} | ||
|
||
private void isBlankValidation() { | ||
if (name.replace(" ", "").isEmpty()) { | ||
throw new IllegalArgumentException("공백이 입력되었습니다."); | ||
} | ||
} | ||
|
||
private void isOverSixLetters() { | ||
if (name.length() > NAME_LENGTH_LIMIT) { | ||
throw new IllegalArgumentException("이름은 5자 이내로 작성해주세요."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package racingcar.domain; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
|
||
public class Names { | ||
public static final String COMMA = ","; | ||
|
||
private final List<Name> names; | ||
|
||
public Names(String carNames) throws IllegalArgumentException { | ||
this.names = Arrays.stream(nameSplitValidation(carNames)) | ||
.map(Name::new) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
private String[] nameSplitValidation(String carNames) { | ||
String[] splittedNames = carNames.split(COMMA); | ||
if (splittedNames.length == 0 || carNames.contains(",,")) { | ||
throw new IllegalArgumentException("이름을 입력해주세요."); | ||
} | ||
return splittedNames; | ||
} | ||
|
||
public List<Name> getNames() { | ||
return names; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package racingcar.domain; | ||
|
||
public class Trial { | ||
private static final int GAME_COUNT_MINIMUM = 1; | ||
private int trial; | ||
|
||
public int getTrial() { | ||
return trial; | ||
} | ||
|
||
public Trial(String trial) throws Exception { | ||
this.trial = isNotNumber(trial); | ||
isNotPositiveNumber(); | ||
} | ||
|
||
private int isNotNumber(String trial) { | ||
try { | ||
return Integer.parseInt(trial); | ||
} catch (NumberFormatException e) { | ||
throw new NumberFormatException("시도 횟수는 숫자를 입력해주세요."); | ||
} | ||
} | ||
|
||
private void isNotPositiveNumber() { | ||
if (this.trial < GAME_COUNT_MINIMUM) { | ||
throw new IllegalArgumentException("시도 횟수는 양수만 입력해주세요."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package racingcar.domain; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
|
||
public class Winners { | ||
private static List<Car> winners = new ArrayList<>(); | ||
|
||
public static void selectWinners(List<Car> cars) { | ||
Car maxDistanceCar = cars.stream() | ||
.max(Car::compareTo) | ||
.get(); | ||
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. get 은 nullable 상태입니다. Stream 을 사용한다면 null 이 나올 경우를 항상 체크를 해주어 명확한 코드를 작성하는 것을 권장합니다. Optaionl 에서 get 보다는 항상 orElse~ 로 값을 반환받는 것이 안전합니다. 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. 아 미처 생각지 못했던 부분이네요, 감사합니다! |
||
for (Car car : cars) { | ||
if (car.isMaxPosition(maxDistanceCar)) { | ||
winners.add(car); | ||
} | ||
} | ||
} | ||
|
||
public static String getWinners() { | ||
return winners.stream() | ||
.map(Car::toString) | ||
.collect(Collectors | ||
.joining(",")); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package racingcar.view; | ||
|
||
import racingcar.domain.Names; | ||
import racingcar.domain.Trial; | ||
|
||
import java.util.Scanner; | ||
|
||
public class InputView { | ||
private static final Scanner scanner = new Scanner(System.in); | ||
|
||
public static Names getNames() { | ||
try { | ||
OutputView.askCarNames(); | ||
String carNames = scanner.nextLine(); | ||
return new Names(carNames); | ||
} catch (IllegalArgumentException e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return getNames(); | ||
} | ||
} | ||
|
||
public static Trial getTrial() { | ||
try { | ||
OutputView.askTrial(); | ||
String trial = scanner.nextLine(); | ||
return new Trial(trial); | ||
} catch (Exception e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return getTrial(); | ||
} | ||
} | ||
} |
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.
로직이 있는 함수 run()을 Game 클래스에 두고, 이를 main에서 호출하는 방식과
로직 자체를 main 함수에서 구현하는 방식을 두고 고민이 많았습니다. 개인적으로는 main 함수에서 로직을 숨기고 싶어서 이렇게 구현했는데 혹시 킹뽀대님은 어떻게 생각하시는지 궁금합니다!