[홍섭] 숫자 야구 #2
[홍섭] 숫자 야구 #2
Conversation
| Judgement judgement = new Judgement(); | ||
| Printer printer = new ConsolePrinter(); | ||
|
|
||
| Game game = new Game(numberGenerator, reader, judgement, printer); |
There was a problem hiding this comment.
여기에 명시되어 있지 않지만 reader, printer가 Game 안으로 들어가서 도메인 로직에 섞이지 않는 조건이 있습니다 홍섭님
| while (true) { | ||
| try { | ||
| printer.print("숫자를 입력해주세요 : "); | ||
| String input = reader.read(); | ||
| List<Integer> playerNumbers = Converter.convertStringToNumberList(input); | ||
|
|
||
| String result = judgement.compareNumber(computerNumbers, playerNumbers); | ||
| printer.print(result); | ||
|
|
||
| if (judgement.isGameWon(result)) { | ||
| printer.print("3개의 숫자를 모두 맞히셨습니다! 게임 종료"); | ||
| if (!askRetry()) { | ||
| break; | ||
| } | ||
| computerNumbers = numberGenerator.generatorRandomNumber(RuleConstants.REQUIRED_LENGTH); | ||
| printer.print("새로운 게임을 시작합니다."); | ||
| } | ||
| } catch (IllegalArgumentException e) { | ||
| printer.print(e.getMessage()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
저희 객체지향 생활체조 들여쓰기 원칙에 맞춰서 리팩토링 되어야 할 것 같습니다 아마 출력 부분이 들어오면서 생긴 부분같습니다!
|
|
||
| public class Judgement { | ||
|
|
||
| public String compareNumber(List<Integer> computerNumberList, List<Integer> playerNumberList) { |
There was a problem hiding this comment.
List는 의미를 가지는 객체가 될 수 있을 것처럼 보이는데 어떠신가요?
| private int getStrikeCount(List<Integer> computerNumberList, List<Integer> playerNumberList) { | ||
| int strike = 0; | ||
|
|
||
| for (int placeIndex = 0; placeIndex < playerNumberList.size(); placeIndex++) { | ||
| if (hasNumberInPlace(computerNumberList, placeIndex, playerNumberList.get(placeIndex))) { | ||
| strike++; | ||
| } | ||
| } | ||
| return strike; | ||
| } |
| Validator.validateListSize(size); | ||
|
|
||
| List<Integer> numberList = new ArrayList<>(); | ||
| Random random = new Random(); |
There was a problem hiding this comment.
이 친구는 한번만 선언되고 쓸 수 있어 보이는데 어떠신가요?
| while (numberList.size() < size) { | ||
| int generatedNumber = random.nextInt(MAX_NUMBER) + MIN_NUMBER; | ||
| if (!numberList.contains(generatedNumber)) { | ||
| numberList.add(generatedNumber); | ||
| } | ||
| } |
java-playground/src/main/java/org/gonza/javaplayground/view/ConsoleReader.java
Show resolved
Hide resolved
java-playground/src/test/java/org/gonza/javaplayground/core/GameTest.java
Outdated
Show resolved
Hide resolved
hongxeob
requested review from
taiyoung12,
currenjin,
Heonbyeong,
leeMK09 and
gyeongjae-ham
|
|
||
| public class Converter { | ||
|
|
||
| public static List<Integer> convertStringToNumberList(String input) { |
There was a problem hiding this comment.
숫자로 이루어진 문자열이 아닌 경우가 발생할 수 있지 않을까요?
작성자는 Validate에서 검증한다 인지할 수 있지만, 해당 함수만 봤을 때에는 오류의 가능성이 보여서요!
There was a problem hiding this comment.
해당 부분은 단순하게 숫자로 이루어진 문자열을 변환만 해주는 역할로 썼습니다!
그래서 호출하는 쪽 (사용자에게 입력 받는 부분)에서 이미 검증을 한 상태입니다. 그래서 말씀주신 숫자로된 문자열 형태가 아닐시 그 전에 예외가 터집니다!
근데 현진님 말씀대로 이 안에서 한 번 더 검증하는 것도 맞는거 같습니다.
| } | ||
|
|
||
| @Override | ||
| public String read() { |
There was a problem hiding this comment.
해당 함수가 withValidate이면 어떨까요? read만 갖고있는 것 같진 않아서요!
아래 함수가 오히려 read의 역할만을 수행한다 생각하는데, 어떠신가요?
There was a problem hiding this comment.
다시 메서드를 읽어보니 그게 의도가 더 명확할 듯 하네요! 굿
| private final Judgement judgement; | ||
| private final Printer printer; | ||
|
|
||
| public Game(NumberGenerator numberGenerator, Reader reader, Judgement judgement, Printer printer) { |
There was a problem hiding this comment.
Game의 역할에 대해서 어떻게 생각하시나요?
사용자의 입력과 출력까지의 역할을 담당하는 것으로 보이는데, 그게 옳을까라는 생각이 들어서 여쭤봅니당
There was a problem hiding this comment.
(동일한 의견) 숫자 야구라는 게임 전체적인 흐름을 담당하는 부분을 의도하고 만드신 것이라고 가정했을 때, 입출력 관련 부분은 입력 담당, 출력 담당의 책임으로 분리할 수 있지 않을까요 ?
| @Test | ||
| @DisplayName("랜덤 숫자를 요청 사이즈에 맞게 생성한다.") | ||
| void generateRandomNumberSuccessTest() throws Exception { | ||
|
|
||
| //given | ||
| int requestSize = 3; | ||
|
|
||
| //when | ||
| List<Integer> generatedNumber = generator.generatorRandomNumber(requestSize); | ||
|
|
||
| //then | ||
| assertThat(generatedNumber).hasSize(requestSize); | ||
| assertThat(generatedNumber).doesNotHaveDuplicates(); | ||
| assertThat(generatedNumber).allMatch(n -> n >= 1 && n <= 9); | ||
| } |
There was a problem hiding this comment.
랜덤 숫자의 범위를 테스트하고 있는데 랜덤 숫자 값 자체를 제어해서 테스트할 수 있어야 한다고 생각하는데 어떠신가요?
There was a problem hiding this comment.
랜덤 값 자체를 지정할 수 있어야 한다는 걸까요?
만약 그런 의미라면 제 생각으론 '랜덤'의 의미가 없어지지 않나 라는 생각이 듭니다!
There was a problem hiding this comment.
음 그냥 랜덤인 범위를 검증하는 것보다 그걸 객체지향적으로 테스트할 때는 고정적인 값을 이끌어 낼 수 있다!!
저 부분이 핵심인 것 같습니다!
| } | ||
|
|
||
| public void play() { | ||
| printer.print("숫자 야구 게임을 시작합니다."); |
There was a problem hiding this comment.
printer 를 통해서 출력해달라는 메시지를 전달하는건 이해했는데
출력에 대한 구체적인 내용까지 Game 이 핸들링 하는 것보다 printer 라고 책임을 가졌으니
printer 가 이런 책임까지 처리하는 건 어떤가유?
| try { | ||
| String input = reader.readWithoutValidation(); | ||
| if (input.equals("1")) return true; | ||
| if (input.equals("2")) return false; |
There was a problem hiding this comment.
재시작을 물어보는 로직인데 2 인 경우까지 판단할 필요는 없을 것 같은데 어떻게 생각하시나유?
1 (재시작) 인 것이 아니면 예외
There was a problem hiding this comment.
GameCommand 객체를 만들어 책임을 부여해서 조금 더 명확하게 했습니다!
| private final Judgement judgement; | ||
| private final Printer printer; | ||
|
|
||
| public Game(NumberGenerator numberGenerator, Reader reader, Judgement judgement, Printer printer) { |
There was a problem hiding this comment.
(동일한 의견) 숫자 야구라는 게임 전체적인 흐름을 담당하는 부분을 의도하고 만드신 것이라고 가정했을 때, 입출력 관련 부분은 입력 담당, 출력 담당의 책임으로 분리할 수 있지 않을까요 ?
There was a problem hiding this comment.
UI 관련 테스트 코드는 작성하지 않아도 된다고 명시되어 있습니다 ! (참고)
There was a problem hiding this comment.
UI 관련 테스트 코드는 작성하지 않아도 된다고 명시되어 있습니다 ! (참고)
⚾️ 숫자 야구 기능 목록 Check List
ballstrike낫싱