[5기]3주차 Wordle 과제 제출 - 싸무엘 #36

samkimuel · 2024-06-17T12:37:54Z

느낀 점

긴 시간동안 페어 프로그래밍을 하면서 상대에 집중하고, 내가 하는 일에도 집중하는 게 중요하다는 것을 느꼈습니다.
나와 다르게 생각하고, 다른 방식으로 개발하는 상대를 이해하면서 소통하는 방법을 조금은 알게 된 것 같습니다.

배운 점

상대를 배려하는 말
테스트 하기 쉬운 코드를 어떻게 작성하는지에 대해 배운 것 같습니다.
안일하게 생각하고 넘어갈 수 있는 부분을 디테일한 관점으로 볼 수 있었습니다.
페어와 이해 정도를 맞추는 커뮤니케이션 과정은 필수적이고 중요하다는 것을 알게 되었습니다.

많은 시간을 투자한 부분

클래스, 메서드 분리
더 좋은 네이밍으로 수정

아쉬운 점

구현 - 리팩토링 - 테스트 사이클
커밋 단위

- 같은 문자가 있는 경우 오동작

yeojiin

싸무엘님 반갑습니다~ 페어 프로그래밍에 이어 리뷰까지 하게 되었는데 제가 코드 리뷰 경험이 많지 않아 부족한 점이 많아요....🥲
리뷰를 통해 많이 배워가고 제 코드에 적용해보고 싶은 부분도 많이 배웠습니다!
특히, 여러 케이스를 검증한 테스트 코드 부분이 흥미로웠고, Map을 사용하셔서 정답을 찾아가는 과정이 인상 깊었습니다! 같은 과제를 다르게 풀이한 걸 보고 아주 재밌게 리뷰했습니다! 😁
중간 중간 달았던 댓글 중 자유롭게 의견을 듣고 싶어서 남긴 부분도 있으니 다음 리뷰 때 싸무엘님의 견해를 듣고 싶네요~

이번 과제하시느라 고생 많으셨습니다~~~ 👏👏👏👏

docs/회고.md

src/main/java/wordle/model/WordsReader.java

yeojiin · 2024-06-22T08:48:11Z

src/main/java/wordle/model/WordService.java

