-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[자동차 경주] 전경호 미션 제출합니다. #74
Conversation
지금 다시 확인해보니 view와 test 폴더가 누락되어 올라간 것 같습니다. 집 가서 바로 추가하겠습니다. (+커밋 완료했습니다.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2주차 미션 수고하셨습니다!
담당 리뷰어가 바쁜 관계로 제가 대신 리뷰 남겼습니다!
질문에 대해서 답변드리자면, model에서 현재 레이싱 게임의 정보를 반환하고 그 정보를 이용해서 view에서 출력해 주는 방식으로 해결해 볼 수있을 것 같습니다!
이번 미션에서 MVC패턴을 이용해서 분리해 보려고 하셨던 것 같아요. 하지만 각 레이어의 책임이 확실하게 분리되지 않은 모습입니다. MVC패턴은 무엇이고 미션에서는 어떻게 적용해야 할지, 어떤 기준으로 레이어를 나누고 책임을 분리해야 할지 고민해보신 뒤 적용해 보시는 것을 추천드립니다!
경호님은 리뷰가 늦어진 만큼 다음주 화요일 까지 리뷰를 반영할 시간을 추가로 드리도록 하겠습니다.
2주차 미션 수고하셨고 다음 미션도 파이팅입니다!
public class Car | ||
{ | ||
private final Random random = new Random(); | ||
private String carName = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이름의 초기값이 ""인 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이전에 python에서 문자열 초기값을 사용할 때 ""로 두는 습관이 있어서 그렇게 두었습니다. java에서는 Null로 쓰는 것이 더 나을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생성자에서 초기화 해주는 것도 방법일 것 같네요😃
src/main/java/domain/Car.java
Outdated
public Car(String inputCarName) | ||
{ | ||
carName = inputCarName; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 컨벤션을 참고해서 중괄호 위치를 수정해주세요!
public Car(String inputCarName) | |
{ | |
carName = inputCarName; | |
} | |
public Car(String inputCarName) { | |
carName = inputCarName; | |
} |
src/main/java/domain/Car.java
Outdated
// // 뷰로 빼야함 | ||
// private void printCurPos() | ||
// { | ||
// System.out.println(carName + " : " + "-".repeat(curPos)); | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 주석 👀
src/main/java/domain/Car.java
Outdated
if (randomSpeed >= 4) | ||
curPos += 1; | ||
|
||
// printCurPos(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
src/main/java/domain/Racing.java
Outdated
// 생성자 함수 | ||
public Racing(String tempCarNames, int count) | ||
{ | ||
for (String name: checkCarCount(splitCarName(tempCarNames))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
splitCatName은 Model의 책임일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputView에서 에러와 함께 처리하도록 수정하겠습니다
src/main/java/domain/Racing.java
Outdated
for (String name: checkCarCount(splitCarName(tempCarNames))) | ||
addCar(name); | ||
|
||
tryCount = checkTryCount(count); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check는 boolean을 반환하는 함수를 의미하기도 해요.
calculateTryCount는 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇군요. 항상 무언가를 검사할 때 함수명에 check를 썼습니다. 새로운거 알아갑니다. 👍
src/main/java/domain/Racing.java
Outdated
return checkFiltering(carName.split(",")); | ||
} | ||
|
||
// 이름이 5글자 이하의 차만 필터링 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드만 보고 어떤 동작인지 이해할 수 있는 것들에 주석을 달 필요가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제거하겠습니당
src/main/java/domain/Racing.java
Outdated
// 모든 자동차에 대해 경주 진행 | ||
public void startRace(int raceCount) | ||
{ | ||
for (int i=0; i<raceCount; i++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공백도 컨벤션 입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@Test | ||
@DisplayName("움직임 테스트") | ||
void movingTest() | ||
{ | ||
int[] moveCount = new int[]{0, 0}; | ||
for (int i=0; i<10000; i++) | ||
{ | ||
String carName = String.format("car{%d}", i); | ||
Car car = new Car(carName); | ||
car.moveCar(); | ||
if (car.getCarPos() == 1) | ||
moveCount[1] += 1; | ||
else | ||
moveCount[0] += 1; | ||
} | ||
System.out.print("0칸 이동 : " + moveCount[0] + ", 1칸 이동 : " + moveCount[1]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트를 테스트답게 짜는 방법은 뭐가있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일반적인 테스트라면 assertThatThrownBy()를 사용하면 될 거 같은데 랜덤 변수에 대한 테스트는 어떻게 해야 할 지 잘 모르겠습니다..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#78
위의 PR의 리뷰 참고해보시는걸 추천드립니다!
수정사항들 반영했습니다! |
1단계 : 자바에서 랜덤값을 어떻게 생성하는지 알 수 있었습니다.
2단계 : 그동안 파이썬만 써와서 "파이썬에 있는 list comprehension과 lambda 함수같은 기능이 java에도 있다면 이를 이용해 indent를 줄여보자" 라고 생각을 했습니다. 찾아보니 java에는 stream이라는 것이 있었습니다. 공부를 해보고 나니 list comprehension보다 훨씬 범용성 있게 사용할 수 있겠다 싶었습니다.
3단계 : 처음에 질문을 잘못 이해해서 4 이상일 경우 나온 수만큼 전진하고, 3 이하일 경우 0칸 전진하는 것으로 잘못 이해해서 자동차의 이동 칸 수가 터미널을 뚫고 나갔습니다 ㅎ... 다행히 커밋하기 전에 알아차려서 고쳤습니다.
4단계 : 정보처리기사 필기를 공부하면서 MVC 패턴이라는 것을 기출에서 본 적이 있는데, 단순히 "사용자 입력을 view에서 처리하고 이를 받아 controller가 model을 조정한다" 라고만 알고 있었습니다. 이번 리펙터링을 통해 MVC 패턴에 대해 더 깊이 이해할 수 있었습니다. 다만, "neo : -----"와 같이 자동차가 몇 번 이동했는지 알려주는 텍스트는 view에서 처리해야 할 거 같은데, move()를 한번 시도한 뒤 이를 출력한다면 domain에서 view를 호출하게 되어 domain이 view에게 의존하는 문제가 발생할 것 같았습니다. 이 문제를 어떻게 해결해야 할 지 몰라서 Car 객체에 해당 부분을 주석처리한 상태로 놔두었습니다... 어떻게 처리하는 것이 좋을까요??