-
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 과제 제출 - Bob #28
base: main
Are you sure you want to change the base?
Conversation
public static Word fromString(String text) { | ||
validateLength(text); | ||
|
||
AtomicInteger position = new AtomicInteger(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.
AtomicInteger Type을 쓴 이유가 궁금합니다!
밑에 Stream이 parallel 하게 진행되지 않아서 딱히 동시성 이슈가 발생할것 같진 않아서요
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.
Stream 내에서 사실상 불변(effectively final) 값을 사용해야 해서 AtomicInteger를 사용했습니다.
지금 다시 고민해보면 IntStream쪽이 더 깔끔했지 않았나 싶네요
String[] singleStrs = text.split("");
List<PositionLetter> collect = IntStream.range(0, singleStrs.length)
.mapToObj(i -> PositionLetter.of(i, singleStrs[i].charAt(0)))
.collect(Collectors.toList());
Scanner sc = new Scanner(System.in); | ||
return sc.nextLine(); | ||
} | ||
} |
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.
이렇게 되면 Scanner를 항상 재정의 하는 것 같습니다.
static으로 Scanner를 추출&설정 하거나 Singleton pattern으로 해두는것은 어떨까요?
public String toString() { | ||
return 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.
Round도 객체로 Wrapping 한게 좋네요!
toString으로 출력 또한 쉽게 한것 같습니다.bb
GameView gameView = new GameView(); | ||
gameView.initialize(); | ||
|
||
Worker worker = new Worker(transToWords(questions)); |
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.
File을 읽어오는 것과 Worker를 통해 정답 Word를 가져오는걸 메서드로 추출하는 것은 어떨까요?
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줄 규칙 위반이군요! ㅋㅋㅋ 감사합니다
|
||
while (!round.isFinal()) { | ||
gameView.inputAnswer(); | ||
InputReader inputReader = new InputReader(); |
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.
InputReader를 계속 인스턴스화 할 필요는 없을 것 같습니다.
GameRecords 처럼 while 바깥에서 정의해주는건 어떨까요?
|
||
String inputData = inputReader.getUserInput(); | ||
|
||
Word inputWord = Word.fromString(inputData); |
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.
저는 Word.compare에서 Result를 직접 반환해주는걸 좋아하는데, 이렇가 하다보면 도메인간 의존성이 생기긴 하더라구요...
밥님은 이 문제에 대해서 어떻게 생각하시나요?
이렇게 List로 받아 다시 Result로 감싸주어 의존성을 없애는 것도 방법인것 같습니다.
package wordle.domain; | ||
|
||
public class Round { | ||
private static final int MAX_ROUND = 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.
private static final이 생성자를 통한 변수의 수정도 막는 "최후의 불변 선언"이였군요.
배워갑니다 bb
this.results = results; | ||
} | ||
|
||
public List<Result> getResults() { |
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.
unmodifiableList b
assertThat(results).isEqualTo(expected); | ||
} | ||
|
||
public static Stream<Arguments> sample() { |
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.
@MethodSource bb
void readAll() { | ||
FileReader fileReader = new FileReader(); | ||
|
||
List<String> lines = fileReader.readAll("sample.txt"); |
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.
Test용 Sample Data bb
public void printRecords(GameRecords gameRecords) { | ||
System.out.println(); | ||
|
||
gameRecords.getRecords().stream() |
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.
Indent가 과제 조건이랑 벗어난것 같아요.
메서드로 추출은 어떠실까요?
- 가안 작성을 위해, Main 클래스에 주석을 남겨두었습니다. - Word 단위로 알파벳 검증 위해 별도 클래스 분리
- 페어프로그래밍 텀을 30분으로 두어 부득이 위와 같이 feature 단위가 혼재되었습니다. - 도메인을 작게 만들고 관리하고자 전체 라운드를 관장하는 클래스를 별도 분리하였습니다.
- Round 자체가 현재 라운드를 표현해주어야 한다 생각하여 toStrinig을 오버라이드 했습니다. - GameRecord가 적재되어 GameRecords를 만들어간다라는 전제로 GameRecords를 복수형으로 표현했습니다.
- Word/Words 의 도메인 경계가 모호하여 Words-> Word 단수로 사용 및 글자를 Letter로 명시한다. - FileReader는 파일 접근에 대한 책임을 분리하기 위함이다.
- 변경된 명칭 적용 - import 정리 - reformat code
- scanner 사용으로 변경
- 정답과 답안 비교 로직 오류 수정(답안 기준으로 검사 -> 정답 기준으로 변경)과 테스트 코드 작성
안녕하세요! 3주차 과제 제출합니다 :)