+import java.time.LocalDate;
+import java.util.List;
+
+public class WordService {


wordService라는 명칭 대신 단어를 로드하는 기능을 하고 있으니 WordLoader와 같은 명칭은 어떨까요?😄
객체가 어떤 기능을 하는지 좀 더 가시적으로 판단이 가능할 거 같습니다!

좋은 의견주셔서 감사합니다 👍🏻 👍🏻
네이밍에 대한 고민을 하다가 일단 Service 라고 해놓고 나중에 바꾸자고 넘어갔었는데 더 명확하게 표현하는 게 좋겠네요.

src/main/java/wordle/model/Words.java

src/main/java/wordle/model/Letters.java

yeojiin · 2024-06-24T07:57:50Z

src/main/java/wordle/model/LetterCounter.java

+import java.util.List;
+import java.util.Map;
+
+public class LetterCounter {


메소드 배치하실 때 어떤 기준으로 하시는 편인지 궁금합니다!
저도 피드백 받은 부분인데 상수 -> 일반 필드 -> 기본 생성자 -> 인자가 있는 생성자 -> public 메소드 -> private 메소드 -> 오버라이딩한 메소드 순으로 배치하는 것을 추천해 주시더라구요.
이번 기회에 같이 전체적으로 적용해보면 좋을 거 같아요~~ 🙈

public 메서드, private 메서드는 순서상관없이 연관성 있는 메서드끼리 묶어놓는 편이고, 다른 순서는 동일해요!
public 메서드와 참조하는 private 메서드의 거리가 멀 때 확인이 불편해서 최대한 가깝게 놓는 편이에요.

src/test/java/wordle/LettersTest.java

yeojiin · 2024-06-24T11:33:18Z

src/main/java/wordle/model/Tiles.java

+    public void addGreenTile(Letters letters) {
+        for(Letter letter : letters.getLetters()) {
+            tiles[letter.getPosition()] = GREEN_TILE;
+        }
+    }
+
+    public void addYellowTile(Letters letters) {
+        for(Letter letter : letters.getLetters()) {
+            tiles[letter.getPosition()] = YELLOW_TILE;
+        }
+    }
+
+    public void addGrayTile(Letters letters) {
+        for (Letter letter : letters.getLetters()) {
+            tiles[letter.getPosition()] = GRAY_TILE;
+        }
+    }


이건 개인적인 궁금증인데 싸무엘님은 이렇게 로직이 중복되는 코드가 있으면 분리해서 작성하는 걸 선호하시나요?
아니면 addTile(letters, tiletype) 이런 식으로 하나의 메소드를 변수를 넘겨 구분하는 걸 선호하시는 편인지 궁금합니다!
실무에서도 이 고민을 많이 해서 여쭤보아요~ 😁

클라이언트(TileService)에서 Tile에 대해 꼭 알 필요는 없다고 생각해서 이렇게 구현했던 것 같네요 ㅎㅎ..
저는 어떤 방식을 선호한다기보다 클라이언트에서 해당 클래스에 대해 알 필요가 있는가를 기준으로 작성하는 것 같아요!
여기서는 tileType을 받아도 괜찮을 것 같아 보이긴 하는데 그렇게 되면 if, switch, enum 등을 사용해서 어떻게 맛있게 처리할까 생각해야겠네요 ㅋㅋㅋ

src/test/java/wordle/TileServiceTest.java

docs/회고.md

samkimuel

얄뭉님 리뷰 답변이 늦었네요 죄송해요ㅠㅠ
세심한 리뷰 너무 감사해요. 다시 한 번 생각하고 고민해보면서 많이 배울 수 있었어요 🙇🏻
리뷰 반영한 부분이나 제 답변에서 모호한 게 있으면 언제든 물어봐주세요! 😄
당신은 최고의 리뷰어 💯

samkimuel · 2024-06-28T16:23:12Z

src/main/java/wordle/Application.java

+public class Application {
+
+    public static void main(String[] args) {
+        Wordle wordle = new Wordle(new Console(), new WordService(new WordsReader()), new WordleValidator(), new TileService(new TileStorage()));


말씀해주신 장점을 생각해 주입 패턴을 사용했는데, 현재 Wordle 에서는 동적으로 바뀌지 않아 Wordle 클래스 내부에서 생성 책임을 가져가도 괜찮을 것 같기도 하네요 🤔
생각해보니 동적으로 바뀌어야할 상황을 가정한다면 추상화를 하는게 좋아보이네요,, 당연하게 생각하고 넘어갔었던 부분인데 상기 시켜주셔서 감사합니다!

samkimuel · 2024-06-28T16:58:42Z

src/main/java/wordle/model/LetterCounter.java

+import java.util.List;
+import java.util.Map;
+
+public class LetterCounter {


public 메서드, private 메서드는 순서상관없이 연관성 있는 메서드끼리 묶어놓는 편이고, 다른 순서는 동일해요!
public 메서드와 참조하는 private 메서드의 거리가 멀 때 확인이 불편해서 최대한 가깝게 놓는 편이에요.

samkimuel · 2024-06-28T17:03:00Z

src/main/java/wordle/model/LetterCounter.java

+
+public class LetterCounter {
+
+    private final Map<Character, Integer> letterCountMap;


철자와 철자 개수를 각각 Map의 key, value로 만들었어요!

samkimuel · 2024-06-28T17:07:18Z

src/main/java/wordle/model/Letters.java

+    private final List<Letter> letters;
+
+    public Letters(String word) {
+        String lowerCase = word.toLowerCase();


대소문자 비교때문에 소문자로 변환해야겠다고 생각해서 추가했는데, 얄뭉님은 대소문자 구분을 어떻게 하셨을까요?

samkimuel · 2024-06-28T17:10:17Z

src/main/java/wordle/model/Letters.java

+    public String combine() {
+        return letters.stream()
+            .map(letter -> String.valueOf(letter.getValue()))
+            .collect(Collectors.joining(""));
+    }


저는 joining을 처음 써봤는데 이번에 딱구님에게 배웠습니다 ㅎㅎ 🚌

src/test/java/wordle/TileServiceTest.java

samkimuel · 2024-06-30T03:34:52Z

src/main/java/wordle/model/Tiles.java

+    public void addGreenTile(Letters letters) {
+        for(Letter letter : letters.getLetters()) {
+            tiles[letter.getPosition()] = GREEN_TILE;
+        }
+    }
+
+    public void addYellowTile(Letters letters) {
+        for(Letter letter : letters.getLetters()) {
+            tiles[letter.getPosition()] = YELLOW_TILE;
+        }
+    }
+
+    public void addGrayTile(Letters letters) {
+        for (Letter letter : letters.getLetters()) {
+            tiles[letter.getPosition()] = GRAY_TILE;
+        }
+    }


클라이언트(TileService)에서 Tile에 대해 꼭 알 필요는 없다고 생각해서 이렇게 구현했던 것 같네요 ㅎㅎ..
저는 어떤 방식을 선호한다기보다 클라이언트에서 해당 클래스에 대해 알 필요가 있는가를 기준으로 작성하는 것 같아요!
여기서는 tileType을 받아도 괜찮을 것 같아 보이긴 하는데 그렇게 되면 if, switch, enum 등을 사용해서 어떻게 맛있게 처리할까 생각해야겠네요 ㅋㅋㅋ

samkimuel · 2024-06-30T08:21:25Z

src/main/java/wordle/model/Wordle.java

+            inputLetters = new Letters(input);
+
+            if (isInputLettersInvalid(words, inputLetters)) {


입력 String 을 Letters 변환해서 해당 클래스를 통해 검증을 진행하려고 했다가 validator에 검증 역할을 위임하면서 불필요한 프로세스가 늘어나게 되어버렸네요,,, 😢
놓치고 있던 부분인데 세심하게 봐주셔서 감사합니다! 최고최고 👍🏻

samkimuel · 2024-06-30T08:37:58Z

src/main/java/wordle/model/Wordle.java

+    private final Console console;
+    private final WordService wordService;
+    private final WordleValidator wordleValidator;
+    private final TileService tileService;


생성 시점에 초기화한다면 말씀해주신 부분에서 확실히 장점이 있네요. 캐시처럼 사용할수도 있구요!
Word -> Letters 변환 책임은 WordService의 역할이 맞을까 고민했었는데 좋은 방법인 것 같습니다 👍🏻

samkimuel · 2024-06-30T08:44:17Z

src/main/java/wordle/model/Wordle.java

+        Words words = wordService.getWords();
+        Letters answerLetters = createAnswerLetters(words);


마감 이슈로 일단 메서드 분리만 해보자해서 내부 메서드로 추출하고 개선은 못했는데 위에서 의견주신 것과 동일하게 위임하면 좋을 것 같아요 ㅎㅎ 좋은 의견 감사합니다 👍🏻

samkim-jn and others added 30 commits June 6, 2024 11:06

docs: 기능 구현 task 목록 작성

8ab95e6

docs: 1일차 회고 작성

5c69323

docs: 2일차 회고 작성

0517eb1

feat: 초기 문구 출력과 정답 입력 구현

334e123

feat: 파일 읽기 로직 구현

bbc2280

feat: 주어진 날짜에 대한 단어를 얻는 로직 구현

61fae53

docs: 3일차 회고 작성

4061d64

feat: 초기 구현

742ebd5

refactor: Answer, Result 클래스 추출

005e447

refactor: Letter 클래스 분리

7d2e79f

docs: 4일차 회고 작성

eb5ee89

docs: 싸무엘 4일차 회고 작성

d960b50

refactor: Input 클래스 분리

883e63a

refactor: WordleValidator 분리

318426a

refactor: checkContainsValue 메서드가 Input을 사용하도록 변경

b300bc1

refactor: Letters, LetterCounter 분리

bd4c2cb

refactor: 초록색 타일 만드는 로직 리팩토링

22eecf7

docs: 5일차 회고 작성(딱구)

e51a0a8

docs: 5일차 회고 작성(싸무엘)

36173d7

refactor: 노란색 타일 만드는 로직 리팩토링

84f1a55

refactor: 회색 타일 로직 리팩토링

15baf55

refactor: 회색 타일로 변환하는 케이스를 추가

b158817

fix: 초록색 타일이 회색 타일로 출력되는 문제 수정

07007a6

- 같은 문자가 있는 경우 오동작

refactor: Result를 Tiles로 변경하고 게임의 정책을 상수로 추출

aaf2b6e

refactor: TileService로 Tiles 생성 로직을 이동

7ae7c87

refactor: TileStorage 클래스 분리

47a549c

refactor: 사용하지 않는 클래스 제거 & 의존성을 주입받도록 변경

7d091de

refactor: 정답 타일 체크로직 리팩토링

d3ad606

test: LetterCounter 테스트 작성

fbe067c

test: Letters 테스트 작성

3860bac

samkim-jn and others added 19 commits June 17, 2024 19:08

docs: 6일차 회고 작성(싸무엘)

46d72cd

refactor: 결과 출력 부분 메서드 분리

08fc345

refactor: 입력 받는 부분 메서드 분리

4fc87cf

refactor: IOView 클래스 이름을 Console로 변경 및 메서드 이름 변경

1aae120

refactor: 결과 출력 부분 Console 클래스로 이동

cffb677

refactor: WordService 분리

438f7b0

refactor: TileService create() 메서드 분리

cdaea0b

refactor: AnswerLetters를 만드는 로직을 메서드로 분리

b3d5b05

refactor: 게임 종료 여부를 체크하는 로직을 분리

2273c00

refactor: 입력값 검증 메서드 분리

f6f55e1

refactor: Letters를 생성할 때 파라미터를 소문자로 변경

0132097

refactor: 메서드 순서 변경

e071b7f

refactor: 패키지 분리

bbaa42c

style: 코드 스타일 수정

d62deea

refactor: WORDS_FILE_PATH 상수 추출

02d8987

test: 테스트 코드 리팩토링

bf05504

docs: task 정리

4b15342

docs: 7일차 회고 작성(딱구)

4f208b2

docs: 7일차 회고 작성(싸무엘)

518e295

yeojiin reviewed Jun 24, 2024

View reviewed changes

samkimuel added 9 commits June 28, 2024 23:55

refactor: 기능을 명확하게 표현하기 위한 클래스명 변경

3ed7bc0

refactor: 날짜 간격을 Period 클래스를 이용하여 수정

d1dffe5

refactor: 값만 일치하는지 판단하는 로직을 Letter 클래스 책임으로 분리

dca9adb

refactor: 테스트 픽스처 메서드로 분리

c5bef02

docs: 요구사항 수정

4f35990

refactor: 입력 검증 로직 개선

1d3cbef

refactor: Word 관련 로직을 WordLoader 책임으로 분리

e3b2d12

refactor: 미사용 메서드 제거

28ea8ec

refactor: Tiles, TileService 클래스를 표현력있는 클래스로 변경

f73a128

samkimuel commented Jun 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[5기]3주차 Wordle 과제 제출 - 싸무엘 #36

[5기]3주차 Wordle 과제 제출 - 싸무엘 #36

samkimuel commented Jun 17, 2024

yeojiin left a comment •

edited

Loading

yeojiin Jun 22, 2024

samkimuel Jun 29, 2024

yeojiin Jun 24, 2024

samkimuel Jun 28, 2024

yeojiin Jun 24, 2024

samkimuel Jun 30, 2024

samkimuel left a comment

samkimuel Jun 28, 2024

samkimuel Jun 28, 2024

samkimuel Jun 28, 2024

samkimuel Jun 28, 2024

samkimuel Jun 28, 2024

samkimuel Jun 30, 2024

samkimuel Jun 30, 2024

samkimuel Jun 30, 2024

samkimuel Jun 30, 2024


		public class LetterCounter {

		private final Map<Character, Integer> letterCountMap;

		inputLetters = new Letters(input);

		if (isInputLettersInvalid(words, inputLetters)) {

		Words words = wordService.getWords();
		Letters answerLetters = createAnswerLetters(words);

[5기]3주차 Wordle 과제 제출 - 싸무엘 #36

Are you sure you want to change the base?

[5기]3주차 Wordle 과제 제출 - 싸무엘 #36

Conversation

samkimuel commented Jun 17, 2024

느낀 점

배운 점

많은 시간을 투자한 부분

아쉬운 점

yeojiin left a comment • edited Loading

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

samkimuel 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

yeojiin left a comment •

edited

Loading