[자동차 경주] 김성아 미션 제출합니다. #81
Conversation
There was a problem hiding this comment.
- 질문은 어떻게 오류가 났는지 잘 상상이 안되네요. import 경로의 문제이지 않았나 싶습니다. 질문이 있다면 누군가에게 물어봤다면 좋았을것같습니다. 질문이란것은 성장에 있어 좋은 습관입니다.
- stream이 아니더라도. 반복문 한번으로도 해결이 가능합니다. 반복문 안에서 최대값을 갱신, 최댓값과 동일할때 리스트에 추가하는 함수처럼요.
이번주차 고생하셨습니다. 살짝 부족한 부분이 좀 보이지만, 오히려 저는 직접 코드를 작성한 티가 나서 좋았습니다.(살짝 알고리즘 문제 풀이 코드보는 느낌). 이 시기에 가장 실력을 빨리 올리는법은 남의 코드를 많이 보는 것입니다. 남의 코드를 그대로 작성해보면서 이해하려하면 좋더라고요 경험담입니다. 어쨌든 앞으로 더 멋진 성장을 이뤄내길 바라겠습니다.
| return 0; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
| if (name.isEmpty()){ | ||
| throw new IllegalArgumentException(); | ||
| } |
There was a problem hiding this comment.
null일 경우도 고려하면 좋을것같고, 에러메세지도 있으면 좋겠네요
함수로 모듈화하는것도 좋아보여요. 다른 예외처리도 동일합니다.
There was a problem hiding this comment.
name이 null일 경우엔 isEmpty에 접근하다가 에러가 발생할 것이기 때문에 따로 if로 확인하지 않았습니다. 그럼에도 처리를 하는 것이 좋을까요?
There was a problem hiding this comment.
isEmpty() 단에서 null이라서 NPE를 발생시킬텐데, 캐치할 수 있는 에러는 캐치하는 것이 디버깅 측면에서 좋습니다.
There was a problem hiding this comment.
참고로 리뷰에 코멘트을 하나하나 달 경우 메일 테러가 발생합니다. 저도 처음 협업할때 메일 테러를 했던 기억이 나네요 ㅋㅋㅋ
리뷰를 하는것처럼 해당 pr에서 FileChanged을 누르고, 각각의 리뷰에 코멘트를 작성후 Add review comment(임시 저장)를 합니다. 그러고 오른쪽 위에 Fininsh your review를 하면 코멘트를 한번에 보낼수 있습니다.
| private static void carNameInput(){ | ||
| Scanner in = new Scanner(System.in); | ||
|
|
||
| // 자동차 이름 | ||
| System.out.println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분)."); | ||
| String[] carNames = in.nextLine().split(","); | ||
|
|
||
| for (String name : carNames){ | ||
| if (name.length() > 5 || name.isEmpty()){ | ||
| throw new IllegalArgumentException(); | ||
| } | ||
| } | ||
|
|
||
| // 객체 생성 | ||
| cars = Arrays.stream(carNames) | ||
| .map(Car::new) | ||
| .toArray(Car[]::new); | ||
|
|
||
| in.close(); | ||
| } | ||
|
|
||
| private static void timesInput(){ | ||
| Scanner in = new Scanner(System.in); | ||
|
|
||
| // 이동 횟수 | ||
| System.out.println("시도할 회수는 몇회인가요?"); | ||
| times = in.nextInt(); | ||
| if (times < 0){ | ||
| throw new IllegalArgumentException(); | ||
| } | ||
|
|
||
| in.close(); | ||
| } |
| import java.util.stream.IntStream; | ||
| import java.util.Arrays; | ||
|
|
||
| public class FindWinCar { |
There was a problem hiding this comment.
이런 클래스는 좋지 않아보입니다.
FindWinCar 클래스가 단순히 승자를 찾는 역할만 하는 것이 아니라, 자동차의 움직임까지 관리하고 있는 것은 SRP에 어긋나는것 같습니다. 한번 더 고민을 해보시면 좋을것같아요
| throw new IllegalArgumentException(); | ||
| } | ||
|
|
||
| this.cars = Objects.requireNonNull(car); |
There was a problem hiding this comment.
자바 수업시간에 null을 체크하는 if를 쓰는 것보다 requireNonNull을 사용하는 것이 간결하다고 배웠던 기억이 있어 사용하였습니다
There was a problem hiding this comment.
(비교적) 최신 기능들을 사용해보는 것은 좋은 습관이예요. 물론 원리에 대한 이해가 있다는 가정하에
좋습니다 👍 👍
| @@ -0,0 +1,29 @@ | |||
| package objectTmp; | |||
There was a problem hiding this comment.
<궁금했던 점>에 달았던 문제가 이 문제입니다..! 저렇게 임시 패키지를 만들어 import를 하면 Car를 사용할 수 있지만, 그렇지 않은 경우에는 Car cannot be resolved to a type라는 에러가 발생하여 정의한 Car를 사용할 수 없었습니다. 그래서 임시방편이나마 임시 패키지를 만들어 import하는 방식으로 제출하였습니다.
There was a problem hiding this comment.
default package(main/java)에 있는 클래스는 다른 패키지에서 호출할수 없기 때문입니다. domain같은 패키지에 두는것이 좋을 것 같습니다. 😄
| import domain.RacingGame; | ||
| import objectTmp.Car; | ||
|
|
||
| public class ResultView { |
There was a problem hiding this comment.
다른 클래스에 있는 모든 출력이 여기에서만 관리되었으면 좋겠네요
| @Test | ||
| public void createTest(){ | ||
| Car c = new Car("mercedes"); | ||
| assertEquals("mercedes", c.getName()); | ||
| } | ||
|
|
||
| @Test | ||
| public void errorTest(){ | ||
| assertThrows(NullPointerException.class, ()->new Car(null)); | ||
| assertThrows(IllegalArgumentException.class, ()->new Car("")); | ||
| } | ||
|
|
||
| @Test | ||
| public void moveTest(){ | ||
| Car c = new Car("a"); | ||
|
|
||
| int tmp; | ||
| for (int i = 0; i < 100; i++) { | ||
| tmp = c.move(); | ||
| assertTrue(tmp == 1 || tmp == 0); | ||
| } | ||
| } |
| fwc.allMove(10); | ||
| Car[] winCar = fwc.findWin(); | ||
|
|
||
| assertTrue(winCar.length > 0); |
There was a problem hiding this comment.
일단 미션에서 마지막 리팩터링 참고를 보았을때, 랜덤을 어떻게 테스트할지에 대한 고민이 필요한데, 일단 랜덤에 대한 테스트가 없습니다.
또한 길이를 체크하는 것이 아닌, 구체적인 결과 값으로 검증이 되면 좋을 것 같습니다.
| for (int i = 0; i < 5; ++i){ | ||
| assertTrue(locate[i]>=0); | ||
| } |
There was a problem hiding this comment.
랜덤이 동작하지 않아 모두 안움직인다면 다 0으로 나와서 문제가 될수도있지 않을까요?
<배운점>
소프트웨어 공학 수업시간에 이론적으로 다뤘던 MVC 패턴을 실습해볼 수 있어서 좋았습니다. 수업 당시에 배웠던 내용으로는 이해하기도 어려웠고 코드를 보고도 잘 모르겠었는데, 직접 코드를 작성하며 이해하니 좀 더 와닿게 되는 것 같습니다.
<궁금했던 점>
MVC 패턴으로 리팩터링을 진행하는데, Car cannot be resolved to a type를 처리할 수 없어 새로운 패키지를 하나 더 만들어 진행하였습니다. 패키지를 하나 더 만들지 않고 처리할 수 있는 방법을 찾을 수 없었습니다. 이 부분에 대해 조언을 듣고 싶습니다.
<중점적으로 고민했던 부분>
indent depth가 제한되어있는 탓에 winCar를 찾는 데 고민이 많았습니다.
을 생각해보았는데, 첫 번째 방법은 두 번째 방법보다 비효율적이라 포기했고, 세 번째 방법은 깊이를 제한한 채로 구현할 수 없을 것 같아 두 번째 방법을 최종적으로 채택하였습니다.
또한, 깊이가 제한되어있는 탓에 한 가지 기능을 구현하기 위해 여러 가지 방법을 생각해야 했습니다. 가령 이긴 자동차 배열을 반환할 때, 전부 순회하며 max를 확인하려면 깊이가 1보다 커지기에 고심하다가, 수업 때 배운 stram 라이브러리를 사용하여 해결하였습니다.