[4기][3주차] Wordle 과제 제출 - 꾸잉 #22
Conversation
JuHyun419
commented
Apr 2, 2023
JuHyun419
commented
- 페어: 제이디님
- 페어 방법: Code With Me + 화면 공유
- 회고
- 코딩을 하면서 여러 팁들을 배울 수 있어서 좋았습니다.
- 높은 몰입도를 통해 온전히 코딩에 집중할 수 있어서 좋았습니다.
- 페어 중 잦은 커뮤니케이션을 통해 서로의 의견을 조금 더 가깝게 유지할 수 있으면 더 좋을 것 같다는 생각이 들었습니다.
authored-by: 꾸잉
authored-by: 꾸잉
authored-by: 꾸잉
authored-by: 꾸잉
authored-by: 꾸잉
authored-by: 꾸잉
- TODO: print, 파일에서 단어 찾기 로직, input 입력
- print, input 로직 구현 - 파일에서 단어 찾기 로직 구현 중
Co-authored-by: 꾸잉
Co-authored-by: 꾸잉
Co-authored-by: 꾸잉
Co-authored-by: 꾸잉
There was a problem hiding this comment.
안녕하세요 꾸잉님! 이번에 리뷰를 맡은 bob 입니다..!
저랑 데일리 미팅 시간대가 안맞아서 많이 못 뵀던 것 같은데 대화해볼 수 있는 기회가 생긴 것 같아 좋네요 :)
리뷰가 조금 늦어서.. 좀 더 열심히 진행해 봤습니다. 잘 부탁드립니다~~
리뷰는 RCA 룰을 따라 작성해봤습니다.
r: 꼭 반영해 주세요. 적극적으로 고려해 주세요. (Request changes)
c: 웬만하면 반영해 주세요. (Comment)
a: 반영해도 좋고 넘어가도 좋습니다. 그냥 사소한 의견입니다. (Approve)
| wordleGames.start(answer); | ||
| } | ||
|
|
||
| private String searchAnswer() { |
There was a problem hiding this comment.
c: searchAnswer 메서드에서 너무 많은 역할을 수행하지 않나 합니다. 파일 읽기 부분을 분리해보면 어떨까요?
There was a problem hiding this comment.
좋네요 ㅎㅎ 너무 많은 역할을 하는 듯 합니다.
감사합니다 🙇
| 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(); | ||
| } |
There was a problem hiding this comment.
c: "몇 번째 인덱스를 추출할 것인가"는 핵심 로직에 속한다고 생각해서 테스트 가능한 형태로 변경해보면 좋을 것 같습니다 :)
| private List<String> readFromInputStream(InputStream inputStream) throws IOException { | ||
| List<String> txtFileLine; | ||
| try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream))) { | ||
| txtFileLine = extractTextLines(br); | ||
| } | ||
| return txtFileLine; | ||
| } |
There was a problem hiding this comment.
a: txtFileLine 변수는 최초 생성시점부터 반환까지 값의 변경에 대한 요구사항이 없습니다. 개인적으로는 생성한 후 변경하여 반환하는 형태보다, 가능한 바로 반환하는 형태가 휴먼 에러를 방지하고 가독성에 도움이 된다고 생각합니다. (언제 값이 변하는지, 언제 반환하는지 추적하지 않아도 됨)
리스트를 생성하고, 리스트에 값을 대입한 다음, 리스트를 반환한다 이 세 가지 행위를 try 블록 안에서 해결하는 것도 고려해주세요!
There was a problem hiding this comment.
저도 말씀하신대로 바로 반환하는 형태를 주로 선호하긴 합니다~
감사합니다!
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class Tile { |
There was a problem hiding this comment.
a: Tile 과 tileColors 의 관계를 말로 설명하면 "타일"은 "다섯 개의 타일 색"을 갖는다고 표현하게되는데 약간 어색하다고 느껴지네요.
|
|
||
| GREEN("🟩"); | ||
|
|
||
| private final String tile; |
There was a problem hiding this comment.
직전 댓글과 비슷한 의견입니다 :) 이 값은 "타일색.타일" 형태로 사용하게 되고, 표현이 어색한 것 같습니다.
| } | ||
|
|
||
| private boolean isSameWordIndex(int i) { | ||
| return answer.get(i).getWord() == input.get(i).getWord(); |
There was a problem hiding this comment.
c: 디미터의 법칙 위반이라고 생각합니다 🤔
|
|
||
| private WordleGame wordleGame = new WordleGame(); | ||
|
|
||
|
|
| Tiles tiles = new Tiles(); | ||
| tiles.addTiles(tile); | ||
|
|
||
| assertThat(tiles.hasAllGreen()).isTrue(); |
There was a problem hiding this comment.
a: TileTest 에서 작성한 테스트에서는 boolean actual = tile.isAllGreen(); 형태로 변수를 통해 비교했는데, 일관성이 있으면 더 좋을 것 같습니다.
There was a problem hiding this comment.
페어를 하다보니 서로 스타일이 다른 부분이 있어서 일관성이 떨어지는 듯 하네요,
의견 감사합니다 🙇
|
|
||
| List<TileColor> result = wordles.makeTileColorList(); | ||
|
|
||
| assertThat(result).contains( |
There was a problem hiding this comment.
a: containsExactly() 메서드 사용으로 좀 더 제한적인 테스트가 가능할 것 같습니다!
|
|
||
| private InputView() { } | ||
|
|
||
| private static final Scanner SCANNER = new Scanner(System.in); |
|
@sang5c 님 정성스러운 리뷰 감사드려요 :) |
