Step3 (사다리 게임 실행) 리뷰 요청드립니다. #446
Merged
Conversation
This file contains 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
ssosso
requested changes
Jun 14, 2020
| @DisplayName("Direction Enum 객체 정상 생성 테스트") | ||
| @ParameterizedTest | ||
| @MethodSource("mockDirectionBuilder") | ||
| public void getDirection(int index, Direction expectedDirection) { |
There was a problem hiding this comment.
질문 주신 부분에 답변을 드리자면 테스트는 한글로 명시해도 문제 없습니다.
실제로 배포시에는 컴파일에 포함되지 않아서요ㅎ
| List<Player> players = playerNames.stream() | ||
| .map(Player::new) | ||
| .collect(Collectors.toList()); | ||
| return new PlayersGroup(players); | ||
| } | ||
|
|
||
| private static void validatePlayerCounts(List<String> playerNames) { | ||
| if (playerNames.size() < MINIMUM_PLAYER_COUNTS) { |
There was a problem hiding this comment.
playerNames.size()
를 중복으로 호출하는데 한번으로 리펙토링 가능해 보입니다.
| if (randomNumber < RANDOM_NUMBER_BOUNDARY) { | ||
| return new Point(false); | ||
| public static Point drawFirstPoint(boolean isDown) { | ||
| if (isDown) { |
There was a problem hiding this comment.
개인적으로 이런 로직에서는 삼항연산자를 사용하는게 깔끔해 보이더라구요.
어떻게 생각하시나요?
There was a problem hiding this comment.
로직이 복잡하지 않아서 그 편이 더 간결하고 눈에 확 들어오겠네요!!
ssosso
approved these changes
Jun 16, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
안녕하세요.
코멘트 남겨주신 두 가지 부분의 피드백 반영해서 코드를 수정했습니다. 확인 부탁드립니다. :)
안녕하세요 코드가 맘에 안들어서 계속 갈아 엎다보니 시간도 오래 소요되고, 커밋 단위도 뭔가 커졌네요. 처음에는 작게 유지하려고 노력했는데... ㅠ ㅠ
Test 메소드명을 한글로 지어도 무방할까요?
그 외 부족한 부분 코멘트 부탁드립니다.