[자동차경주] 김경수 2주차 미션 제출합니다#8
[자동차경주] 김경수 2주차 미션 제출합니다#8Siul49 wants to merge 8 commits intoJava-JavaScript-Language-Stuty:mainfrom
Conversation
seulnan
left a comment
There was a problem hiding this comment.
너무 고생하셨습니다:)
pr설명대로 변수명에 정말 신경많이쓰셨네요ㅎㅎ
다만, static이나 캡슐화부분에서는 어떤건 적용하셨는데 어떤 부분은 적용안하신 코드도 보입니다!
appconfig파일을 만들어서 DI 흐름을 정리하는 것은 어떨까용??
추가적으로 오류가 난 부분들은 테스트코드를 많이 작성해보면서 충분히 고칠 수 있다고 생각해요.
갠적으로 저는 디버깅하는 과정에서 많은 걸 배우는 편이라 꼭 오류도 고쳐보셨으면 좋겠습니다 파이팅이요 👊🏻👊🏻
| import camp.nextstep.edu.missionutils.Randoms; | ||
|
|
||
| public class Car{ | ||
| public int Distance; |
There was a problem hiding this comment.
distance필드를 public으로 선언한 이유가 있을까요?? 캡슐화에 신경쓰셨다고해서용! @Siul49
There was a problem hiding this comment.
field 변수명의 첫 글자는 소문자가 맞는 것 같습니다.
There was a problem hiding this comment.
field 변수명의 첫 글자가 소문자여야하는 이유가 따로 있는건가요? @BYEONGHWALEE-dev
| StringTokenizer separatedCarNames = new StringTokenizer(CarNames, ","); | ||
|
|
||
| while(separatedCarNames.hasMoreTokens()){ | ||
| if (separatedCarNames.nextToken().trim().length() < 5) { |
There was a problem hiding this comment.
중복코드가 사알짝 보이네용 하필 nextToken이라 두번 호출하면 건너뛰어질 가능성도 있어보여용
nextToken을 한번만 호출한 값을 변수에 저장하고 사용하는 방식으로 수정이 필요해보입니다!
| public void RacingGame() { | ||
| output.CarNameRequestMessage(); | ||
| input userInput = new input(); | ||
| userInput.setCarName(); | ||
|
|
||
| output.trialRequestMessage(); | ||
| userInput.setTrial(); | ||
|
|
||
| setCar(userInput.getCarName()); | ||
|
|
||
| for (int k = 0; k < userInput.getTrial(); k++){ | ||
| for (int i = 0; i < numbersOfCar; i++) { | ||
| int randomDistance = Randoms.pickNumberInRange(0, 9); | ||
| car[i].Distance += randomDistance; | ||
| } | ||
|
|
||
| System.out.print(car[k].getName() + " : "); | ||
| for (int i = 0; i < car[i].Distance; i++){ | ||
| if (i != car[i].Distance){ | ||
| System.out.print("-"); | ||
| } | ||
| else { | ||
| System.out.println("-"); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
여기에 게임진행하는 로직과 출력로직이 같이 섞여있는 것 같아용 util이나 service을 활용해서 분리하는 것은 어떨까용?
|
|
||
| setCar(userInput.getCarName()); | ||
|
|
||
| for (int k = 0; k < userInput.getTrial(); k++){ |
There was a problem hiding this comment.
여기서 이중 for문으로 인해 가독성이 살짝 떨어지는 것 같은데 다른 API를 쓰지않고 이중 for문을 활용하신 이유가 있을까용?
There was a problem hiding this comment.
동의합니다. for문을 한번만 써서 시간 복잡도를 줄일 수 있을 것 같습니다.
There was a problem hiding this comment.
API 공부가 조금 부족했던 것 같네요..ㅜㅜ 혹시 여기서 추천해주실만한 API가 있을까요? @seulnan @BYEONGHWALEE-dev
There was a problem hiding this comment.
음 어떻게 분리하냐에 따라 달라질 것 같긴하데, 다른 분들 코드처럼 forEach()나 Stream으로 충분히 가능해보여요!
| car[i].Distance += randomDistance; | ||
| } | ||
|
|
||
| System.out.print(car[k].getName() + " : "); |
There was a problem hiding this comment.
추가적으로 바깥 for루프에서는 k를 활용해서 시도횟수를 반복하는것 같고,
안쪽 for루프에서는 i라는 인덱스를 활용하여서 각 자동차를 이동하는 것 같은데
자동차를 출력할때는 그러면 car[k]가 아닌 안쪽 for루프에서 car[i]를 써야하는거아닌가요...?
|
|
||
| setCar(userInput.getCarName()); | ||
|
|
||
| for (int k = 0; k < userInput.getTrial(); k++){ |
There was a problem hiding this comment.
동의합니다. for문을 한번만 써서 시간 복잡도를 줄일 수 있을 것 같습니다.
| public void RacingGame() { | ||
| output.CarNameRequestMessage(); | ||
| input userInput = new input(); | ||
| userInput.setCarName(); | ||
|
|
||
| output.trialRequestMessage(); | ||
| userInput.setTrial(); | ||
|
|
||
| setCar(userInput.getCarName()); | ||
|
|
||
| for (int k = 0; k < userInput.getTrial(); k++){ | ||
| for (int i = 0; i < numbersOfCar; i++) { | ||
| int randomDistance = Randoms.pickNumberInRange(0, 9); | ||
| car[i].Distance += randomDistance; | ||
| } | ||
|
|
||
| System.out.print(car[k].getName() + " : "); | ||
| for (int i = 0; i < car[i].Distance; i++){ | ||
| if (i != car[i].Distance){ | ||
| System.out.print("-"); | ||
| } | ||
| else { | ||
| System.out.println("-"); | ||
| } | ||
| } | ||
| } | ||
| } |
| maxDistance = cars.car[i].Distance; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Stream API를 통해 코드를 더 간결하게 짤 수 있을 것 같아요!
| import camp.nextstep.edu.missionutils.Randoms; | ||
|
|
||
| public class Car{ | ||
| public int Distance; |
There was a problem hiding this comment.
field 변수명의 첫 글자는 소문자가 맞는 것 같습니다.
| import java.util.ArrayList; | ||
|
|
||
| public class RacingCarController{ | ||
| public int numbersOfCar = 1; |
| else { | ||
| System.out.println("-"); | ||
| } | ||
| } |
There was a problem hiding this comment.
"-" 출력을 하나로 합치고 마지막에 '\n'을 출력하는 방법은 어떨까요!
There was a problem hiding this comment.
동의합니다! indent depth를 2까지 허용한다고 했는데 지금은 3인 것 같아요!
| System.out.print(winner.getFirst()); | ||
| winner.removeFirst(); | ||
| } | ||
| } |
리뷰하러 와주신 분들 감사합니다!
오늘하루도 행복하세요~
1. 신경써서 구현한 부분
2. 피드백이 필요한 부분
3. 내가 고려한 예외