[5기] 3주차 Wordle 과제 제출 - 점프 #34
Conversation
feat: filepath config class
feat: 게임 안내 문구 출력 뷰 컴포넌트
feat: 비교 로직 feat: 비교결과 일급 객체
feat: 힌트 객체
InputWordTest -> AnswerTest로 위치 변경
hint 판단로직 함수로 분리함
바로 접근 가능
| public enum Hint { | ||
|
|
||
| CORRECT("🟩"), //\uD83D\\uDFE9 | ||
| EXIST("🟨"), //\uD83D\\uDFE8 | ||
| NOT_EXIST("⬜"); | ||
|
|
||
| private final String tile; |
There was a problem hiding this comment.
게임의 결과를 보여줄 타일들을 Enum 타입으로 정의 해놓으셨네요 👍
개인적으로 🟩 와 같은 타일의 모양들은 view 의 영역이라 생각되어서 도메인 영역과 분리되면 좋을 것 같다 생각하는데 어떻게 생각하시나요? 😃
There was a problem hiding this comment.
장단점이 있는것같아요
저도 분리파였는데 Hint 종류가 늘어났을때 view에 반영이 안될수 있는게 더 손해인것같아서 설득당했어요
미리 누락을 방지할만한 방법이 있을까요?
There was a problem hiding this comment.
맞습니다. 장단점이 분명 존재하죠 👍
이런식의 접근은 어떨까요?
- Hint 의 종류가 늘어났을때, view 와 분리가 되어 있다면, view 도 추가 작업이 생기는 것이 맞는 순환이라 생각해요.
- view 가 웹으로 분리된다고 가정하면, 타일의 모양은 css 나 html 로 처리될 가능성이 크다고 생각해요.
점프님 생각은 어떠신가요? 😃
There was a problem hiding this comment.
와잼님 말이 백번 맞지만 실제로 누락을 자주 봐서 공감이 갔달까요..ㅋㅋ
enum 같은거 서버에만 추가되서 클라쪽은 깨지거나 기본값으로 나오거나 하는..
There was a problem hiding this comment.
그럴수도 있겠네요! 😮 저는 Front-Back 작업을 둘 다 하다보니 둘을 최대한 분리하는 관점으로 많이 생각하게 되는 것 같아요. 두 팀이 나눠져 있는데 prod 에 나갈때까지 누락 되는 부분이 있다면, 협업관점에서 문제가 있는 부분이 아닐까 라는 생각도 듭니다 🤔
한쪽에서만 관리하면 좋긴 하겠지만 상황에 맞춰 적절하게 선택을 하면 좋을 것 같습니다! 👍
src/main/java/domain/Hint.java
Outdated
| public static boolean isCorrect(Hint hint) { | ||
| return Hint.CORRECT.equals(hint); | ||
| } |
There was a problem hiding this comment.
enum 내부에 CORRECT 가 맞는지 체크하는 메서드를 만드셨군요 💯
static 메서드라는 점이 아쉬운데요! MatchResult.isWinning 에서만 사용되고 있는데, static 이 아니어도 되지 않을까요? enum 도 하나의 객체이니 객체에게 직접 물어보는 방식은 어떨까요? :)
| public class MatchResult implements Iterable<Hint>{ | ||
| private final List<Hint> hints; | ||
|
|
||
| public MatchResult(List<Hint> hints) { | ||
| this.hints = hints; | ||
| } |
| public class MatchResults implements Iterable<MatchResult> { | ||
| private final List<MatchResult> results; | ||
|
|
||
| public MatchResults() { | ||
| this.results = new ArrayList<>(); | ||
| } | ||
|
|
||
| @Override | ||
| public Iterator<MatchResult> iterator() { | ||
| return this.results.iterator(); | ||
| } | ||
|
|
||
| public void add(MatchResult matchResult) { | ||
| this.results.add(matchResult); | ||
| } | ||
| } |
There was a problem hiding this comment.
MatchResults 도 일급컬렉션으로 만드신점 👍
다만, 만들어진 MatchResult와 MatchResults 일급컬렉션의 책임이 조금 부족한것 같아요. 😢
좀 더 객체에게 책임을 부여해보는건 어떨까요? 😃
There was a problem hiding this comment.
넵 동의합니다 컨트롤러에서 승패판단로직을 뺏어와야겠네요
src/main/java/domain/Round.java
Outdated
| private Round(int limit, int current) { | ||
| this.limit = limit; | ||
| this.current = current; | ||
| } | ||
|
|
||
| public Round(int limit) { | ||
| this(limit, 1); |
There was a problem hiding this comment.
Round를 객체로 도출과 주 생성자, 부생성자의 활용 💯
생성시점에 limit 와 current 는 음수가 되거나 limit 가 current 보다 작은게 들어와도 괜찮을까요? 🤔
| @DisplayName("입력단어 유효성 검증 성공 테스트") | ||
| void validateSuccessInputWord() { | ||
| List<String> words = List.of("apples", "cherry"); | ||
| assertDoesNotThrow(() -> Word.createInput("cherry", words)); | ||
|
|
||
| } |
There was a problem hiding this comment.
@Test 가 빠져있네요 🤔
해당 테스트를 돌리면 실패하는것 같아요 😢
| @Test | ||
| @DisplayName("입력단어 유효성 검증 실패 테스트") | ||
| void | ||
| validateFailInputWord() { | ||
| List<String> words = List.of("apples", "banana"); | ||
| assertThrows(IllegalArgumentException.class,() -> Word.createInput("pangyo", words)); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("입력단어 글자수 유효성 검증 실패 테스트") | ||
| void validateInputWordLength() { | ||
| List<String> words = List.of("apple", "abcdef"); | ||
| assertThrows(IllegalArgumentException.class,() -> Word.createInput("abcdef", words)); | ||
| } |
There was a problem hiding this comment.
두개의 테스트가 같은 validation 에 걸리는데, 다른 테스트로 분리 되어있어요.
Exception 이 세분화 되어있거나, message 를 검증하면 어떨가요? :)
| @Test | ||
| @DisplayName("입력단어 영단어 유효성 검증 실패 테스트") | ||
| void validateInputWordOnlyEnglish() { | ||
| List<String> words = List.of("apple", "abcd1", "안녕하세요"); | ||
|
|
||
| assertAll( | ||
| () -> assertThrows(IllegalArgumentException.class,() -> Word.createInput("abcd1", words)), | ||
| () -> assertThrows(IllegalArgumentException.class,() -> Word.createInput("안녕하세요", words)) | ||
| ); | ||
| } |
| cigar | ||
| rebut | ||
| sissy | ||
| humph | ||
| awake |
| class WordLoaderTest { | ||
| @Test | ||
| @DisplayName("단어목록파일 읽기") | ||
| void loadWordsFromFile(){ | ||
| List<String> words = WordLoader.read("src/test/resources/words.txt"); | ||
| assertThat(words).hasSize(5); | ||
| } | ||
| } |
There was a problem hiding this comment.
이왕 test 패키지 밑에 words.txt 을 만드셨으니, size 대신 정확한 단어를 검증해보는건 어떨까요? 😃
|
반영한번해봤습니다~ |
src/main/java/domain/Hint.java
Outdated
| public boolean isCorrect() { | ||
| return this == CORRECT; | ||
| } |
| public class Round { | ||
| public final static int ROUND_LIMIT = 6; | ||
| private final int limit; | ||
| private int current; | ||
|
|
||
| public Round() { | ||
| limit = ROUND_LIMIT; | ||
| current = 1; | ||
| } |
There was a problem hiding this comment.
Round 클래스에서 limit 를 받던 것을 제거하셨네요 :)
생성 시 상수로 가지고 있는 ROUND_LIMIT 을 final 변수에 할당하고 사용되고 있는데, 변수로 담아서 쓰는 이유가 명확하지 않다고 느껴져요 🤔
어떤 의도로 변수에 담아서 사용하시는 걸까요?
| public MatchResult match(Word otherWord) { | ||
| boolean[] visited = new boolean[letters.length()]; | ||
| List<Hint> hints = IntStream.range(0, letters.length()) | ||
| .mapToObj(i -> matchLetter(otherWord, visited, i)) | ||
| .toList(); | ||
|
|
||
| return new MatchResult(hints); | ||
| } |
There was a problem hiding this comment.
visited 을 이용한 알고리즘으로 푸셨군요 👍 저희 페어도 첫 구현 당시 이렇게 구현 했었습니다 😄
하지만, 객체지향적이지 못하고, 파라미터의 인자가 가변인 점에서 모양인 점이 너무 아쉽더라구요!
적절한 객체를 새로 만들어서 책임을 넘겨보는건 어떨까요? :)
| private Boolean exists(char letter, boolean[] visited) { | ||
| return IntStream.range(0, letters.length()) | ||
| .filter(i -> !visited[i] && letters.charAt(i) == letter) | ||
| .findFirst() | ||
| .isPresent(); | ||
| } |
There was a problem hiding this comment.
- private Boolean exists(char letter, boolean[] visited) {
+ private boolean exists(char letter, boolean[] visited) {Optional.isPresent() 리턴 타입이 기본형이라 해당 메서드의 리턴 타입도 기본형이어도 될 것 같아요! 😃
| private static void validate(String input) { | ||
| validateLength(input); | ||
| validateOnlyEnglish(input); | ||
| } | ||
|
|
||
| private static void validateOnlyEnglish(String input) { | ||
| if (!input.matches("^[a-zA-Z]+$")) { | ||
| throw new LetterNotEnglishException("영단어를 입력해주세요. [" + input + "]"); | ||
| } | ||
| } | ||
|
|
||
| private static void validateLength(String input) { | ||
| if (input.length() != MAX_LENGTH) { | ||
| throw new OverMaxLengthException(MAX_LENGTH + "자리의 단어를 입력해주세요."); | ||
| } | ||
| } |
| // 와잼님이 짜준 테스트 | ||
| @Test | ||
| void 같은문자_두번_입력시_하나가_존재하면_다른하나는_없다고_나와야한다() { | ||
| Word answerWord = new Word("oxxxx"); // 정답 | ||
| Word inputWord = new Word("zoozz"); // input | ||
|
|
||
| MatchResult results = answerWord.match(inputWord); | ||
|
|
||
| assertThat(results).containsExactly( | ||
| Hint.NOT_EXIST, | ||
| Hint.EXIST, | ||
| Hint.NOT_EXIST, | ||
| Hint.NOT_EXIST, | ||
| Hint.NOT_EXIST); | ||
| } |
| public boolean isWinning() { | ||
| return matchResults.stream().anyMatch(MatchResult::isWinning); | ||
| } |
There was a problem hiding this comment.
matchResults 를 순회하며 전부 CORRECT 인 것을 찾고 있네요!
현재 요구사항이라면, 전부 순회하는 것이 아닌 마지막으로 들어온 MatchResult 가 전부 CORRECT 인것을 찾는게 좀 더 효율적이지 않을까요? :)
| public void add(MatchResult matchResult) { | ||
| matchResults.add(matchResult); | ||
| round.goNext(); | ||
| } | ||
|
|
||
| public boolean shouldContinueGame() { | ||
| return round.isNotFinalRound() && isNotWinning(); | ||
| } |
| System.out.println(e.getMessage()); | ||
| return; | ||
| } | ||
| if (wordDictionary.hasNot(inputWord)) { | ||
| System.out.println("사전에 없는 단어입니다."); | ||
| return; |
There was a problem hiding this comment.
CustomException 들이 단순히 RuntimeException 을 상속받고있어서, 잘못 입력되는 경우 그냥 게임이 종료 되고 있어요! catch 부분이 변경 되어야 할 것 같아요 😢
또한, View 를 따로 분리하셨으니 여기서 출력을 하는것 보단 책임을 넘겨보는건 어떨까요? 👍
| } | ||
|
|
||
| private int getIndex(LocalDate currentDate) { | ||
| LocalDate fixedDate = LocalDate.of(2021, 6, 19); |
There was a problem hiding this comment.
해당 값은 변수명처럼, 고정되어 있는 값인데 상수로 추출해보는건 어떨까요? 😃
진행방식
느낀점
개선점