Conversation
eungyeong12
left a comment
There was a problem hiding this comment.
2주차도 고생 많으셨습니다!
1주차보다 성장한 것이 느껴졌습니다~
| companion object { | ||
| const val INPUT_IS_EMPTY = "입력값이 비어 있습니다." | ||
| private const val NAME_LENGTH_MAX = "자동차 이름이 5자를 초과 했습니다." | ||
| private const val NAME_DUPLICATION = "자동차 이름이 중복 입력 되었습니다." | ||
| private const val NAME_IS_EMPTY = "자동차 이름이 비어 있습니다." | ||
| private const val NAME_CONTAINS_BLANK = "자동차 이름 사이에 공백이 포함되어 있습니다." | ||
| private const val LENGTH_MAX = 5 | ||
| } |
There was a problem hiding this comment.
코틀린 컨벤션에 따르면 companion object는 가장 아래쪽에 작성되어야 할 것 같습니다!
https://effortguy.tistory.com/257
| fun validateNameDuplication(names: List<String>) { | ||
| val nameDuplication = names.toSet() | ||
|
|
||
| if (names.size != nameDuplication.size) { | ||
| throw IllegalArgumentException(NAME_DUPLICATION) | ||
| } | ||
| } |
There was a problem hiding this comment.
이름이 중복되는 경우는 생각하지 못했는데 예외 처리가 꼼꼼하네요!
| fun validateContainsBlank(names: List<String>) { | ||
| if (names.any { it.contains(" ") }) { | ||
| throw IllegalArgumentException(NAME_CONTAINS_BLANK) | ||
| } | ||
| } |
There was a problem hiding this comment.
이름 사이에 공백이 있는 경우 예외 처리를 하신 이유가 궁금해요!
There was a problem hiding this comment.
pobi,woni,jun으로 입력되야하는데 pobi, woni, jun으로 쓸대없는 공백이 입력되는 것을 방지하기 위해 예외에 포함하였습니다!
| class RacingGame( | ||
| private val cars: List<Car>, | ||
| private val count: Int | ||
| ) { | ||
| fun start(carsMover: CarsMover): List<Car> { | ||
| repeat(count) { | ||
| carsMover.moveAll(cars) | ||
| OutputView().printOfRaceResult(cars) | ||
| } | ||
| return cars | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
View와 관련된 로직은 Controller에 있어야 할 것 같아요!
There was a problem hiding this comment.
매 라운드 결과를 어떻게 출력해야 될까 고민을 하다가 해결하지 못한 부분입니다.. ㅠ
방법을 찾아봐야겠네요
There was a problem hiding this comment.
저도 이 부분을 고민했었는데, 매 라운드 결과를 DTO에 저장해 두고 한 번에 출력하는 방식을 사용했어요!
| class FakeRandomNumberGenerator(numbers: List<Int>) : NumberGenerator { | ||
| private val iterator = numbers.iterator() | ||
|
|
||
| override fun getRandomNumber(): Int { | ||
| return iterator.next() | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
저도 이 부분에서 어떻게 테스트를 해야할지 고민이 있었는데 미리 정해둔 순서대로 숫자를 반환하는 클래스를 만든 점 인상 깊네요!
There was a problem hiding this comment.
이 부분 때문에 많이 헤맸어서.. 저도 처음 활용해본거라 앞으로 쓸 일이 많았으면 좋겠네요 ㅎㅎ
| // 레이싱 게임 준비물 | ||
| val numberGenerator = RandomNumberGenerator() | ||
| val movementRule = MovementRule(numberGenerator) | ||
| val cars = parser.parseCarName(carNamesInput) | ||
| val singleMove = SingleCarMover() | ||
| val carsMover = CarsMover(singleMove, movementRule) | ||
| val game = RacingGame(cars, countAttemptInput.toInt()) | ||
|
|
||
| // 게임 진행 | ||
| outputView.printExecutionResultMessage() | ||
| game.start(carsMover) | ||
| val winners = FinalWinner().findWinners(cars) | ||
| outputView.printFinalWinner(winners) |
There was a problem hiding this comment.
게임 진행 로직과 View를 분리하고, 레이싱 준비&진행이 다른 클래스로 분리되면 어떨까 싶습니다!
There was a problem hiding this comment.
run이 많은 책임을 떠안고 있었네요 다음 과제부터는 조금더 세분화 하도록 노력하겠습니다!
CommitTheKermit
left a comment
There was a problem hiding this comment.
코드 잘 읽었습니다. 상세한 테스트 코드가 인상깊었습니다. 한가지, exception 폴더에 validator 들이 있던데 폴더명을 validator로 하는건 어떨까요?
|
|
||
| fun move(car: Car, rule: Boolean) { | ||
| if (rule) { | ||
| car.position++ |
There was a problem hiding this comment.
public으로 지정된 프로퍼티라 하더라도 조회까지만 하는 게 좋고 값 수정은 Car 클래스 내부에서 이뤄지는 게 좋을 거 같아요~
There was a problem hiding this comment.
캡슐화에 신경을 쓰지 못한 것 같습니다.. 지적 감사합니다!!
| fun readCarNameInput(): String { | ||
| return Console.readLine() | ||
| } | ||
|
|
||
| fun readAttemptCountInput(): String { | ||
| return Console.readLine() | ||
| } |
There was a problem hiding this comment.
두 함수의 반환형과 내용이 같아서 하나로 통합해도 좋을 거 같습니다!
There was a problem hiding this comment.
하나의 함수로 같이써도 됐었네요 이제보니 굳이 나눌 필요가 없어보이네요!
There was a problem hiding this comment.
앗 다시 생각해보니 이 2개는 검증 방식이 달라서 각각의 입력함수로 분리 해두는 것이 좋은 것 같습니다.. 제 생각에는..!
| val winners = jointWinners.map { it.name } | ||
|
|
||
| return winners | ||
| } |
|
|
||
| fun move(car: Car, rule: Boolean) { | ||
| if (rule) { | ||
| car.position++ |
There was a problem hiding this comment.
CarsMover와 SingleCarMover클래스를 CarMover안에 같이 쓰신 이유가 궁금합니다!
합치면 별로일까요..?
There was a problem hiding this comment.
지금 생각해보면 SingleCarMover가 불필요한 클래스로 보이네요..
CarsMover 에 합치고 함수만 분리해놓는게 조금더 간결한 것 같습니다!
todays-sun-day
left a comment
There was a problem hiding this comment.
코드 잘 봤습니다!! 공부 많이 하신게 느껴지는 코드였어요!
2주차 고생많으셨습니다. 3주차도 화이팅입니다 :)
There was a problem hiding this comment.
리드미에 작성한 거라, 깃 커밋할 때 docs 를 사용했으면 더 좋았을 것 같아요!
feat 는 정말 새로운 기능을 추가할 때, docs 는 문서 변경을 할 때라서, 리드미는 docs 가 더 가까울 것 같습니다!
There was a problem hiding this comment.
앗.. 실수한 것 같아요 사소한 것 까지 지적해주셔서 감사합니다!
| - [ ] 구분자가 쉼표(,)가 아니면 예외 처리 | ||
| - [ ] 구분자 기준으로 자동차 이름이 6자 이상이면 예외 처리 | ||
| - [ ] 자동차 이름에 공백이 포함되어 있으면 예외 처리 | ||
| - [ ] 자동차 이름이 중복 되면 예외 처리 |
There was a problem hiding this comment.
자동차 이름이 중복인 것은 생각 못 했는데, 이런 경우도 있겠네요 !!
| - [ ] 들여쓰기 2까지만 활용 | ||
| - [ ] 필요한 공백만 활용 | ||
|
|
||
| ### 1주차 공통 피드백 |
There was a problem hiding this comment.
피드백을 꼼꼼하게 정리하고 스스로 점검하시려는 모습이 정말 인상 깊습니다!
다만, README.md 파일은 이 프로젝트를 처음 접하는 사람들을 위한 '프로젝트 설명서' 역할을 하는 것이 주 목적이라고 생각합니다..!
학습 기록을 README보다는 Notion이나 개인 블로그에 정리하시거나, 혹은 레포지토리 내에 my-notes.md 같은 별도 파일로 분리하시는 건 어떨까요?
There was a problem hiding this comment.
클래스가 기능 하나만 갖고 있는데, 특별히 그렇게 하신 이유가 있나요?? RacingGame 에 fun 으로 추가되었어도 좋았을 것 같아요 !
There was a problem hiding this comment.
RacingGame은 오로지 게임 진행만 책임지고, 우승자 계산은 다른 객체를 통해 도출하려고 했습니다!
구현할 기능 목록 정리
입력 기능
랜덤 넘버 기능
자동차 이름 구분
레이싱 기능
출력 기능
예외 처리 기능