[럿고] 자동차 경주 게임 제출합니다.#2
Open
ksy90101 wants to merge 20 commits intoWoohan-TDD:masterfrom
Open
Conversation
added 16 commits
June 22, 2020 13:18
- 5자 초과시 예외 발생
- 2미만이거나 10을 초과했을 경우 예외처리
- 2미만이거나 10을 초과했을 경우 예외처리
- 위치는 0 ~ 10만 가지고 있다.
hongbin-dev
reviewed
Jun 25, 2020
Member
hongbin-dev
left a comment
hongbin-dev
left a comment
There was a problem hiding this comment.
안녕하십니까 럿고.
어제 리뷰 남기기로했는데ㅠ 잊고있었네요.
구현하느라 고생하셨습니다. 개인적인 의견 남겨봅니다.
감사합니다.✌
| @@ -0,0 +1,7 @@ | |||
| package domain.car; | |||
|
|
|||
| @FunctionalInterface | |||
Comment on lines
+4
to
+6
| public interface CarMoveStrategy { | ||
|
|
||
| int move(); |
Member
There was a problem hiding this comment.
interface만 봤을 때 이동하는 전략일까? 라는 생각이 드는 네이밍이에요.
move의 메서드이름도 integer를 생성하는 일만 하고있는데 move가 맞을까 생각이 듭니다.
Author
There was a problem hiding this comment.
네이밍을 조금 더 고민했어야 하는데, ㅎㅎ 감사합니다~
| public class RacingController { | ||
|
|
||
| private static final String SPLIT_DELIMITER = ","; | ||
| private static final CarMoveStrategy carMoveStrategy = () -> (int)(Math.random() * 10); |
Comment on lines
+40
to
+44
| OutputView.runResultGuide(); | ||
| for (int i = 0; i < gameCount.getGameCount(); i++) { | ||
| cars.moveCars(carMoveStrategy); | ||
| OutputView.printRunResult(cars.getCars()); | ||
| } |
Member
There was a problem hiding this comment.
for-each문으로 바꿀수있는 방법은 없을까요?
Author
There was a problem hiding this comment.
getCount.getGameCount()가 int인데 어떻게 해야 하나요.. ㅠㅠ 정말 몰라서 물어봅니다 ㅠㅠ
Comment on lines
+47
to
+54
| private List<Car> createCars(final String inputCarNames) { | ||
| List<String> carNames = Arrays.stream(inputCarNames.split(SPLIT_DELIMITER)) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| return carNames.stream() | ||
| .map(Name::new) | ||
| .map(name -> new Car(name, PositionFactory.of(0))) | ||
| .collect(Collectors.toList()); |
Member
There was a problem hiding this comment.
Suggested change
| private List<Car> createCars(final String inputCarNames) { | |
| List<String> carNames = Arrays.stream(inputCarNames.split(SPLIT_DELIMITER)) | |
| .collect(Collectors.toList()); | |
| return carNames.stream() | |
| .map(Name::new) | |
| .map(name -> new Car(name, PositionFactory.of(0))) | |
| .collect(Collectors.toList()); | |
| private Cars createCars(final String inputCarNames) { | |
| List<String> carNames = Arrays.stream(inputCarNames.split(SPLIT_DELIMITER)) | |
| .collect(Collectors.toList()); | |
| return carNames.stream() | |
| .map(Name::new) | |
| .map(name -> new Car(name, PositionFactory.of(0))) | |
| .collect(collectingAndThen(toList(), Cars::new)); |
이렇게 가져가도 가독성이 좋을 것 같아요.
Comment on lines
+9
to
+11
| protected Position(final int position) { | ||
| this.position = position; | ||
| } |
Member
There was a problem hiding this comment.
패키지레벨을 더 디테일하게 만들고, default 접근지정자는 어떨까요?
Comment on lines
+9
to
+22
| private static final List<Position> positions = IntStream.range(0, 11) | ||
| .mapToObj(Position::new) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| private PositionFactory() { | ||
|
|
||
| } | ||
|
|
||
| public static Position of(int positionValue) { | ||
| return positions.stream() | ||
| .filter(position -> position.getPosition() == positionValue) | ||
| .findFirst() | ||
| .orElseThrow(() -> new IllegalArgumentException("나올 수 없는 위치 입니다. position = " + positionValue)); | ||
| } |
Member
There was a problem hiding this comment.
-
10개밖에 없어서 큰 차이는 아니지만 Map<Integer, Position> map 으로 갖고있으면, of에서 순회하면서 찾는것보다 빠를것 같아요.
-
Intstream.rangeClosed(0,10)이 가독성이 더 좋을 것 같군요
Author
There was a problem hiding this comment.
- 다음에 고려해볼께요. 절대로 귀찮아서가 아닙니다!
- 그렇네요~
Comment on lines
+9
to
+11
| private static final List<GameCount> gameCounts = IntStream.range(2, 11) | ||
| .mapToObj(GameCount::new) | ||
| .collect(Collectors.toList()); |
Member
There was a problem hiding this comment.
stream.rangeClosed() 사용을 추천드리고, 상수 값을 활용해볼까요?
Comment on lines
+25
to
+31
| private static void validateNotNumber(final String gameCount){ | ||
| try{ | ||
| Integer.parseInt(gameCount); | ||
| }catch (NumberFormatException e){ | ||
| throw new NumberFormatException("숫자만 입력이 가능합니다. gameCount = " + gameCount); | ||
| } | ||
| } |
Comment on lines
+28
to
+30
| String winner = winners.stream() | ||
| .map(Car::getName) | ||
| .collect(Collectors.joining(", ")); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
오랜만에 하니깐 재미있네요 ㅎㅎ
전체적으로 부족한 코드인데 마음껏 까주세요 ㅎㅎ
잘 부탁드려요!