[자동차 경주] 김주언 제출합니다 - #1482
Conversation
jinu0328
left a comment
jinu0328
left a comment
There was a problem hiding this comment.
전체적으로 함수 분리와 가독성이 좋아서 쉽게 코드를 이해할 수 있었습니다 👍
고생하셨습니다!
| - [x] 전진하는 조건은 0에서 9 사이에서 무작위 값을 구한 후 무작위 값이 4 이상일 경우이다. | ||
| - [x] 자동차 경주 게임을 완료한 후 누가 우승했는지를 알려준다. 우승자는 한 명 이상일 수 있다. | ||
| - [x] 우승자가 여러 명일 경우 쉼표(,)를 이용하여 구분한다. | ||
| - [ ] 사용자가 잘못된 값을 입력할 경우 IllegalArgumentException을 발생시킨 후 애플리케이션은 종료되어야 한다. |
| System.out.println("시도할 횟수는 몇 회인가요?"); | ||
| return Integer.parseInt(Console.readLine()); | ||
| } | ||
| } |
There was a problem hiding this comment.
다른 함수들은 모두 동사형이지만 여기 두 메소드 이름만 명사입니다!
getCarNames()와 getTryCount()로 바꾸는 게 좋을 것 같아요 😃
추가로 문자열로 시도 횟수를 여기서 바로 정수로 parse하여 반환하고 있는데 문자열을 정수로 변환하는 것이 과연 View의 역할일까..? 조심스레 의문을 남기고갑니다
|
|
||
| public void init() { | ||
| String input = inputView.inputCarNames(); | ||
| String[] carNames = input.split(","); |
There was a problem hiding this comment.
Car 모델을 묶는 일급컬렉션 Cars를 만들어주신 것 처럼 String[]을 사용하기보다는 List을 멤버로 가지는 일급 컬렉션 CarNames를 만들어서 사용해주시는 게 좋을 것 같습니다!
There was a problem hiding this comment.
"배열 대신 컬렉션을 사용한다"는 우테코의 1주차 피드백을 보시면 좋을 것 같아요!
| String[] carNames = input.split(","); | ||
| carNameValidator.validate(carNames); | ||
| cars = new Cars(carNames); | ||
| tryCount = inputView.inputTryCount(); |
There was a problem hiding this comment.
다른 View와 Validator는 생성자 주입으로 받아서 사용하는데 Cars만 컨트롤러에서 직접 생성해주는 이유가 있을까요?
| public int inputTryCount() { | ||
| System.out.println("시도할 횟수는 몇 회인가요?"); | ||
| return Integer.parseInt(Console.readLine()); |
There was a problem hiding this comment.
Integer로 바로 형 변환을 하게되면 숫자가 아닌 문자가 입력됐을 때 에러 처리가 힘들거 같아요
lsh1215
left a comment
lsh1215
left a comment
There was a problem hiding this comment.
테스트 코드도 추가해보시면 좋을 것 같습니다. 이번 요구 조건에 기능 단위 테스트 코드에 대한 내용이 있었어요!😁
Kimbobae1
left a comment
Kimbobae1
left a comment
There was a problem hiding this comment.
기능별로 클래스와 메서드가 잘 분리되어 있어서 읽기 좋았습니다!
잘 보고 갑니다 👍👍
|
|
||
| public class RandomNumber { | ||
| public static int getRandomNumber() { | ||
| return Randoms.pickNumberInRange(0, 9); |
There was a problem hiding this comment.
0(최솟값), 9(최댓값) 과 같은 매직 넘버(의미있는 이름의 상수로 대체될 수 있는 숫자)는 상수로 관리하는 것도 좋을 것 같습니다.
주언님은 이부분에 대해 어떻게 생각하시나요?
| public void printWinners(List<String> winners) { | ||
| String message = "최종 우승자 : "; | ||
| if (winners.size() == 1) { | ||
| printSingleWinner(message, winners); | ||
| return; | ||
| } | ||
| printMultipleWinners(message, winners); | ||
|
|
||
| } | ||
|
|
||
| private void printSingleWinner(String message, List<String> winners) { | ||
| System.out.println(message + winners.get(0)); | ||
| } | ||
|
|
||
| private void printMultipleWinners(String message, List<String> winners) { | ||
| System.out.println(message + String.join(", ", winners)); | ||
| } | ||
| } |
There was a problem hiding this comment.
String.join() 메서드는List의 요소가 하나일 때 그 요소만 반환하는 것으로 알고 있습니다.
혹시 SingleWinner와 MultipleWinner를 따로 나누어 놓으신 이유가 따로 있으실까요?
|
|
||
| public String getResult() { | ||
| StringBuilder result = new StringBuilder(name + " : "); | ||
| result.append("-".repeat(Math.max(0, forward))); |
There was a problem hiding this comment.
코드에서 forward를 0으로 초기화하셨는데, Math.max 메서드로 다시 0과 비교하신 이유가 궁금합니다 :)
4 participants
No description provided.