-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[5기] 3주차 Wordle 과제 제출 - 와잼 #38
base: main
Are you sure you want to change the base?
Conversation
- 위치에 대한 책임을 가진 클래스
- 알파벳과 위치를 가진 클래스
- List<Letter> 를 가지는 클래스
- Tile, Position 을 가지는 클래스
- isSameAlphabet로 간결하게 변경 - 해당 메서드가 너무 많은 컨텍스트를 알고있음
- Result 일급컬렉션 클래스
- resource 밑에 있는 파일을 읽어온다
- resource 밑에 있는 파일을 읽어온다
- index로 사용되는 용도이므로 변경
- 단어장 책임을 가진 클래스
- 알파벳을 소문자로 저장하는 클래스
# Conflicts: # src/main/java/wordle/infra/FileReader.java # src/test/java/wordle/infra/FileReaderTest.java
- 게임 비정상적 종료 처리 추가
- CurrentResult inner class 도출 및 비교 책임 위임
- resource 밑에 있는 파일을 읽어온다
- 안쓰는 메서드 정리 및 포맷 맞추기
- final 변수 생성 및 메서드 정리
- 용어 사전 수정 및 클래스 다이어그램 수정
…in4943 # Conflicts: # src/main/java/wordle/domain/Word.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와부장님의 리뷰를 하게 될 용자는 누구인가! 라고 생각했는데 저였습니다...
리뷰 쪼렙인지라 너무 큰 부담을 안고 리뷰를 하게 되었어요 🥹
아니나다를까 배울 점이 가득한 너무 멋진 코드였습니다.
리뷰에 너무 따봉👍만 가득 적은 것 같아서 죄송스럽기도 하네요ㅎㅎ...
특히 워들의 메인 로직인 단어 비교 로직에서 정말 많이 고민하신 흔적이 느껴졌습니다!
읽으면서 우와...!를 남발했던 것 같아요.
그리고 테스트에서는 어떤 광기를... 느꼈습니다.
이렇게나 세세하게 테스트를 작성하시고 TDD로 작업하셨다니 정말 대단합니다. 너무 고생 많으셨어요! 👏
정말 많이 부족한 리뷰어지만, 열심히 코드를 분석하고 리뷰를 달아봤습니다.
와잼님 입장에서는 다소 이상한 부분에서 코멘트를 했다고 느끼실 수도 있을 것 같아요ㅜㅜ
제 리뷰 내용에 대해서도 피드백 주실 부분이 있다면 얼마든지 환영입니다!
그럼 답변을 기다리고 있겠습니다 🤗
- 6번 안에 맞추면 게임을 종료한다. | ||
- 6번 안에 맞추지 못하면 그래도 종료한다. | ||
|
||
## 용어 사전 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
용어 사전 너무 좋네요! DDD 그 자체 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DDD 천재 호아님의 가르침! 🙇
|
||
## 모델링 | ||
|
||
### 클래스 다이어그램 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스 다이어그램 세세하게 그려주셔서 이것만 봐도 전체 구조가 다 보이네요! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
천재 호아님의 작품입니닷 😆
|
||
private final OutputView outputView; | ||
|
||
private final Record record; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Record
를 멤버 변수로 두니까 Wordle 내의 전체 메서드들이 이 객체를 참조할 수 있어서
로직을 전개하는 것이 편하네요!
게임 한 번에 하나만 존재해야 하는 객체라 생성 시점에 초기화해서 쭉 사용하는 것이 맞는 방향이라는 생각도 들고요.
왜 저희는 이 방법을 생각 안했나 싶네요 🫢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
게임 한 번에 하나만 존재해야 하는 객체라 생성 시점에 초기화해서 쭉 사용하는 것이 맞는 방향이라는 생각도 들고요.
이런 관점으로도 보일 수 있겠네요! 😮
페어코딩을 할 때 가장 작은 객체부터 만들어 나가며 조립 하다보니 자연스레 도출되었던 클래스 였습니다 😃
결과들을 모아두는 책임을 가진 클래스가 필요했고, 그 클래스는 어떤 책임을 가지면 좋을까 고민하다 보니 Record
가 탄생했습니다! 👶
private void runGame(Word answerWord) { | ||
while (!record.isEnd()) { | ||
outputView.showRecord(record); | ||
handleWrongAnswer(() -> processTurn(answerWord)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleWrongAnswer
라는 Exception handler를 정의하고 거기에 Runnable로 실행할 메서드를 넣는 패턴이 굉장히 신선하고 깔끔해 보입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 자주 사용하는 패턴 중 하나인 템플릿 메서드
패턴의 일종입니닷! 🔥
} | ||
} | ||
|
||
private void processTurn(Word answerWord) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단어를 입력받고 정답과 비교하는 워들 게임의 핵심 로직이 들어있는 메서드로 보이는데요,
processTurn
이라는 메서드명이 다소 직관적이지는 않은 느낌이 드는 것 같아요 🤔
다만 어느 정도는 추측이 가능하고 처음 딱 봤을 때만 어라? 싶었던 거라 개인적인 의견으로 생각해주셔도 좋습니다!
Turn이라는 개념이 여기에서만 등장해서 그런 것 같기도 하네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요. Turn
이라는 단어를 여기에서만 쓰이고, 용어 정리나 다른 곳에 등장한 적이 없네요 😢
개발자들의 영원한 숙제인 이름 짓기에서 나온 문제입니다 ㅎㅎ
초기에는 Round
라는 객체를 도출했었는데 중간에 필요가 없어지면서 제거 했습니다. 그런데 일부 그 아이디어를 차용하면서 생긴 흔적이네요.
Turn
이라는 개념을 추가하거나, 메서드 명에서 지우면 어떨까요? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6732d19 작업완료입니닷 👍
System.out.println(); | ||
} | ||
|
||
private static void showResults(Results results) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 메서드를 private static
으로 선언하신 이유가 궁금합니다!
제가 미처 몰랐던 이점이 있는 걸까요? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분이 위에 잠깐 언급한 Intelij 의 메서드 추출을 사용한 흔적입니다 ㅎㅎ
메서드 추출을 할때, 클래스에 종속되지 않아도 된다 판단하면 static 으로 만들어 주더라구요. 추출 후 지워주는 편인데 깜빡한 부분인 것 같습니닷! 이런 것까지 봐주시다니 🥇 리뷰 메달 드립니다 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import org.mockito.MockedStatic; | ||
import org.mockito.Mockito; | ||
|
||
public abstract class ConsoleIntegrationTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
통합테스트란 이렇게 하는 것이다! 정말 많이 배우고 갑니다 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 제 개인적 욕심이 반영된 부분인데, Console Black box testing 을 한번 해보고 싶어 넣어 봤습니닷! 🔥
import wordle.domain.Record; | ||
import wordle.domain.Results; | ||
|
||
public class RecordFixture { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 전반에서 재사용하기 위한 Fixture 객체 👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
갓 호아님의 아이디어랍니다! 💯
@@ -16,6 +16,7 @@ repositories { | |||
} | |||
|
|||
dependencies { | |||
testImplementation 'org.mockito:mockito-inline:3.12.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트코드를 보고서야 왜 이 의존성을 차마 제거할 수 없었는지 이해했습니다...🥹
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적 욕심인 ConsoleIntegrationTest
를 위한 선택이랍니다 하핫..
throw new AnswerFormulaException(); | ||
} | ||
|
||
return (int) ChronoUnit.DAYS.between(BASE, LocalDate.now()) % wordCount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정말 깔끔한 시간 계산법! 배우고 갑니다 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구월님쪽 계산법도 아주 훌륭한 접근이었습니다! 👍
잼아팀🔥 페어프로그래밍
잘한점
단순히 구현에만 집중하지 않고, 페어와의 의사소통을 통해 즐겁게 진행한 점이 좋았습니다. 호아님도 적극적으로 참여해주셔서 저도 몰입감 있게 진행할 수 있었습니다. 🚀
DDD 세레나데를 수강하신 호아님에게 많은 것을 배울 수 있었습니다. 용어 사전을 정리하고, 클래스 다이어그램을 그리면서 시작했습니다. 이런 방식의 정리는 처음이라 신선했는데, 페어와 용어를 맞추고 그림도 그려가며 진행한 덕분에 페어프로그래밍이 매우 매끄럽게 진행되었습니다. 👍
진행 중 농담도 하며 딱딱하지 않게 진행한 덕분에 서로 의견을 내기 편했다고 생각합니다. 기초가 탄탄한 호아님도 많은 의견을 내주셔서 새로운 관점을 배울 수 있었습니다. 💯
아쉬운점
바쁜 기간이 겹쳐서 약간 정신없이 진행한 것이 제일 아쉽습니다. 😢 개선해 보고 싶은 부분이 몇 군데 있었지만, 호아님과 어떻게 개선할지 얘기만 나누고 코딩은 마무리했습니다.
제가 한 번 몰입하면 주변을 덜 신경 쓰게 되어, 브레이크가 고장 난 트럭처럼 달리는 문제가 있습니다. 페어는 충분히 피곤할 수 있기 때문에 틈틈이 휴식을 취하려 했지만, 호아님이 얘기해주시기 전까지 정신없이 달리고 있더라구요 ㅎㅎ 죄송합니다. 🙇
페어 첫 시작 때 호아님과 얘기해 좀 더 타이트한 제약사항을 가지고 해보기로 했습니다.
1510라인을 넘어가지 않도록 구현한다32이 넘지 않도록 구현한다.하지만 만들다 보니 지키기가 쉽지 않아 완벽히 지키지 못한 점이 아쉽습니다. 😭 또한, 중간에 Mockito 라이브러리를 추가했는데, 나중에 요구사항에
제공된 라이브러리 이외의 외부 라이브러리는 사용하지 않는다.
는 조항이 있다는 것을 알게 되었습니다. 호아님과 얘기해 그냥 두기로 했지만, 요구사항을 제대로 확인하지 못한 부분이 아쉽습니다.배운점
호아님의 스펀지 같은 흡수력과 열린 마인드를 배울 수 있었습니다. 저 또한 호아님의 다양한 관점을 직간접적으로 경험할 수 있었습니다. 🔥
호아님에게 마크업으로 다이어그램을 그리는 Mermaid 를 배웠습니다. 기회가 되면 좀 더 써보고 싶습니다 😃
용어 사전을 정리하고 나니, 동료와 협업할 때 매우 편하다고 느꼈습니다. 지속적인 업데이트가 중요하지만 놓치기 쉬운 부분이라는 것도 알게 되었습니다. 👍
많은 시간을 투자한 부분
페어프로그래밍 도중 가장 많은 시간을 투자한 부분은 토론을 통한 정리였습니다. 첫날은 거의 모든 시간을 정리에 썼습니다. 그 후에도 추가로 나오는 무언가가 있으면 그림을 그리면서 지속적으로 정리해 나갔고, 그 덕분에 페어로 코딩하는 시간은 편안했습니다.
코드에서 가장 많은 시간을 투자한 부분은 글자를 비교하는 부분입니다.
두 개의 동일한 문자를 입력하고 그중 하나가 회색으로 표시되면 해당 문자 중 하나만 최종 단어에 나타난다.
라는 요구사항을 깔끔한 코드로 풀어내기가 어려웠습니다. 풀어내긴 했지만, 썩 마음에 드는 풀이는 아니었습니다. 호아님과 회고를 통해 또 어떤 방식이 있을지 의견을 나눴는데, 기회가 된다면 다시 도전해보고 싶습니다. 🔥페어에게 하고 싶은 말
덕분에 재미있게 진행할 수 있었습니다! 제 억지도 잘 받아주시고, 아재개그도 반응해주셔서 감사합니다 😄 같이 페어할 수 있어서 영광이었습니다! 또 이것저것 알게 되시면 전파 부탁드립니다 🙇 수고하셨어요!