[4기] [3번 과제] Wordle 과제 제출 - MUTO #16

MoonHKLee · 2023-04-02T08:00:26Z

(MUTO, 윤백) 의 페어프로그래밍 과제 제출합니다.

- 디렉토리 경로에서 파일을 읽어와 문자열 list로 반환하는 기능 구현.

- 파일에서 요구사항 로직에 해당하는 단어를 가져오는 기능 구현.

- getWordList(): public -> private

- Answer -> Word

- 테스트 wordPath 전역변수처리 - 이름 직관적으로 변경 - 생성자 필요없는 값 노출 제거

anwjrrp33

안녕하세요. 이번에 무토님 리뷰어를 맡게된 하현입니다 :)
전체적으로 기능 요구사항과 테스트 코드를 꼼꼼하게 작성해주셔서 완성도 있는 소프트웨어가 작성된 거 같습니다! 💯
몇 가지 피드백 드렸는데 저의 개인적인 생각이니 무조건 반영하는 것이 아닌 생각이 다르다면 알려주시면 감사하겠습니다. 🙂

anwjrrp33 · 2023-04-09T09:07:43Z

src/main/java/controller/Game.java

+            if (isCorrect()) {
+                break;
+            }


이번 미션에서는 depth가 3까지 허용이여서 이 부분도 허용되지만 메서드를 분리하는 방식으로 depth가 2를 넘어가지 않는 방식으로 해보시는건 어떨까요?

제가 봤을때 depth2 인것같은데 아닐까요?

아아 제가 잘못 말했네요 depth가 2까지 허용인데 이번 미션에서는 1까지로 해보는건 어떨까요?

anwjrrp33 · 2023-04-09T09:14:57Z

src/main/java/controller/Game.java

+    private void validateInput(String inputString) {
+        inputValidator.validateEnglish(inputString);
+        inputValidator.validateLength(inputString);
+    }


사용자의 input 값을 validate를 통해서 확인 후 Word로 생성해주는 로직인데 이 부분은 관점에 따라서 다를거 같아요 이미 Word 객체와 Letter 객체가 이 역할 대신 해주고 있는데 따로 input에서 확인한 이유가 궁금합니다 🤔

저도 코드를 작성하면서 생각을 했던 부분인데
한군데서 validate을 하는것보다 모든 객체 각각에서 validate을 한번 더 작성해주는게 좋을것 같아서 중복 작성했습니다.
코드가 많아지다보면 중간에 값이 변경되면서 문제가 생길수도 있으니까요.
그런데 생각해보니 final 키워드를 사용해서 불변으로 값을 유지하면 validate 한번으로 충분하겠네요.

수정하다보니 쓸데없는 로직이라는걸 확인했습니다. 바로 삭제하도록 하겠습니다.

anwjrrp33 · 2023-04-09T09:17:29Z

src/main/java/controller/Game.java

+import java.time.LocalDate;
+
+public class Game {
+    private static final String FILE_PATH = "src/main/resources/words.txt";


정답 단어들이 들어있는 파일 경로가 컨트롤러에 상수로 선언되어 있는데 보통 스프링을 쓴다면 yml에 입력하거나 Config 파일로 선언하는 편인데 이 부분은 어떻게 생각하시나요?

PATH등 설정값이 모여있는 Config 클래스를 따로 만들어 관리할까 생각도 했었지만 해당 관련값이 한개밖에 없어서 따로 만들지 않았습니다.

anwjrrp33 · 2023-04-09T09:33:26Z

src/main/java/domain/Answer.java

+    private MatchStatus match(Letter letter) {
+        if (!answerWord.contains(letter)) {
+            return MatchStatus.GRAY;
+        }
+        if (letter.equals(answerWord.get(letter.getPosition()))) {
+            return MatchStatus.GREEN;
+        }
+        return MatchStatus.YELLOW;
+    }


기능 요구사항에 "두 개의 동일한 문자를 입력하고 그중 하나가 회색으로 표시되면 해당 문자 중 하나만 최종 단어에 나타난다." 라는 요구사항이 존재하는데 제가 이해하기론 이미 한번 소모된 단어의 경우 다시 사용하지 않는다로 이해했습니다.
예를들어 오늘 정답 값이 deapt인데 eeapt인 경우 회색 1, 초록 4개로 나와야하는거죠. 현재는 노란 1개 초록 4개로 나오고 있습니다.
제가 생각한 관점과는 조금 다른데 어떻게 생각하시나요?

제가 생각한 관점이 맞다면 Letter에 사용된 position 값이 사라질거 같습니다 :)

