[4기][3주차] Wordle 과제 제출 - 꾸잉 by JuHyun419 · Pull Request #22 · woowahan-pjs/java-wordle

JuHyun419 · 2023-04-02T14:53:01Z

페어: 제이디님
페어 방법: Code With Me + 화면 공유
회고
- 코딩을 하면서 여러 팁들을 배울 수 있어서 좋았습니다.
- 높은 몰입도를 통해 온전히 코딩에 집중할 수 있어서 좋았습니다.
- 페어 중 잦은 커뮤니케이션을 통해 서로의 의견을 조금 더 가깝게 유지할 수 있으면 더 좋을 것 같다는 생각이 들었습니다.

authored-by: 꾸잉

- TODO: print, 파일에서 단어 찾기 로직, input 입력

- print, input 로직 구현 - 파일에서 단어 찾기 로직 구현 중

Co-authored-by: 꾸잉

sang5c

안녕하세요 꾸잉님! 이번에 리뷰를 맡은 bob 입니다..!

저랑 데일리 미팅 시간대가 안맞아서 많이 못 뵀던 것 같은데 대화해볼 수 있는 기회가 생긴 것 같아 좋네요 :)
리뷰가 조금 늦어서.. 좀 더 열심히 진행해 봤습니다. 잘 부탁드립니다~~

리뷰는 RCA 룰을 따라 작성해봤습니다.

r: 꼭 반영해 주세요. 적극적으로 고려해 주세요. (Request changes)
c: 웬만하면 반영해 주세요. (Comment)
a: 반영해도 좋고 넘어가도 좋습니다. 그냥 사소한 의견입니다. (Approve)

sang5c · 2023-04-12T14:05:31Z

src/main/java/controller/WordleController.java

+        wordleGames.start(answer);
+    }
+
+    private String searchAnswer() {


c: searchAnswer 메서드에서 너무 많은 역할을 수행하지 않나 합니다. 파일 읽기 부분을 분리해보면 어떨까요?

좋네요 ㅎㅎ 너무 많은 역할을 하는 듯 합니다.
감사합니다 🙇

sang5c · 2023-04-12T14:08:54Z

src/main/java/controller/WordleController.java

+    private Long calculateAnswerIndex(List<String> strings) {
+        LocalDate today = LocalDate.now();
+        LocalDate date = LocalDate.of(2021, 6, 19);
+        long diffDays = Math.abs(ChronoUnit.DAYS.between(today, date));
+        return diffDays % strings.size();
+    }


c: "몇 번째 인덱스를 추출할 것인가"는 핵심 로직에 속한다고 생각해서 테스트 가능한 형태로 변경해보면 좋을 것 같습니다 :)

sang5c · 2023-04-12T14:12:46Z

src/main/java/controller/WordleController.java

+    private List<String> readFromInputStream(InputStream inputStream) throws IOException {
+        List<String> txtFileLine;
+        try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream))) {
+            txtFileLine = extractTextLines(br);
+        }
+        return txtFileLine;
+    }


a: txtFileLine 변수는 최초 생성시점부터 반환까지 값의 변경에 대한 요구사항이 없습니다. 개인적으로는 생성한 후 변경하여 반환하는 형태보다, 가능한 바로 반환하는 형태가 휴먼 에러를 방지하고 가독성에 도움이 된다고 생각합니다. (언제 값이 변하는지, 언제 반환하는지 추적하지 않아도 됨)

리스트를 생성하고, 리스트에 값을 대입한 다음, 리스트를 반환한다 이 세 가지 행위를 try 블록 안에서 해결하는 것도 고려해주세요!

저도 말씀하신대로 바로 반환하는 형태를 주로 선호하긴 합니다~
감사합니다!

sang5c · 2023-04-12T14:19:49Z

src/main/java/domain/Tile.java

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class Tile {


a: Tile 과 tileColors 의 관계를 말로 설명하면 "타일"은 "다섯 개의 타일 색"을 갖는다고 표현하게되는데 약간 어색하다고 느껴지네요.

sang5c · 2023-04-12T14:20:40Z

src/main/java/domain/TileColor.java

+
+    GREEN("🟩");
+
+    private final String tile;


직전 댓글과 비슷한 의견입니다 :) 이 값은 "타일색.타일" 형태로 사용하게 되고, 표현이 어색한 것 같습니다.

sang5c · 2023-04-12T14:40:11Z

src/main/java/domain/Wordles.java

