-
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
[자동차 경주] 정예림 미션 제출합니다. #75
Conversation
RacingCar 클래스 및 move() 메서드에 다음과 같은 기능을 추가 - 다음 요구사항 충족을 위해 getName()과 getPosition() 메서드 추가 - 0~9 범위의 난수 중 4 이상일 경우 전진하도록 로직 설계 - 전진 성공 시 position 증가, 실패 시 정지 유지 Signed-off-by: yeris <[email protected]>
RacingCar 클래스를 추상 클래스에서 일반 클래스로 변경 - 테스트 코드에서 RacingCar 객체 생성을 위해 구체화된 클래스로 전환 Signed-off-by: yeris <[email protected]>
RacingCar 클래스의 move() 메서드에 대해 다음과 같은 테스트를 실행 - MockRandom 클래스를 사용해 고정된 난수 값을 이용 - 랜덤값이 4 이상일 경우 자동차가 전진하는지 테스트 - 랜덤값이 3 이하일 경우 자동차가 정지하는지 테스트 Signed-off-by: yeris <[email protected]>
Race 클래스에 다음과 같은 기능들을 추가 - RacingCar 객체들을 관리 - 자동차들을 주어진 라운드만큼 이동시키는 기능 추가 - 가장 멀리 간 자동차들을 찾아 목록을 반환하는 기능 추가 Signed-off-by: yeris <[email protected]>
Race 클래스의 race() 및 findWinners() 메서드에 대해 다음과 같은 단위 테스트를 실행 - race() 메서드가 라운드 수만큼 자동차를 전진/정지시키는지 확인 - findWinners() 메서드가 올바른 우승자를 반환하는지 확인 - 동점인 경우 여러 명의 우승자가 나오는지 확인 Signed-off-by: yeris <[email protected]>
- 경기 결과 출력을 위해 Race 클래스에서 자동차 목록을 조회하는 getCars() 메서드 추가 - main 메서드에서 자동차들의 위치를 출력할 수 있도록 기능 확장 Signed-off-by: yeris <[email protected]>
게임 실행을 위해 Main 클래스를 구현하고, 다음과 같은 기능을 추가 - 사용자 입력을 받아 자동차 객체 생성 - 경기 횟수를 입력받고 게임을 실행 - 경기 결과 출력 - 자동차 이름, 시도 횟수 유효성 검사 - 예외 처리로 잘못된 입력 방지 Signed-off-by: yeris <[email protected]>
Main 클래스에서 다음과 같이 입출력 로직을 분리 - InputView : 사용자 입력 처리 (자동차 이름 및 시도 횟수 입력) - ResultView : 경기 결과 출력 (자동차 위치 및 우승자 출력) Signed-off-by: yeris <[email protected]>
Main 클래스의 로직을 분리하여 RacingCarApplication 생성 - InputView를 활용해 사용자 입력 처리 (자동차 이름 및 시도 횟수 입력) - ResultView를 활용해 경기 결과 출력 - 예외 처리 추가하여 잘못된 입력 방지 Signed-off-by: yeris <[email protected]>
- MVC 구조를 명확히 하기 위해 domain 패키지로 이동 Signed-off-by: yeris <[email protected]>
- RaceTest : 기존 import문을 변경된 domain 패키지 경로에 맞게 수정 - RacingCarTest : 기존 import문을 변경된 domain 패키지 경로에 맞게 수정 - RacingCarTest : 가독성 향상을 위해 MockRandom 클래스를 적절한 위치로 재배치 Signed-off-by: yeris <[email protected]>
MVC 구조의 구현을 위해 다음과 같이 Main 클래스를 수정 - Main 클래스에서 RacingCarApplication을 호출하여 프로그램 실행 - 기존 로직을 컨트롤러로 이동 Signed-off-by: yeris <[email protected]>
- InputView에서 처리하던 verifyCarName() 메서드를 RacingCar로 이동 Signed-off-by: yeris <[email protected]>
RacingCarTest에 자동차 이름 검증 테스트를 추가하여 다음과 같은 사항을 확인 - 자동차 이름이 5자를 초과할 경우 예외가 발생하는지 확인 - RacingCar 생성 시 이름 검증이 올바르게 동작하는지 확인 Signed-off-by: yeris <[email protected]>
RaceTest에 테스트를 추가하여 다음과 같은 사항을 확인 - 모든 자동차가 정지한 경우, 전부 우승자로 처리되는지 확인 - findWinners()가 올바르게 동작하는지 확인 Signed-off-by: yeris <[email protected]>
- RacingCarApplication 클래스에 레이싱 게임 실행에 관한 주석 추가 - Main 클래스에 레이싱 게임 애플리케이션 실행에 관한 주석 추가 Signed-off-by: yeris <[email protected]>
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.
이번 과제도 고생하셨습니다. 스스로 많은 고민을 하고 손으로 코드를 짜면서, 더 좋은 코드를 만드는 습관을 잘 만드셨으면 좋겠습니다. 이미 충분히 잘하고 계세요. 졸작 만드느라 리뷰가 조금 늦었지만, 정말 열심히 봤으니 용서해주시면 감사하겠습니다 ㅎㅎ 🙇♂️
1️⃣ MockRandom을 활용한 난수 테스트 방법
일단 ThreadLocalRandom
을 굳이 써야하나 싶네요. 요구사항이나 필요한 상황이 아니었기 때문에..
물론 사용해야한다면 도피는 안되겠죠. 저는 Spy 객체를 활용해서 random값을 불러오는 함수를 mockup 할것 같습니다.
2️⃣ RacingCar 클래스의 abstract 설계
상속이 필요하면 abstract를 쓰겠지만(예를 들어 포르쉐 박스터 같은 자식 클래스들이 생길 경우), 아니라면 굳이 만들 필요는 없죠. 아니면 움직이는 기능과 같은 특성에 대해 interface를 쓰는것도 방법이 될 수 있습니다.
3️⃣ String.repeat(int count) vs StringBuilder
그정도로 큰 차이는 나지 않습니다. 😃
4️⃣ ResultView의 독립적 구조
정말 독립적 구조를 쓴다면, Map<차이름, 위치>
를 쓰시는게 낫습니다.
좋은 고민이예요 👍
💡궁금했던 점 & 공유하고 싶은 점
1️⃣ assertAll() vs assertThat()
그룹화 하는 이유는 assertAll()로 묶을 경우, assertThat() 하나가 실패해도 모든 테스트를 돌려서 디버깅이 쉽습니다.
assertThat()만을 여러개 쓸 경우 앞에있는 assertThat() 하나가 실패하면 뒤에있는 assertThat()을 실행하지 않습니다.
뭐가 더 좋다 보다는 상황에 맞게 쓰시면 됩니다.
2️⃣ System.out.println() vs ResultView.printError()
물론 출력을 위한 클래스를 만들었다면, 해당 클래스에서 써야합니다.
리뷰에도 있지만 try로 감싸는것은 좋지 않습니다. 실제 백엔드에서는 예외를 던지면 이를 캐치하는 ExceptionHandler를 만들어서 예외를 로깅합니다. throw new AnyException("error") 이렇게만 하셔도 됩니다.
3️⃣ 터미널 인코딩 문제
더 간단한 해결책은 Mac을 사시면됩니다.
public class Main { | ||
public static void main(String[] args) { | ||
|
||
// 레이싱 게임 애플리케이션 실행 |
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.
코드로 이미 설명이 된다면 주석이 굳이 필요할까도 고민해보셔도 좋을 것 같습니다. 다른 부분도 포함입니담
try { | ||
List<RacingCar> cars = InputView.getCarNames(); | ||
int rounds = InputView.getRoundsFromUser(); | ||
|
||
Race race = new Race(cars); | ||
race.race(rounds); | ||
|
||
ResultView.printRaceResults(race); | ||
ResultView.printWinners(race.findWinners()); | ||
} 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.
이러한 방식은 좋지 않습니다.
InputView, Race, ResultView 중 어디에서 예외가 발생했는지 명확하지 않아서 디버깅이 힘들어집니다.
List<RacingCar> cars = InputView.getCarNames(); | ||
int rounds = InputView.getRoundsFromUser(); |
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.
getCarNames
에서는 FromUser를 안쓰지만, getRoundsFromUser
에서는 쓰셨군요. 코드든 네이밍이든 일관성을 지키면 좋을 것 같습니다.
|
||
private void verifyCarName(String name) { | ||
if (name.isEmpty() || name.length() > 5) { | ||
throw new IllegalArgumentException("자동차 이름은 1~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.
커스텀 예외에 대해서도 알아보시면 좋을 것 같아요
while (!scanner.hasNextInt()) { | ||
System.out.println("숫자를 입력해야 합니다. 다시 입력하세요:"); | ||
scanner.nextLine(); | ||
} |
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.
👍
// 항상 전진하는 moveRandom 객체 생성 | ||
private static final Random moveRandom = new Random() { | ||
@Override | ||
public int nextInt(int bound) { | ||
return 4; | ||
} | ||
}; | ||
// 항상 정지하는 stopRandom 객체 생성 | ||
private static final Random stopRandom = new Random() { | ||
@Override | ||
public int nextInt(int bound) { | ||
return 3; | ||
} | ||
}; |
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.
👍 👍 좋은 방법이네요.
대신 클래스를 분리하거나 하면 좋을 것 같습니다.
RacingCar에서도 비슷한 로직이 있는것 같아서요
|
||
@BeforeEach | ||
void setup() { | ||
// CarA는 매 라운드마다 전진하고, CarB는 매 라운드마다 정지함 |
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.
그렇다면 차 이름을 moveCar
, stopCar
이렇게 지어도 좋았을 것 같아요.
주석은 정보를 제공하는 좋은 요소지만, 너무 많은 정보는 오히려 불편할수도있다고 생각해요
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
@DisplayName("우승 자동차 구하기 테스트") | ||
class RaceTest { |
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.
해당 클래스는 들여쓰기 길이가 다른것 같습니다. 체크해주세요!
if (rounds <= 0) { | ||
throw new IllegalArgumentException("시도 횟수는 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.
해당 예외는 Race에 있는게 적절할 것 같습니다. 이유를 설명하는것은 숙제로 드리겠습니다 📖
int[] testValues = {4, 5, 6, 7, 8, 9}; // 전진해야 하는 테스트 값 | ||
|
||
for (int mockedValue : testValues) { | ||
RacingCar car = carWithMockRandom(mockedValue); | ||
boolean isMoved = car.move(); | ||
|
||
assertThat(isMoved) | ||
.as("랜덤 값이 %d일 때 자동차가 전진해야 합니다.", mockedValue) | ||
.isTrue(); | ||
} |
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.
고생하셨습니다!
고민을 많이 한 코드인게 느껴지네요.
import java.util.List; | ||
|
||
public class Race { | ||
private final List<RacingCar> cars; |
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.
컬렉션은 null 안정성을 위해 필드에서 바로 초기화하는게 좋습니다.
|
||
@DisplayName("race()가 라운드 수만큼 자동차를 전진/정지 시키는가?") | ||
@Test | ||
void checkCarsMoveCountAfterRace() { |
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.
DisplayName을 사용하지않고 한글로 테스트명을 작성하는 것도 좋습니다
📌 고민했던 부분 & 리뷰받고 싶은 부분
1️⃣ MockRandom을 활용한 난수 테스트 방법
ThreadLocalRandom
을 사용하려 했지만, 테스트가 어려워MockRandom
클래스를 도입하였습니다.이 방식이 테스트의 신뢰성을 확보하는 최선의 방법인지, 혹은 더 좋은 방법이 있을지 궁금합니다!
2️⃣ RacingCar 클래스의 abstract 설계
RacingCar
를abstract
클래스로 설계했으나,오히려 객체 직접 생성이 불가능해지고, 테스트가 어려워지는 문제가 발생했습니다.
이후
abstract
를 제거하면서 더 명확한 객체 생성을 가능하게 변경했는데,객체 지향적 설계 측면에서 더 좋은 접근 방법이 있을까요?!
3️⃣ String.repeat(int count) vs StringBuilder
for
문을String.repeat(int count)
로 변경하여 자동차 위치를 출력하였습니다.하지만 성능을 고려했을 때,
StringBuilder
를 활용하는 것이 더 좋은 선택일까요?유지보수성과 가독성을 고려할 때 어떤 방식이 더 적절한지 피드백을 받고 싶습니다!
4️⃣ ResultView의 독립적 구조
ResultView.printRaceResults(race)
형태로 호출하면Race
객체에 의존하게 되는데,이를
List<RacingCar>
만 받아서 출력하도록 수정하는 것이 더 좋은 설계일까요?MVC 패턴을 유지하면서도 더 캡슐화된 구조를 만들 수 있는 방법이 있을지 고민됩니다.
💡궁금했던 점 & 공유하고 싶은 점
1️⃣ assertAll() vs assertThat()
assertThat()
을 각각 호출하고 있지만,assertAll("", () -> assertThat(…), () -> assertThat(…))
형식을 사용하면 한 번에 여러 개의 테스트를 그룹화할 수 있다고 알고 있습니다.
어떤 방식이 더 나은 테스트 방법인지, 차이가 궁금합니다!
2️⃣ System.out.println() vs ResultView.printError()
System.out.println("오류: " + e.getMessage());
대신ResultView.printError(e.getMessage());
로 개선하는 것이 더 나을까요?MVC 패턴으로 리팩토링할 때
Main
의 코드 변화 없이 그대로 가져와서.. 아쉽지만 여쭤보고 싶습니다!3️⃣ 터미널 인코딩 문제
Main
을 실행할 때, 터미널에서 인코딩 문제가 발생하더라고요.윈도우의 경우 같은 문제를 겪으신 분이 있다면,
intl.cpl 실행 → 시스템 로캘 변경 → Beta기능 활성화
를 통해 해결될 거에요!(1시간 동안 해맸어요.🥲)
📖 학습 테스트를 통해 배운 점