position 값은 사라졌는데... 이게 테스트를 통과하는 코드를 만들지를 못하겠네여 ㅠㅠ...

anwjrrp33 · 2023-04-09T09:34:18Z

src/main/java/domain/Word.java

+    // public List<MatchStatus> compare(Word userInputWord) {
+    // }


불필요한 코드가 남아있는거 같아요.

주석은 항상 삭제하고 커밋해야하는데... 감사합니다.

anwjrrp33 · 2023-04-09T11:00:17Z

src/test/java/domain/WordProviderTest.java

+    private static final String NO_WORD_FILE = "src/test/resources/noFile.txt";
+    private static final String TEN_WORDS = "src/test/resources/test10words.txt";
+    private static final String EMPTY_WORDS = "src/test/resources/testEmptyWords.txt";


테스트를 하기 위해서 워드가 들어있는 파일을 분리시킨 점 매우 좋습니다. 👍

anwjrrp33 · 2023-04-09T11:08:30Z

src/main/java/domain/AnswerGenerator.java

+    public String getAnswer(LocalDate currentDate) {
+        Period period = Period.between(criteriaDate, currentDate);
+        int days = period.getDays();
+        return words.get(days % words.size());
+    }


코드의 유연함을 위해서 오늘 날짜를 함수 내부에서 now로 가져오는 것이 아니라 매개변수로 받는거 너무 좋네요. 👏

anwjrrp33 · 2023-04-09T11:12:03Z

src/main/java/domain/InputValidator.java

+    public boolean validateEnglish(String input) {
+        Matcher matcher = ENGLISH_PATTERN.matcher(input);
+        return matcher.matches();
+    }
+
+    public boolean validateLength(String input) {
+        String trim = input.trim();
+        return trim.length() == INPUT_WORD_LENGTH;
+    }


공백을 trim으로 처리해주셨는데 공백 값이 있는 경우엔 이미 validateEnglish 메서드에서 처리가 될거같은데 길이만 체크해줘도 문제 없을거 같습니다.

그러네요 중복코드 생각을 못한것같네요.

anwjrrp33 · 2023-04-09T11:12:56Z

src/main/java/domain/MatchStatus.java

