[1주차] 정재우 #2
[1주차] 정재우 #2
Conversation
게임을 플레이하는 플레이어에 관한 객체
플레이어의 숫자를 나타내는 변수
플레이어의 숫자와 관련된 기능을 담담하는 객체
입력과 출력을 담당하는 객체
플레이어 숫자와 관련된 유틸리티 기능을 담당하는 클래스
같은 패키지 외에서는 NumberUtil의 인스턴스를 생성을 막고, 정적 메소드를 호출할 수 있도록 함.
숫자 비교에 따른 힌트와 관련된 클래스
strike 개수와 ball 개수를 나타내는 변수
Hint 클래스가 처음 생성될 때, strike과 ball의 개수를 모두 0으로 초기화 한다.
해당 로직이 ASCII 값을 기반으로 숫자를 검증하도록 수정
There was a problem hiding this comment.
docs/README.md가 아니라 원본 README.md를 변경한 이유가 궁금합니당
There was a problem hiding this comment.
ㅋㅋㅋㅋ아니 이거는 docs 폴더에도 리드미가 있는 줄 몰랐습니다... 바로 옮겨야겠네요
| public List<Integer> getRandomNumber() { | ||
| Set<Integer> numberSet = new HashSet<>(); | ||
| while (numberSet.size() < 3) { | ||
| numberSet.add(Randoms.pickNumberInRange(1, 9)); | ||
| } | ||
|
|
||
| return new ArrayList<>(numberSet); | ||
| } | ||
|
|
||
| public Hint getHint(Player computer, Player player) { | ||
| List<Integer> computerNumber = computer.getNumbers(); | ||
| List<Integer> playerNumber = player.getNumbers(); | ||
| Hint hint = new Hint(); | ||
|
|
||
| compareNumber(computerNumber, playerNumber, hint); | ||
|
|
||
| return hint; | ||
| } |
There was a problem hiding this comment.
저는 get*** 메소드를 들었을 때, getter의 인식이 강하게 들었어요.
RandomNumber를 만드는 메소드니까 generate, create등 메소드 명을 고려해봐도 좋을 것 같아요.
There was a problem hiding this comment.
확실히 get 말고 다른 이름을 쓰는 게 좀 더 명시적인 것 같습니다...!
| if (Objects.equals(myNumber, computerNumber.get(i))) { | ||
| hint.increaseStrike(); | ||
| } else if (computerNumber.contains(myNumber)) { | ||
| hint.increaseBall(); | ||
| } |
There was a problem hiding this comment.
스트라이크 비교로직이랑 볼 비교로직도 분리하면 더 직관적일 것 같아요!
There was a problem hiding this comment.
확실히 처음 보면 많이 복잡한 느낌이 있기는 하네요...! 수정해보겠습니당
There was a problem hiding this comment.
NumberService, ViewService는 멤버변수가 없는 객체인 것 같아요.
멤버변수가 없는 객체를 new로 생성해야만 하는 이유가 있는지 궁금합니다!
만약 뾰족한 이유가 없다면, static으로 처리해서 생성자로 생성하는 부분을 해치워도 좋을 것 같구요!
There was a problem hiding this comment.
별 생각 없이 의존성 주입을 생각하고 저렇게 코드를 짰었는데 다시보니 딱히 저럴 필요가 없었던 것 같네요. 말씀하신 것처럼 static 처리하는 게 나을 것 같습니다...! 찾아보니까 이런 식으로 인스턴스를 한 개만 생성하는 게 싱글톤 패턴이라고 하네요. 이름만 많이 들어봤는데 이번에 배우고 갑니다!
| Player computer = prepareComputer(); | ||
|
|
||
| play(computer); | ||
|
|
||
| } while (!Objects.equals(viewService.readNumber(), "2")); // 종료(2)를 누르지 않으면 게임을 다시 시작한다. | ||
| } | ||
|
|
||
| private void play(Player computer) { | ||
| while (true) { | ||
| Player player = preparePlayer(); |
There was a problem hiding this comment.
Player computer
Player player
플레이어 객체의 변수명이 조금 헷갈리는 이슈!
There was a problem hiding this comment.
인정합니다... 만들다보니까 computer랑 player가 각각 역할이 좀 다르긴 해서 다른 객체로 만들어봐도 좋았을 것 같습니다
| public boolean isStrikeExist() { | ||
| if (strike > 0) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
| public boolean isStrikeExist() { | |
| if (strike > 0) { | |
| return true; | |
| } | |
| return false; | |
| } | |
| public boolean isStrikeExist() { | |
| return strike > 0; | |
| } | |
| ``` |
| public boolean isBallExist() { | ||
| if (ball > 0) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| public boolean isThreeStrike() { | ||
| if (strike == 3) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
| protected static void validateLength(String number) { | ||
| if (number.length() != 3) { | ||
| throw new IllegalArgumentException("입력값은 3자리이어야 합니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
이번 과제를 하면서 제가 하나 포인트로 잡고 갔던 부분이 있었어요.
요구조건이 갑자기 변한다면 유연하게 대처할 수 있는 프로그램을 만드는 것이 목표였어요.
가령 입력 숫자가 갑자기 3자리에서 5자리로 바뀌는 경우,
숫자에 0이 포함되어 랜덤 숫자를 만들어야 하는 경우.
이러한 상황에서 최소한의 변화로 유연하게 대처할 수 있도록 설계하려고 노력했어요!
그래서 이런 상수를 Enum이나, static final 변수로 분리해서 활용하면 좋을 것 같아요.
There was a problem hiding this comment.
저도 만들면서 규모가 커지면 이게 다 하드코딩이 되지 않을까 생각했는데 역시 검증 관련 에러메시지나 View 쪽 텍스트들은 Enum을 적용해야하지 않을까 생각합니다. static final 같은 경우는 아직 정확하게 어떻게 써야할 지 감이 잘 안 잡히는데 이번 기회에 공부를 더 해봐야 할 것 같습니다...!!!
| public static void validate(String number) { | ||
| validateEmpty(number); | ||
| validateLength(number); | ||
| validateNumber(number); | ||
| validateUnique(number); | ||
| validateSign(number); | ||
| validateRange(number); | ||
| } |
There was a problem hiding this comment.
Player객체를 private List numbers로 설정하시는 이유가 아마 일급 컬렉션 사용을 목표로 작성하셨을 거라 조심스럽게 예측해봅니다.
궁극적으로 이 과제에서 일급 컬렉션을 쓰는 이유는, 생성자를 통해 생성된 Player 객체가 해당 숫자에 대한 검증이 완전히 마쳐진 상태로 final하게 관리되기 때문이라고 생각하는데, 해당 로직이 생성자를 통해 검증되면 더 좋을 것 같다는 생각입니다!
There was a problem hiding this comment.
일급 컬렉션...이 뭔지 몰랐는데 운으로 때려 맞춰서 저렇게 설계했네요😂 찾아보니까 말씀하신대로 생성자에 미리 검증 로직을 넣어서 생성 시점에 검증을 마칠 수 있고, 그 외에도 클래스 외부에서는 그 값을 변경시킬 수 없는 장점이 있는 것 같습니다. 하나 또 배우고 갑니다...!
해당 로직이 ASCII 값을 정수 값으로 변환하여 검증하도록 수정
1. final을 통한 재할당 방지 2. 방어적 복사 적용을 통해 생성 시점의 변수 조작 방지 3. unmodifiableList 적용을 통해 getter 사용으로 인한 변수 조작 방지
1메소드당 1역할로 하려고 이것 저것 손 보기는 했는데 뭔가 좀 분리가 제대로 안 된 것 같은 느낌이 계속 나네요ㅋㅋㅋ getter setter도 바꿔야 될 것 같고 암튼 계속 고쳐봐야겠습니다...