-
Notifications
You must be signed in to change notification settings - Fork 79
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
[자동차 경주] 제세형 미션 제출합니다. #72
Conversation
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주차 미션 고생 많으셨습니다 😄
InteliJ 의 콘솔에서 프로그램의 출력에 포함된 한글이 <?> 로 표시되는 문제가 있어 출력 메세지와 예외 메세지를 영어로 작성했습니다.
아마 인코딩 관련 문제인 것 같습니다. 관련해서 구글에 인텔리제이 한글 깨짐 인코딩
으로 검색하면 관련 자료가 나와서 참고하시면 될 것 같습니다. 혹시나 해결되지 않는다면 DM 주세요 ! 참고자료
MVC 패턴이란 것을 처음 접하다 보니 제대로 적용을 하였는지 모르겠습니다.. 일단 view와 domain을 분리하고 domain이 view에 의존하지 않도록 수정했습니다.
처음 MVC 패턴을 접하면 이게 뭐지라는 반응이 정상인 것 같아요 😢 세형님은 잘 해주신 것 같아요 !
의존하지 않도록 노력한 모습 좋습니다 ! 각 계층에서 어떤 역할을 수행해야 하면 좋은지 코멘트를 달았습니다. 확인 해주시고, 코드를 수정하다 보면 MVC 패턴의 대략적인 느낌을 얻을 수 있을 것 같아요 👍 위키 백과
3주차도 화이팅 입니다 !
while(true){ | ||
if (getValiedNames()){ | ||
break; //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.
재시도 로직 ! 저도 생각도 못해 본 부분이네요 👀
다음 같은 방법이 있을 것 같아요.
- 메소드를 세세하게 더 쪼갠다. (이 부분은 작성해보지 못 했지만, 코드가 쉽지 않아질 것 같다는 생각이 드네요.)
- do-while문을 사용한다. (이렇게 되면 뎁스가 1이하가 되고, 재시도로직도 잘 작동할 것 같아요!)
while(true){ | |
if (getValiedNames()){ | |
break; //5글자를 넘는 이름을 입력받는 경우 재시도 | |
} | |
} | |
String carNames; | |
do{ | |
carNames = InputView.getCarNames(); | |
} while(!getValiedNames(carNames)); |
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.
do-while이 있었네요! 맨날 for, while만 쓰고 do-while은 정말 쓰는 케이스가 적어서 생각을 못했습니다 👍
race = new Race(InputView.getCarNames()); | ||
return true; | ||
}catch (IllegalArgumentException e){ | ||
System.out.println(e.getMessage()); |
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.
세형님은 Application 클래스를 컨트롤러로 사용하신 것 같아요 !
컨트롤러에 시스템 아웃풋과 관련된 로직이 있어, 이 부분은 View로 옮기면 좋을 것 같아요 !
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.
콘솔과 관련된 모든 로직은 View에서만 구현할 수 있게 수정해보겠습니다!
private static void runRace(){ | ||
System.out.println("domain.Race result"); | ||
race.runRace(round); |
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.
여기도 동일합니다 !
레이싱 결과를 출력하기 위한 View만 존재해야 하는지 등 생각해보면 좋을 것 같아요.
@@ -0,0 +1,47 @@ | |||
package domain; | |||
|
|||
public class Car { |
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.
a6025cd 커밋을 보니, setLocation()
메소드 삭제하셨더라구요.
커밋 메시지까지 보니 삭제하신 의도가 좋았습니다 👍
private int calculateNextLocation(int speed){ | ||
if(speed >= 4){ | ||
return this.location+1; | ||
} | ||
return this.location; | ||
} |
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.
뎁스를 줄이기 위해 메소드로 추출한 부분 좋습니다 👍
//승자 구하기 | ||
private void checkWinners(int maxLocation){ | ||
winners = new ArrayList<>(); | ||
|
||
for(Car car: cars){ | ||
addWinners(car, maxLocation); | ||
} | ||
} | ||
|
||
//승자 추가하기 | ||
private void addWinners(Car car, int maxLocation){ | ||
if(car.getLocation() == maxLocation){ | ||
winners.add(car); | ||
} | ||
} |
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.
checkWinners
이라는 메소드명과 다르게 void를 반환하고 있습니다 !
아래 작성된 addWinners의 메소드의 if 조건이 check에 더 가까운 느낌도 있습니다.
addWinners 메소드를 checkWinners로 변경 후 boolean을 반환하는 방식으로 변경하고, checkWinners 메소드를 addWinners로 변경 후 리스트에 넣는 방식을 사용한다면 메소드 명과 반환형을 일치시킬 수 있을 거 같다는 생각이 듭니다 !
//승자 구하기 | |
private void checkWinners(int maxLocation){ | |
winners = new ArrayList<>(); | |
for(Car car: cars){ | |
addWinners(car, maxLocation); | |
} | |
} | |
//승자 추가하기 | |
private void addWinners(Car car, int maxLocation){ | |
if(car.getLocation() == maxLocation){ | |
winners.add(car); | |
} | |
} | |
private void addWinners(Car car, int maxLocation){ | |
if (checkWinners(car, maxLocation)) { | |
winners.add(car); | |
} | |
} | |
private boolean checkWinners(Car car, int maxLocation){ | |
return car.getLocation() == maxLocation; | |
} |
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.
메서드 명명이 제일 어려운 것 같아요 😢
메소드명과 필요인자만 보고서도 메서드가 하는 일과 반환형을 추론할 수 있도록 작성해보겠습니다!
import java.util.Scanner; | ||
|
||
public class InputView { | ||
private final static Scanner scanner = new Scanner(System.in); |
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.
fianl static
과 static final
가 큰 차이는 없지만, 자바에서는 static final
를 공식 컨벤션으로 사용한다고 하네요 !
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 class InputView { | ||
private final static Scanner scanner = new Scanner(System.in); | ||
|
||
public static String getCarNames(){ |
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.
뷰 관련 메소드를 static 메소드로 선언하신 이유가 궁금합니다 !
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.
4단계 - 리펙터링 힌트에서 InputView.getCarNames();
로 별도 인스턴스 생성 없이 클래스에서 바로 호출하기도 했었고, 처음 코드를 작성할 때 Application 클래스 안에서 작성해놓고 나중에 View로 옮기면서 생각없이 그냥 그대로 놔둔거기도 합니다..
그런데 조금 찾아보니 객체 지향에서 정적 메서드가 갖는 단점이 몇가지 있는 것 같아요..! Mocking을 통한 테스트가 불가능 하다는 점과 다형성을 사용할 수 없는 단점이 있는 것 같습니다. 😨
굳이 static을 사용하지 않고도 Application 안에서 지역변수나 필드로 인스턴스화 하여 메서드 호출을 해도 문제가 없기 때문에 static을 사용하지 않는 쪽으로 수정하는게 좋을 것 같기도 합니다 👀
List<String> winnerNames = new ArrayList<>(); | ||
for (Car winner : winners) { | ||
winnerNames.add(winner.getName()); | ||
} |
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.
winners에 있는 Car의 name을 게터로 얻어서 출력을 한다면, view에서 리스트에 넣는 로직을 수행하지 않아도 될 것 같아요 !
|
||
@Test | ||
@DisplayName("우승 자동차 구하기 테스트") | ||
void testRace(){ |
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.
테스트 코드에서는 내가 작성한 코드가 요구 사항을 만족 시켰는지, 코드를 수정한 이후 요구사항을 만족하고 있는지 확인하는 용도로도 사용할 수 있어요. 이 테스트 코드에서 요구 사항을 만족 시켰는 지 확인할 수 있는 방법은 조금 부족하다고 생각이 듭니다.
코드에서는 최소 거리를 이동한 사람이 우승하도록 작성하였지만, 주어진 요구사항은 최대 거리를 이동한 사람이 우승하는 것이라면 이 테스트에서는 이를 검증하기 힘들 것 같아요.
어떻게 하면 이를 입증할 수 있을까요?? 게터를 이용해서 레이싱에 참여하는 자동차에 move함수를 호출해서 임의로 이동 거리를 설정하는 방법도 있을 것 같아요 !
새로운 프로그래밍 요구사항들
이번 미션의 요구사항들을 지키면서 좀더 보기쉽고 누구나 알기 쉬운 코드를 작성하는 방법에 대해 배울 수 있었던 것 같습니다
자바 코드 컨벤션을 학습하기 위해 다음의 문서를 활용하여 학습했습니다. 다만 그 내용이 너무나 방대하여 일단은 클래스,변수,패키지등의 명명규칙과 전반적인 코드 스타일을 중점적으로 지키려고 노력했습니다.
들여쓰기가 1까지만 허용되기 때문에 for 문안에 if를 넣는 것 조차 허용되지 않음을 확인했습니다. 따라서 for 안에 if가 들어가야 하는 코드들에 대해서 메서드로 추출하여 인덴트를 줄이는 방식을 주로 활용했습니다.
Application.java 의 main() 메서드에 while() - if() - break; 구조로 작성된 코드가 있습니다. 이 코드의 인덴트는 2로 새로운 프로그래밍 요구사항을 지키지 못합니다. 예외가 발생할 수도 있는 입력에 대해서 사용자가 올바른 입력을 넣을 때 까지 재시도 하는 의도로 코드를 작성하였는데, if문의 결과로 while문을 탈출 하는 구조에서 인덴트를 줄일 수 있는 방법을 찾지 못했습니다...
각 함수마다 목적을 명확히 분리 하여 작성하려고 노력했습니다. 대부분의 함수가 10라인을 넘어가지 않도록 작성했습니다.
Car 클래스
처음에는 자동차를 움직이는 함수 내부에서 Random 값을 생성하여 움직이도록 했습니다. 하지만 그렇게 하니 자동차가 Random 클래스에 의존하는 문제와 테스트에 어려움이 있어 매개변수로 값을 입력받도록 수정했습니다.
Race 클래스
4단계 미션을 진행하며 MVC 패턴 적용을 위해 UI 기능을 view 패키지에 넣으려고 하였으나, 차량을 한단계씩 움직이면서 자동차의 진행 상황을 표시하는 것을 view 패키지 의존 없이 구현하는 방법을 찾지 못했습니다... 현재 코드에서는 moveAllCars() 메서드와 printCarProgress(Car) 메서드에서 직접 System.out을 호출하고 있습니다.
View
InteliJ 의 콘솔에서 프로그램의 출력에 포함된 한글이 <?> 로 표시되는 문제가 있어 출력 메세지와 예외 메세지를 영어로 작성했습니다.
MVC 패턴
MVC 패턴이란 것을 처음 접하다 보니 제대로 적용을 하였는지 모르겠습니다.. 일단 view와 domain을 분리하고 domain이 view에 의존하지 않도록 수정했습니다.