Conversation
| feat (feature) | ||
| fix (bug fix) | ||
| docs (documentation) | ||
| style (formatting, missing semi colons, …) | ||
| refactor | ||
| test (when adding missing tests) | ||
| chore (maintain) |
There was a problem hiding this comment.
AngularJS의 커밋 컨벤션을 찾아보셨네요 👍
하나의 커밋에 모든 작업을 하기보다는 작은 작업 단위의 커밋으로 나누어서 구현해보면 좋을 것 같습니다 :)
|
|
||
| } |
There was a problem hiding this comment.
파일 마지막에 엔터(개행문자)를 넣어주세요 :)
이유는 리뷰를 진행할 때 깃허브에서 경고메시지를 지우고 혹시 모르는 파일 읽기 오류에 대비하기 위함입니다.
좀 더 알고싶으시면 참고 링크를 보셔도 재밌을 것 같네요 :)
Intellij 를 사용하실 경우엔Preferences -> Editor -> General -> Ensure line feed at file end on save 를 체크해주시면파일 저장 시 마지막에 개행문자를 자동으로 넣어줍니다!
https://minz.dev/19https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline
|
|
||
|
|
||
|
|
| @@ -0,0 +1,62 @@ | |||
| import java.util.*; | |||
|
|
|||
| public class ReadyGame { | |||
There was a problem hiding this comment.
ReadyGame 클래스의 인스턴스는 어떤 상태(내부 필드)도 가지지 않기 때문에 이 클래스의 모든 객체들은 동일한 객체이겠네요 !
너무 많은 상태를 가지는 것은 좋은 설계가 아니지만, 아무것도 가지지 않는 방식 또한 바람직한 설계는 아닙니다.
적절한 상태를 가진 객체를 설계해보면 좋을 것 같아요.
다른 클래스들에도 함께 적용되는 피드백일 것 같습니다 :)
| String userNumber; | ||
| Scanner scanner; | ||
|
|
||
| scanner = new Scanner(System.in); | ||
| userNumber = scanner.nextLine(); |
There was a problem hiding this comment.
지역 변수의 초기화는 아래와 같은 방식으로 해주시면 좋을 것 같아요 :)
| String userNumber; | |
| Scanner scanner; | |
| scanner = new Scanner(System.in); | |
| userNumber = scanner.nextLine(); | |
| String scanner = new Scanner(System.in); | |
| Scanner userNumber = scanner.nextLine(); |
| } | ||
|
|
||
| for (Integer number : userNumberList) { | ||
| if (!(0 < number && number < 10)) { |
There was a problem hiding this comment.
indent(인덴트, 들여쓰기) depth를 2가 넘지 않도록 구현한다. 1까지만 허용한다.
위 프로그래밍의 요구사항을 만족하도록 개선해보면 좋을 것 같아요.
depth를 줄이는 가장 쉬운 방법은 메서드로 분리하는 것입니다.
| @Test | ||
| void 동일한_숫자_갯수_확인() throws Exception { | ||
| // given | ||
| // 숫자 list 2개가 주어진다 |
There was a problem hiding this comment.
테스트를 given when then 패턴으로 잘 짜주셨네요!
다만 잘 짜여진 테스트에서 주석은 필요하지 않을 것 같습니다.
|
|
||
| // given | ||
| // '동일한 숫자의 갯수'와 '위치가 동일한 숫자의 갯수'가 주어진다. | ||
|
|
|
|
||
|
|
||
| // 1. 게임 준비 | ||
| System.out.println("숫자 야구를 시작합니다."); |
There was a problem hiding this comment.
- 핵심 로직을 구현하는 코드와 UI를 담당하는 로직을 구분한다.
- UI 로직을 InputView, ResultView와 같은 클래스를 추가해 분리한다.
프로그래밍 요구사항 중에 위와 같은 요구사항이 있었는데요. 이 부분을 어떻게 구현할지 생각해보시면 좋을 것 같습니다.
| ## 1차 구현 - 기능 목록 메모 | ||
| 0. 1차 구현 과정 정리: https://velog.io/@woply/야구-게임-TDD-리팩토링-1차-구현 | ||
|
|
||
| 1. 게임 준비하기: ReadyGame |
There was a problem hiding this comment.
전체적으로 클래스를 분리하셔서 깔끔하게 구현해주셨는데요 👍
아쉬운 점이 있다면 절차지향적인 코드가 주를 이루고 있다는 것입니다.
절차지향적인 코드로 작성되다보니 테스트 또한 객체 단위의 단위테스트 보다는 기능 테스트로 작성된 것 같아요!
적절한 객체를 도출해보는 방식으로 다시 구현해보면 좋은 인사이트가 있을 것 같아요.
|
바쁘신 와중에도 상세한 코멘트 남겨주셔서 감사합니다. 피드백 주신 내용 적용하여 리팩토링 해보겠습니다. 감사합니다! |
2 participants
안녕하세요 요한님.
리뷰 도와주셔서 감사합니다!
피드백 내용 열심히 공부하고 리팩토링 해보겠습니다.