+    }
+
+    private boolean isSameWordIndex(int i) {
+        return answer.get(i).getWord() == input.get(i).getWord();


c: 디미터의 법칙 위반이라고 생각합니다 🤔

참고: https://tecoble.techcourse.co.kr/post/2020-06-02-law-of-demeter/

sang5c · 2023-04-12T14:43:21Z

src/main/java/domain/WordleGames.java

+
+    private WordleGame wordleGame = new WordleGame();
+
+


a: 개행을 하나 줄일 수 있을 것 같아요!

sang5c · 2023-04-12T14:46:46Z

src/test/java/domain/TilesTest.java

+        Tiles tiles = new Tiles();
+        tiles.addTiles(tile);
+
+        assertThat(tiles.hasAllGreen()).isTrue();


a: TileTest 에서 작성한 테스트에서는 boolean actual = tile.isAllGreen(); 형태로 변수를 통해 비교했는데, 일관성이 있으면 더 좋을 것 같습니다.

페어를 하다보니 서로 스타일이 다른 부분이 있어서 일관성이 떨어지는 듯 하네요,
의견 감사합니다 🙇

sang5c · 2023-04-12T14:48:09Z

src/test/java/domain/WordlesTest.java

+
+        List<TileColor> result = wordles.makeTileColorList();
+
+        assertThat(result).contains(


a: containsExactly() 메서드 사용으로 좀 더 제한적인 테스트가 가능할 것 같습니다!

sang5c · 2023-04-12T14:49:09Z

src/main/java/view/input/InputView.java

+
+    private InputView() { }
+
+    private static final Scanner SCANNER = new Scanner(System.in);


오호.. 스캐너를 상수로 만들어둘 수 있군요 ㅋㅋ

JuHyun419 · 2023-04-16T11:59:37Z

@sang5c 님 정성스러운 리뷰 감사드려요 :)
회사에서는 Pn 룰을 사용을 하고있는데, RCA룰 이라는 방식도 배워갑니다~
c(Comments)로 리뷰를 달아주신 부분은 r(Request changes)로 달아도 무색할만큼 좋은 피드백 인 것 같습니다~
감사합니다 🙇

koola97620 and others added 30 commits March 28, 2023 22:12

docs: 설계

37050a2

feat: Application 생성

d7785d1

feat: Application 생성

f3616e1

feat: WordleGame 생성

3bd7935

feat(ing): WordleGame 구현중

682f05d

feat(ing): WorldGame 구현중

3d7eaab

authored-by: 꾸잉

test 커밋

28febda

test 커밋 롤백

4b711c6

Merge branch 'main' of https://github.com/koola97620/java-wordle

bcffe2e

refactor: WordleController 추가

f05e0e0

authored-by: 꾸잉

feat: WordleComparerTest 추가

55094e4

authored-by: 꾸잉

test(failed): WordleComparerTest 실패하는 테스트 추가

5fd31fa

authored-by: 꾸잉

feat: 작업자 교체 -> 꾸잉

8df9e4e

authored-by: 꾸잉

Merge branch 'main' of https://github.com/koola97620/java-wordle

56cd31a

feat: 사용자 단어와 정답 단어 비교 메서드 추가

af78dea

refactor: 오타 수정

5180d8b

feat: 사용자 단어와 정답 단어 비교 메서드 추가

0604c8d

feat(ing): tile 관련 객체 추가

a883121

feat(ing): Wordle getter 추가

4ea07ba

feat(ing): 컴파일 에러 제거

9d111d6

feat: 비교테스트 진행중 작업자 교체 -> 꾸잉

b417e17

authored-by: 꾸잉

feat(ing): Wordle 체크 로직 구현

5583978

- TODO: print, 파일에서 단어 찾기 로직, input 입력

feat(ing):

b09e81d

- print, input 로직 구현 - 파일에서 단어 찾기 로직 구현 중

feat: words.txt 에서 정답 검색 및 Tile 출력코드 추가

c48d1e4

Co-authored-by: 꾸잉

refactor: 유효성 검증 추가, 요구사항에 맞는 depth 수정, 불필요한 코드 제거

47a7c4f

refactor: 유효성 검증 추가, 요구사항에 맞는 depth 수정, 불필요한 코드 제거

ed0ccb5

refactor: 유효성 검사 추가, 메서드명 변경, 출력로직 변경

78dacf8

Co-authored-by: 꾸잉

refactor: 미사용 코드 제거

2d8703e

Co-authored-by: 꾸잉

test: TilesTest, WordleGameTest 추가

5e29353

Co-authored-by: 꾸잉

test: Tile, Wordle, Wordles 테스트 추가

7d1a14e

Merge branch 'main' of https://github.com/koola97620/java-wordle

6f44dda

sang5c reviewed Apr 12, 2023

View reviewed changes


		List<TileColor> result = wordles.makeTileColorList();

		assertThat(result).contains(


		private InputView() { }

		private static final Scanner SCANNER = new Scanner(System.in);

Conversation

JuHyun419 commented Apr 2, 2023

Uh oh!

sang5c left a comment

Choose a reason for hiding this comment

리뷰는 RCA 룰을 따라 작성해봤습니다.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JuHyun419 commented Apr 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JuHyun419 commented Apr 16, 2023 •

edited

Loading