+public enum MatchStatus {
+    GRAY("⬜"),
+    YELLOW("\uD83D\uDFE8"),
+    GREEN("\uD83D\uDFE9");


타일을 enum으로 설계한 부분 좋네요 👍

anwjrrp33 · 2023-04-09T11:14:29Z

src/main/java/domain/Result.java

+
+public class Result {
+
+    private static final int RESULT_SIZE = 5;


해당 상수인 5는 InputValidator.INPUT_WORD_LENGTH에 같은 값으로 상수 선언되어있는데 공통으로 사용할 수 있다면 좋을거 같은데 어떻게 생각하시나요?

제생각엔 Result와 관련된 값이기 때문에 내부에서만 사용되어야한다고 생각하는데 공통으로 사용해야 할 이유가 있을까요?

공통된 상수 5라고 생각했기 때문에 제안드렸었는데 사실 값이 같더라도 사용하는 목적이 다르기 때문에 공통으로 사용되지 않아도 될거 같네요 😊

yunbaek and others added 30 commits March 24, 2023 23:42

docs: 기능 목록 작성

6a551fd

docs: 1회차 페어 프로그래밍 진행 사항 작성

3d5cb7f

feat: add main class

0512172

feat: 게임 시작 메시지 출력

e442ed1

feat: 입력값 영어 검증 기능 추가

8f0fecf

feat: 입력값 길이 검증 추가

4509261

feat: WordProvider

c4c9dd1

- 디렉토리 경로에서 파일을 읽어와 문자열 list로 반환하는 기능 구현.

refactor: 상수 처리

d1e096b

test: 정답 생성 테스트 작성

54a0441

feat(wordProvider): Answer 호출

ea221ae

- 파일에서 요구사항 로직에 해당하는 단어를 가져오는 기능 구현.

refactor(WordProvider): 메서드 접근 제어자 변경

1548059

- getWordList(): public -> private

docs: 요구사항 정리

0b093f8

feat: 글자 값 객체 생성

30b683b

feat: 비교 상태 enum 생성

e06c4dc

feat: Letter 객체 position 속성 추가

41a9179

feat: 결과 상태 반환 로직 추가

66e12bc

feat: 게임 실행 구현

66fc321

docs: 요구사항 정리

102f840

refactor: 글자 일급 컬랙션(Word) 생성

6a0044b

refactor: 포함 여부 확인, 객체 획득 책임 위치 변경

9595720

- Answer -> Word

chore: 깨진 메서드 임시 주석처리

4a355b6

refactor(Answer) : 주석 처리 된 깨진 메서드 수정.

097a70e

refactor(WordProvider) : wordProvider 리팩터링

3056aec

- 테스트 wordPath 전역변수처리 - 이름 직관적으로 변경 - 생성자 필요없는 값 노출 제거

feat(Game) : Game 메인 메서드 생성

0bff183

refactor(Result): 매치 상태 일급 컬랙션 생성

f6cbe52

chore: 실행 클래스 수정

1b160ef

feat(Results): 결과 목록 일급 컬랙션 생성

bb537fc

feat(MatchStatus): 색상 코드 추가

f06ae47

feat(Game): 결과 목록에 추가 하는 기능 구현

0e5c8b3

fix(InputView) : 오타 수정.

b08b65c

MoonHKLee and others added 15 commits April 2, 2023 16:03

feat(Results) : Results 결과 목록 출력 기능 구현.

07a770c

test(Results) : 게임 종료 판별 여부 테스트 추가.

d2e7b7c

feat(Results) : toString override 하여 출력 구현.

1d5ac6c

feat(Results): 정답 포함 여부 반환 기능 추가

b11f84d

feat(Game): 정답 시 라운드 수 출력 기능 구현

d7d4a67

feat(Game) : wordle 게임 기능 구현 완료.

fecc62b

fix(Application) : 기능 요구사항 반영.

3058e52

refactor(AnswerGenerator) : 사용하지 않는 생성자 제거

46dab33

refactor(Game) : Application에 노출된 filepath 은닉

1699c55

refactor(Game) : 사용하지 않는 메서드 삭제

b3f14f8

refactor(Game) : 15line 이상의 메서드 분리

406edef

refactor(MatchStatus) : unicode final화.

4e391b4

feat(Game) : input validate 추가.

f6ab170

docs : todo list 반영

4676d2f

refactor : 전체 코드 reformat.

62354fa

MoonHKLee changed the title ~~[4기] [3번 과제] Wordle 과제 제출 - MUTO , 윤백~~ [4기] [3번 과제] Wordle 과제 제출 - MUTO Apr 2, 2023

anwjrrp33 reviewed Apr 9, 2023

View reviewed changes

refactor : 리뷰 전체 반영

ca3ea3a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4기] [3번 과제] Wordle 과제 제출 - MUTO #16

[4기] [3번 과제] Wordle 과제 제출 - MUTO #16

MoonHKLee commented Apr 2, 2023

anwjrrp33 left a comment

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 11, 2023

anwjrrp33 Apr 15, 2023

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 11, 2023

MoonHKLee Apr 11, 2023

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 11, 2023

anwjrrp33 Apr 9, 2023

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 17, 2023

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 11, 2023

anwjrrp33 Apr 9, 2023

anwjrrp33 Apr 9, 2023

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 11, 2023

anwjrrp33 Apr 9, 2023

anwjrrp33 Apr 9, 2023

MoonHKLee Apr 11, 2023

anwjrrp33 Apr 15, 2023

		// public List<MatchStatus> compare(Word userInputWord) {
		// }


		public class Result {

		private static final int RESULT_SIZE = 5;

[4기] [3번 과제] Wordle 과제 제출 - MUTO #16

Are you sure you want to change the base?

[4기] [3번 과제] Wordle 과제 제출 - MUTO #16

Conversation

MoonHKLee commented Apr 2, 2023

anwjrrp33 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment