-
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
[4기]3주차 Wordle 과제 제출 - 개발인생 #29
base: main
Are you sure you want to change the base?
Conversation
dlwnsgus777
commented
Apr 6, 2023
- 페어 프로그래밍 인원 : 개발인생, Bob, 밈
- 진행 방식
- code with me + 게더
- 느낀 점
- 혼자 개발할 때와는 다르게 머릿속에 있는 내용을 말로 전달하며 코딩하는 게 정말 어렵다는 걸 몸소 체험했습니다.
- 다 같이 목표로 하는 기능에 대해 논의하고 작업하는 데 다양한 의견과 스타일을 접할 수 있어서 유익한 경험이었습니다.
- 회고 및 반성
- 일정 조율에 있어 성실히 임하지 못한 것 같아 깊은 반성 중입니다.... (새벽 0시부터 시작하기로 함 -> 기다리다 잠들어버림.... 등등..)
- 적극적으로 의견을 제시하고 싶었지만 머릿속의 내용들이 정리가 안돼 의견을 제시할 때마다 혼선을 야기한 것 같습니다..
- 가안 작성을 위해, Main 클래스에 주석을 남겨두었습니다. - Word 단위로 알파벳 검증 위해 별도 클래스 분리
- 페어프로그래밍 텀을 30분으로 두어 부득이 위와 같이 feature 단위가 혼재되었습니다. - 도메인을 작게 만들고 관리하고자 전체 라운드를 관장하는 클래스를 별도 분리하였습니다.
- Round 자체가 현재 라운드를 표현해주어야 한다 생각하여 toStrinig을 오버라이드 했습니다. - GameRecord가 적재되어 GameRecords를 만들어간다라는 전제로 GameRecords를 복수형으로 표현했습니다.
- Word/Words 의 도메인 경계가 모호하여 Words-> Word 단수로 사용 및 글자를 Letter로 명시한다. - FileReader는 파일 접근에 대한 책임을 분리하기 위함이다.
- 변경된 명칭 적용 - import 정리 - reformat code
- scanner 사용으로 변경
- 정답과 답안 비교 로직 오류 수정(답안 기준으로 검사 -> 정답 기준으로 변경)과 테스트 코드 작성
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.
개발인생님 안녕하세요!
3차 과제 리뷰어를 맡게 된 윤백입니다 😄
작성해 주신 코드 보면서 저도 많이 배웠습니다! 😆👍
제가 미션할 때에도 신경쓰지 못한 부분들을 코멘트로 남긴다는게 부끄럽네요 😂
혹시 더 논의하고 싶은 부분이 있으시면
언제든지 댓글 혹은 카톡 혹은 데일리 미팅 때 말씀주세요!! 👍👍👍
- 색깔은 세개가 필요하다 | ||
- 여섯 번의 기회를 갖는다. | ||
- 응시자 입력을 통해 답안을 제공받는다. | ||
|
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.
요구사항 관련해서 자세하게 정리해 주신 점 너무 좋네요! 👍
} | ||
|
||
public List<Result> getResults() { | ||
return Collections.unmodifiableList(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.
Unmodifiable collection 사용 👍
|
||
public class GameMachine { | ||
|
||
public void start() { |
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.
메서드의 길이가 15라인을 넘어가지 않도록 구현한다.
라는 요구사항이 있는데 이 메서드는 요구사항을 지키지 못한 것 같습니다!
메서드 추출 등을 통해 길이를 줄여서 요구사항도 충족하고 가독성도 향상시키는 것이 어떨까요? 🤔
} | ||
|
||
public boolean isFinal() { | ||
return this.current >= MAX_ROUND; |
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.
요구사항에선 사용자의 답안 제출을 6번 까지 입력받을 수 있어야 하는데
현재 오답을 5번 입력하면 isFinal
값이 true
로 반환되어 게임이 종료됩니다!
이 부분 확인이 필요해 보입니다! 😃
import java.util.Collections; | ||
import java.util.List; | ||
|
||
public class GameRecords { |
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.
일급 컬렉션 사용 👍
} | ||
|
||
@Override | ||
public boolean equals(Object o) { |
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.
equals 재정의 💯
import java.util.Objects; | ||
|
||
public class Word { | ||
private static final int MAX_LENGTH = 5; |
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.
MAX_LENGTH
라는 변수명은 최대 길이가 5라서
0 ~ 5 혹은 1 ~ 5 까지의 길이를 가질 수 있는 것처럼 보일 수도 있을 것 같아요!
가독성을 위해 VALID_WORD_LENGTH
등으로 변수명을 바꾸는 것에 대해 어떻게 생각하시나요? 🤔
역시 변수명 짓는게 제일 어려운 것 같습니다 😂
int index = 0; | ||
for (char ch : text.toCharArray()) { | ||
letters.add(new Letter(index++, ch)); | ||
} |
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.
letters = IntStream.range(0, text.length())
.mapToObj(i -> new Letter(i, text.charAt(i)))
.collect(Collectors.toList());
변경이 필요한 부분은 아니고 이 부분을 stream을 활용하는 방법으로 바꿀 수 있는지
개인적으로 궁금하여 chatGPT한테 물어보니 나온 코드입니다 😂
Round round = new Round(); | ||
GameRecords gameRecords = new GameRecords(); | ||
|
||
while (!round.isFinal()) { |
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.
class LetterTest { | ||
|
||
@Test | ||
void test() { |
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.
해당 테스트들이 어떤 내용을 검증하려고 하는지
한 눈에 파악이 어려워 보입니다 😂
@DisplayName
혹은 테스트 메서드명을 통해 검증하려고 하는 내용을 명확히 하는 것은 어떨까요 🤔