-
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
[자동차 경주] 이동훈 미션 제출합니다. #76
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주차 미션 수고하셨습니다!
담당 리뷰어가 바쁜 관계로 제가 대신 리뷰 남겼습니다!
먼저 궁금해 햐셨던 부분에 대해서 답변을 드려 보자면
- BeforeEach를 예외적으로 실행하지 않는 방법
- 기본적으로 BeforeEach를 사용하면 해당 클래스의 모든 테스트 함수에서 실행 됩니다. 따라서 내부 클래스를 이용해서 해당 내부 클래스 에서만 BeforeEach가 실행되도록 할 수 있을 것 같습니다.
- 다른 방법으로는 Junit5의 MethodSource를 이용하는 방법이 있습니다. 관련한 학습 자료는 1주차 학습테스트의 자료에 포함되어 있으니 공부해보시는 것을 추천드립니다.
- 함수에서의 한가지 역할의 기준은 무엇인가?
- 여러가지 기준이 있겠지만 저는 함수의 이름을 기준으로 합니다. validateBlank라는 이름인데, 입력값을 출력한다던지, 파싱하는 코드가 있다면 함수의 역할이 명확하지 않게 되는 것이지요
- 또 다른 기준으로는 하나의 for문, if문, while문 등을 기준으로 나누기도 한답니다.
이번 미션에서 MVC패턴을 이용해서 분리해 보려고 하셨던 것 같아요. 하지만 각 레이어의 책임이 확실하게 분리되지 않은 모습입니다. MVC패턴은 무엇이고 미션에서는 어떻게 적용해야 할지, 어떤 기준으로 레이어를 나누고 책임을 분리해야 할지 고민해보신 뒤 적용해 보시는 것을 추천드립니다!
동훈님은 리뷰가 늦어진 만큼 다음주 화요일 까지 리뷰를 반영할 시간을 추가로 드리도록 하겠습니다.
2주차 미션 수고하셨고 다음 미션도 파이팅입니다!
import java.util.Random; | ||
import java.util.stream.Collectors; | ||
|
||
public class RacingGame { |
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.
RacingGame은 Model에 속한다고 생각되는데 동훈님은 어떻게 생각하시나요?
import java.util.List; | ||
import java.util.Scanner; | ||
|
||
public class InputController { |
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의 역할과 Controller의 역할이 공존하는 것 같습니다!
view패키지로 나눈 만큼 view와 controller로 관련된 역할을 분리해 보는건 어떨까요?
public void race(int times) { | ||
while (times != 0) { | ||
carList.forEach( | ||
car -> car.move(random.nextInt(10)) | ||
); | ||
|
||
outputController.printProgressResult(carList); | ||
|
||
times--; | ||
} | ||
} |
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.
하나의 함수에서 출력까지 담당하고 있는데, controller를 통해서 레이스의 결과를 출력 함수를 호출 하도록 하는건 어떨까요?
@@ -0,0 +1,39 @@ | |||
package util; | |||
|
|||
public class Validation { |
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.
유효성 검사를 하나의 validation클래스로 관리하도록 했군요!
이렇게 했을때의 장점과 단점은 어떤것이 있을까요?
public class OutputController { | ||
public void printProgressResult(List<Car> carList) { | ||
for (Car car : carList) { | ||
System.out.println(car.getName() + " : " + "-".repeat(car.getPos())); |
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.
String.format을 이용하면 문자열 결합 비용을 줄일 수 있답니.
사소하지만 습관을 들여두면 좋을 것 같아요👍�
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주차 미션도 고생 많으셨어요!
|
||
public class Car { | ||
private final String name; | ||
private int pos; |
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 void move(int num) { | ||
if (num >= 4) |
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가 무엇을 의미하는 숫자인지 상수로 표현해보는 것도 괜찮을 것 같아요
String strNames; | ||
String strTimes; | ||
int times; | ||
List<String> nameList = new ArrayList<>(); |
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.
접근제어자를 default로 두신 이유가 궁금합니다!
|
||
public class Validation { | ||
|
||
public void validName(String name) throws RuntimeException { |
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 List<Car> getWinners() { | ||
int maxPos = carList.stream().mapToInt(Car::getPos).max().orElse(Integer.MIN_VALUE); |
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.
스트림을 잘 활용하셨네요!
📚 배운 점
학습 테스트 진행 중 throws 라는 키워드는 예외를 메소드를 호출한 쪽으로 던진다는 내용을 보고 혹시나 해서 메소드의 내부 코드를 뜯어봤는데(Ctrl + B) 메소드에서 무슨 예외를 던지는 지 다 나와있는 것을 확인했습니다. 기존에는 구글링으로 어떠한 예외를 캐치해서 처리했는지 찾아봤는데 이제는 그럴 필요가 없을 것 같습니다.
입력받은 이름은 쉼표를 기준으로 구분한다는 말에 자동반사적으로 split이라는 메소드를 사용했습니다. 그러나 쉼표 뒤에 문자열이 없다면 공백의 문자열이 같이 나뉠거라는 생각을 했으나 공백은 무시되어 입력된다는 것을 단위 테스트 도중에 확인했습니다. 해당 문제를 구글링한 결과 저랑 같은 고민을 가진 블로그 글을 찾아볼 수 있었고 해결법은 아주 간단한 것을 확인했습니다. split 메소드에 인자값으로 -1을 추가한다면 공백도 같이 나뉜다는 것을 알았습니다.
출처 : https://alexander96.tistory.com/10
테스트 코드 작성 중 메소드마다 같은 객체를 생성하고 초기화 하는 코드가 중복되어 이를 어떻게 해결하지? 라는 고민을 해봤고 BeforeEach 어노테이션을 사용하면 된다는 결론을 내렸습니다. 테스트 메소드 실행 전에 수행되어 객체를 생성하고 초기화한다면 각 테스트 메소드마다 중복되는 코드는 없앨 수 있지만 동시에 그러면 객체의 값이 변경된다면 다른 테스트 메소드에 영향이 가는 것 아닌가? 하는 의문도 들었습니다. 다행히 BeforeEach는 각각의 테스트 메소드마다 매번 실행되어 독립적인 테스트 환경을 보장한다고 합니다.
❓ 궁금한 점