4단계 - 자동차 경주(우승자) #6207
4단계 - 자동차 경주(우승자) #6207
Conversation
README.md - 기존에 포함되어 있던 모든 단계별 학습 내용 제거 - TDD 학습 과제 소개 및 단계별 문서 링크 추가 - 간결한 README 구조로 변경 docs/01-learning-test-practice.md - 1단계 학습 테스트 실습 문서 신규 추가 - String, Set 관련 학습 테스트 요구사항 및 코드리뷰 링크 포함 docs/02-string-calculator.md - 2단계 문자열 덧셈 계산기 과제 문서 신규 추가 - 기능 요구사항, 프로그래밍 요구사항 및 코드리뷰 링크 정리 docs/03-racing-car.md - 3단계 자동차 경주 과제 문서 신규 추가 - 기능 요구사항, 프로그래밍 제약사항, 커밋 컨벤션, 구현 기능 목록 포함
PositiveOrZero.java - 레코드 canonical 생성자 활용으로 불필요한 생성자 제거 - validateInvariant 메서드에 value 전달 방식으로 변경 - 불변성 검증 로직을 명확하게 분리 Car.java - distance 필드 위치를 상수 아래로 이동하여 가독성 향상 Cars.java - 필드 선언 순서를 상수 아래로 재배치하여 코드 일관성 유지
README.md - 4단계 문서 링크 추가 docs/04-racing-car-winner.md - 자동차 이름 및 우승자 기능 요구사항 명세 추가 - 프로그래밍 요구사항 및 제약사항 정리 - 구현 기능 목록(도메인 중심) 초안 작성
CarName.java - 자동차 이름을 감싸는 Value Object `CarName` 추가 - 생성 시 `null` 값에 대한 유효성 검증 로직 구현 CarNameTest.java - `CarName` 생성자에 `null` 입력 시 예외 발생 검증 테스트 작성
CarName.java - 자동차 이름이 `null`이거나 공백(`isBlank()`)인 경우 예외 발생하도록 검증 조건 확장 CarNameTest.java - `@ParameterizedTest`와 `@NullAndEmptySource`, `@ValueSource`를 활용해 다양한 비어있는 입력 케이스 테스트 추가 - 기존 단일 `null` 테스트를 제거하고, 공백 문자열 포함한 검증 테스트로 개선
CarName.java - 자동차 이름이 5자를 초과할 경우 예외를 발생시키는 검증 로직 추가 CarNameTest.java - 자동차 이름이 5자를 초과할 때 발생하는지 검증하는 테스트 추가
CarName.java - 최대 이름 길이 `MAX_NAME_LENGTH` 상수로 추출 - 공백 및 길이 검증 로직을 각각 `validateNotBlank`, `validateLength` 메서드로 분리 - 예외 메시지에 상수 값 동적 반영으로 유지보수성 향상 CarNameTest.java - 정상 입력값(`일`, `일이삼사오`)에 대한 생성 성공 테스트 추가 - 기존 예외 테스트와 함께 정상 케이스까지 커버하도록 테스트 범위 확장
Car.java - `CarName` 필드 추가 및 생성자에서 초기화 - `getName()` 메서드 추가 Cars.java - `Car` 생성 시 테스트 통과를 위한 임시 값 전달하도록 수정 CarTest.java - 이름 유효성 및 초기 거리 검증 테스트 추가 - 기존 테스트를 새 생성자 기반으로 수정
Application.java - `Cars` 생성 시 자동차 개수 대신 이름 리스트를 전달하도록 수정 Cars.java - 생성자를 자동차 이름 리스트 기반으로 변경 - 기존 `count` 기반 검증 및 생성 로직을 `names` 기반으로 리팩터링 InputView.java - `RaceGameInput` 생성 시 자동차 이름 리스트 전달하도록 임시 수정 (`TODO` 주석 추가) RaceGameInput.java - 자동차 이름 리스트(`List<String> carNames`)를 필드로 추가 CarsTest.java - 테스트를 자동차 개수 기반에서 이름 리스트 기반으로 변경 - 입력 이름 개수 검증 및 이동 테스트 수정 RacingGameTest.java - `Cars` 객체 생성 시 자동차 이름 리스트 사용하도록 변경
CarTest.java - 자동차 이름이 5자를 초과할 경우 예외 발생 여부를 검증하는 테스트 추가 - 예외 메시지 내용 확인으로 `CarName` 유효성 검증 로직 정상 동작 확인
04-racing-car-winner.md - 자동차 이름 검증 항목을 구체화 (올바른 값, 비어있는 값, 5자 초과 예외) - 자동차 생성 방식을 “이름 기반 생성”으로 수정 - 자동차 그룹 생성 방식을 “이름 목록 기반”으로 변경
Cars.java - `validateUnique()` 메서드 추가하여 자동차 이름 중복 시 예외 발생 처리 - Set을 활용해 중복 여부 검사 CarsTest.java - 중복된 자동차 이름 입력 시 예외 발생을 검증하는 테스트 추가 - 기존 테스트 메서드명 일부를 명확하게 수정하여 가독성 향상
Car.java - `toSnapshot()` 메서드 추가하여 현재 상태를 CarSnapshot으로 변환 - name 필드를 final로 변경하여 불변성 보장 CarSnapshot.java - 특정 시점의 자동차 상태를 담는 불변 VO 추가 CarTest.java - `toSnapshot()` 메서드가 현재 상태를 정확히 반환하는지 검증하는 테스트 추가
Cars.java - `toSnapshots()` 메서드 추가하여 각 자동차의 현재 상태를 CarSnapshot 목록으로 변환 CarsTest.java - `toSnapshots()` 메서드가 모든 자동차의 상태를 정확히 반환하는지 검증하는 테스트 추가
RoundResult.java - `List<Integer> positions`를 `List<CarSnapshot> snapshots`로 변경 - 자동차 이름과 위치 정보를 함께 관리하도록 개선 - 검증 로직을 간결하게 리팩터링 RacingGame.java - `getRoundResult()`에서 `cars.toSnapshots()` 사용 ResultView.java - CarSnapshot에서 직접 distance 정보를 가져오도록 수정 RaceHistoryTest.java, RacingGameTest.java, RoundResultTest.java - CarSnapshot 기반으로 테스트 케이스 수정 - RoundResultTest를 ParameterizedTest로 개선하여 중복 제거
- 자동차 이름, 자동차, 자동차 그룹 구현 완료 체크 - 자동차 스냅샷 기능 추가 반영 - 라운드 결과 관련 기능 목록 추가
Cars.java - 테스트만을 위해 존재하던 `getDistances()` 메서드 제거 - 프로덕션 코드에서 사용하는 `toSnapshots()`로 일원화 CarsTest.java - `getDistances()` 사용 테스트를 `toSnapshots()`로 변경 - CarSnapshot 기반으로 완전한 상태 검증
RaceHistory.java - 마지막 라운드 기준으로 우승자 목록을 반환하는 `winners()` 메서드 추가 RoundResult.java - 최대 이동 거리의 자동차 이름을 반환하는 `findLeaders()` 메서드 추가 - 내부에서 최대 거리 계산을 수행하는 `maxDistance()` 메서드 구현 ResultView.java - 전체 경기 결과와 우승자 출력 기능 추가 - 출력 형식 정리 및 상수 도입 (`WINNER_SUFFIX`, `NAME_DISTANCE_FORMAT`) - `System.out` 대신 `PrintStream` 상수(`OUT`) 사용으로 테스트 용이성 확보 RaceHistoryTest.java - `winners()` 기능 검증 테스트 추가 (공동 우승 포함 검증) RoundResultTest.java - `findLeaders()` 기능 검증 테스트 추가 (최대 거리 자동차 추출 확인)
InputView.java - 자동차 이름을 쉼표(`,`)로 구분하여 입력받는 기능 추가 - 이름 입력이 비었을 경우 재입력 요청하도록 검증 로직 추가 - `splitCarNames()` 메서드로 입력 문자열을 파싱하여 `List<String>` 반환 - 라운드 수 입력 시 1 이상인지 검증하도록 개선 - 기존 자동차 대수 입력 로직 제거, 관련 메시지 및 메서드 정리
RacingGame.java - `race()` 내부 로직을 `executeRounds()`로 분리하여 가독성 및 책임 명확화 - 라운드 진행과 결과 기록을 별도 메서드로 구성해 응집도 향상 RoundResult.java - `validate()` 파라미터 이름 통일 및 명확화 (`positions` → `snapshots`) - 리더 판별 로직을 `addIfLeader()`로 분리하여 단일 책임 강화 - 최대 거리 계산 로직을 `largerDistance()`로 추출해 중복 제거 및 명확성 향상 InputView.java - 이름 파싱 시 토큰 처리 로직을 `addName()` 메서드로 분리하여 가독성 개선
CarTest.java - 테스트 메서드명을 `move_...`, `toSnapshot_...` 형식으로 변경하여 대상 메서드 명시 - 테스트 의도를 명확히 하여 가독성과 유지보수성 향상
CarSnapshot.java - 이름 공백 및 음수 거리 예외 검증 로직 추가 CarNameTest.java - `@NullAndEmptySource` → `@NullSource`로 수정 CarSnapshotTest.java - 이름·거리 유효성 검증 테스트 추가
04-racing-car-winner.md - 경주 기록에 ‘우승자 목록 조회’ 항목 추가 - 라운드 결과에 ‘선두 자동차 목록 조회’ 항목 추가 - 기존 경주 게임 관련 항목 정리
.gitignore - `/diff` 디렉토리 예외 처리 수정 Makefile - `ghtjr410-diff` 타겟 추가 (origin/ghtjr410과의 diff 생성) scripts/git/diff/origin-ghtjr410-diff.sh - 현재 브랜치와 `origin/ghtjr410` 간의 변경 사항을 patch 파일로 저장하는 스크립트 추가 - 결과 파일은 `diff/commit` 디렉토리에 타임스탬프 기반으로 생성
CarSnapshot.java - 이름 및 거리 검증 로직을 `validateName`, `validateDistance` 메서드로 분리하여 가독성 향상
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
4단계 미션 진행 💯
전체적인 객체 설계와 이에 따른 테스트 구현까지 넘 잘 했네요.
객체 설계를 이미 잘하고 있어 '규칙 3: 모든 원시값과 문자열을 포장한다.' 원칙을 적용해야할 부분을 더 찾아 적용해 보면 좋겠습니다.
추가로 test case가 상당히 잘 구현되어 있는데요.
이미 테스트한 부분을 다른 곳에서도 테스트하는 경우들이 보이는데요.
너무 많은 테스트는 오히려 유지보수를 힘들게 하는 경향도 있기 때문에 정말 필요한 수준으로만 작성하는 것도 중요하다 생각해요.
이번 기회에 이 부분도 의식하고 리팩터링해볼 것을 추천해요.
| test (when adding missing tests) | ||
| chore (maintain) | ||
| ``` | ||
| ## 구현 기능 목록 |
| ## 코드 리뷰 | ||
| > PR 링크: | ||
| > | ||
| ## 나의 학습 목표 |
There was a problem hiding this comment.
단계별 개인 학습 목표를 추가한 점이 인상적이네요
|
|
||
| public Car() { | ||
| private final CarName name; | ||
| private int distance; |
There was a problem hiding this comment.
로또에서 진행해도 되지만 지금 단계에서 도전해 보면 좋을 것 같아 제안해 봅니다.
'규칙 3: 모든 원시값과 문자열을 포장한다.' 원칙을 지키도록 리팩터링해보세요.
리팩터링할 때 TDD 원칙을 지키보면 도전해 보세요.
CarName에는 이미 적용했네요.
| @@ -0,0 +1,22 @@ | |||
| package racingcar.domain; | |||
|
|
|||
| public record CarName(String value) { | |||
| import java.util.Set; | ||
| import racingcar.random.RandomNumber; | ||
|
|
||
| public class Cars { |
| } | ||
|
|
||
| private void addIfLeader(CarSnapshot snapshot, List<String> leaders) { | ||
| if (snapshot.distance() == maxDistance()) { |
There was a problem hiding this comment.
객체의 상태 값을 꺼내지 말고 객체에 메시지를 보내는 방식으로 도전해보면 어떨까요?
작지만 OOP적 사고에서 중요함
|
|
||
| @ParameterizedTest(name = "입력값: {0}") | ||
| @ValueSource(strings = {"일", "일이삼사오"}) | ||
| void 생성자_올바른_이름으로_정상적으로_생성(String input) { |
There was a problem hiding this comment.
CarNameTest에서 이미 테스트했는데 중복이지 않을까?
CarName을 믿고 Car에서는 테스트 진행하지 않아도 되지 않을까?
|
|
||
| assertThat(snapShot.name()).isEqualTo("자동차이름"); | ||
| assertThat(snapShot.distance()).isEqualTo(0); | ||
| } |
There was a problem hiding this comment.
test case의 적정 수준은 어느 정도일까?
무조건 많다고 좋은 것일까?
이번 기회에 어느 정도가 적정 수준일지 의식해 보면 좋겠습니다.
| } | ||
|
|
||
| @Test | ||
| void winners_우승자_목록을_반환한다() { |
| assertThatThrownBy(() -> new RoundResult(List.of())) | ||
| .isInstanceOf(IllegalArgumentException.class) | ||
| .hasMessageContaining("위치 정보는 비어있을 수 없습니다."); | ||
| void findLeaders_선두_자동차_목록을_반환한다() { |
There was a problem hiding this comment.
RaceHistoryTest의 우승자 구하는 부분과 중복된 테스트이지 않을까?
이미 한 곳에서 검증한 로직이라면 굳이 이곳에서는 테스트하지 않아도 되지 않을까?
CarDistance.java - 이동 거리 값 객체 추가 및 기본값 0 설정 CarDistanceTest.java - 기본 생성 시 거리 0 검증 테스트 추가
CarDistance.java - 이동 거리 증가 상수(MOVE_DISTANCE = 1) 추가 - 거리 값을 1 증가시키는 increase() 메서드 구현 CarDistanceTest.java - increase() 호출 시 거리가 1 증가하는 테스트 추가
Car.java - distance 필드를 int에서 CarDistance 객체로 변경 - 이동 시 CarDistance.increase() 호출로 거리 증가 처리 - CarSnapshot 생성 및 getter에서 CarDistance의 값 반환하도록 수정 - 거리 관련 상수 및 직접 연산 로직 제거
- RandomNumber → RandomNumberGenerator로 변경 - SimpleRandomNumber → SimpleRandomNumberGenerator로 변경 - 관련 클래스 및 테스트 코드 전체 수정 반영
- RacingGame이 Round 객체를 사용해 라운드 진행 및 종료 판단하도록 변경 - Round.isFinished() 조건 수정 (현재 라운드 > 최대 라운드)
RaceHistory.java - 경주 기록이 없을 경우 winners() 호출 시 예외 발생하도록 변경 RacingGame.java - 불필요한 MIN_ROUND 상수 제거 CarsTest.java - moveAll 테스트를 메시지 전달 중심으로 단순화 RaceHistoryTest.java - 빈 경주 기록 예외 검증 추가 - 마지막 라운드의 우승자만 반환하는 테스트 추가 RacingGameTest.java - 라운드별 스냅샷 크기 검증으로 테스트 단순화
04-racing-car-winner.md - 자동차, 거리, 랜덤 숫자, 라운드, 스냅샷, 경주 기록, 경주 게임 등 세부 기능 완료 항목 추가 - 각 도메인별 검증 및 동작 시나리오 정리 완료
피드백 반영 사항1. 테스트 중복 제거 — “맥락의 재발견”고민한 점리뷰를 받고 테스트 코드를 다시 살펴보니, 저도 모르게 같은 로직을 여러 곳에서 검증하고 있었습니다. 개선 방향
예시
두 메서드가 비슷해 보이지만, "마지막 라운드"라는 맥락을 진짜로 검증하려면 기존에는 이 차이를 제대로 드러내지 못했는데, 또한 맥락도 동일하고 테스트케이스도 같은 경우는 전부 삭제하였습니다. 2. 원시값 포장 강화고민한 점"어디까지 포장해야 하는가?"에 대한 기준이 명확하지 않았습니다. 개선 방향다음 질문을 기준으로 삼았습니다:
이 기준으로 코드를 다시 보니, 세심한 피드백 덕분에 한 단계 성장할 수 있었습니다. 정말 감사합니다! 리뷰 잘 부탁드립니다! |
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
commit 히스토리만 봐도 미션에 집중하는 모습이 보이네요. 💯
회사에서 시간 나는 틈틈히 미션 진행하는 것 같네요. ㅋㅋㅋ
원시값 포장을 통한 객체 분리 넘 잘 했네요.
바로 merge해도 되지만 5단계는 단순 리팩터링이라 4단계를 찐하게 경험해보고 넘어 갔으면 하는 바람으로 추가 피드백 남겨봤어요.
4단계 찐하게 경험하고 5단계는 빠르게 마무리하시죠.
| RandomNumber randomNumber = generator.generate(); | ||
|
|
||
| if (shouldMoveForward(randomValue)) { | ||
| this.distance += MOVE_DISTANCE; | ||
| if (shouldMoveForward(randomNumber)) { |
There was a problem hiding this comment.
| RandomNumber randomNumber = generator.generate(); | |
| if (shouldMoveForward(randomValue)) { | |
| this.distance += MOVE_DISTANCE; | |
| if (shouldMoveForward(randomNumber)) { | |
| if (randomNumber.movable()) { |
위와 같이 randomNumber가 이동 유무를 판단하도록 구현하는 것은 어떨까?
| public String getName() { | ||
| return this.name.value(); | ||
| } | ||
|
|
||
| public int getDistance() { | ||
| return this.distance; | ||
| return this.distance.getValue(); | ||
| } |
There was a problem hiding this comment.
toSnapshot() 메서드를 통해 값을 외부에서 접근 가능한데 이 두 개의 메서드가 필요할까?
이 2개의 getter 메서드 없이 구현할 수는 없을까?
| @@ -0,0 +1,20 @@ | |||
| package racingcar.domain; | |||
|
|
|||
| public class CarDistance { | |||
|
|
||
| public void increase() { | ||
| this.value += MOVE_DISTANCE; | ||
| } |
There was a problem hiding this comment.
이 같은 vo는 기본적으로 equals, hashCode, toString은 오버라이드하면 어떨까?
아니면 record로 구현
| validateName(name); | ||
| validateDistance(distance); |
There was a problem hiding this comment.
유효성 체크를 다시 해야 한다면 굳이 String, int가 아니라 Name, CarDistance 객체를 생성자의 인자로 받는 것은 어떨까?
| @@ -0,0 +1,24 @@ | |||
| package racingcar.domain; | |||
|
|
|||
| public record RandomNumber(int value) { | |||
| @@ -0,0 +1,34 @@ | |||
| package racingcar.domain; | |||
|
|
|||
| public class Round { | |||
| @Test | ||
| void move_랜덤값이_3_이하이면_정지한다() { | ||
| Car car = new Car("자동차"); | ||
| RandomNumberGenerator generator = () -> new RandomNumber(3); |
| history.record(new RoundResult( | ||
| List.of(new CarSnapshot("apple", 0), new CarSnapshot("banana", 0), new CarSnapshot("orange", 0)))); | ||
|
|
||
| history.record(new RoundResult( | ||
| List.of(new CarSnapshot("apple", 0), new CarSnapshot("banana", 1), new CarSnapshot("orange", 0)))); |
There was a problem hiding this comment.
전체 히스토리를 관리하게 될 경우 round 수가 많아질수록 레이싱 경주 동안 차지하는 메모리 크기가 커질 수 있다.
각 round의 snapshot이 단순히 로그를 위한 출력용이라면 히스토리 전체를 관리하기 보다 마지막 레이스 결과만 가지도록 구현해 보는 것은 어떨까
히스토리 전체를 관리하다보니 테스트 코드 또한 지저분해지는 느낌이 든다.
Car.java - 전진 판단 로직을 RandomNumber로 위임 - 불필요한 shouldMoveForward 메서드 제거 RandomNumber.java - FORWARD_THRESHOLD 상수 추가 - isGreaterThanOrEqual 메서드를 canMoveForward로 변경 - 전진 가능 여부를 내부에서 직접 판단하도록 책임 이전 RandomNumberTest.java - canMoveForward 검증 테스트 추가 - CsvSource를 사용해 다양한 입력값에 대한 전진 가능 여부 테스트로 개선
Car.java - 불필요한 getter(getName, getDistance) 제거 - 스냅샷 객체(CarSnapshot)를 통해 상태 접근하도록 변경 CarTest.java - getter 기반 검증을 스냅샷 기반 검증으로 변경 - 테스트 전반에서 toSnapshot() 사용으로 일관성 확보
Car.java - CarDistance를 불변 객체로 변경함에 따라 필드 재할당 방식으로 수정 - distance.increase() 호출 시 새로운 CarDistance 인스턴스를 생성하도록 변경 CarDistance.java - 클래스를 record로 변경하여 불변성 보장 - increase 메서드가 내부 상태 변경 대신 새로운 인스턴스를 반환하도록 수정 CarDistanceTest.java - 불변 구조에 맞게 테스트 수정 - 메서드 체이닝을 통해 거리 증가 누적 검증하도록 변경
Car.java - CarSnapshot 생성 시 CarName과 CarDistance를 직접 전달하도록 변경 CarSnapshot.java - 생성자를 CarName, CarDistance 기반으로 수정 - 문자열 및 정수 검증 로직 제거 (도메인 객체에 책임 위임) CarSnapshotTest.java - 생성자 인자 변경에 맞춰 테스트 수정 - 음수 거리 예외 테스트 제거 (도메인 책임으로 이전)
RoundResult.java - 클래스를 record로 변경해 불변성 보장 - snapshots() 메서드에서 List.copyOf() 사용으로 외부 변경 방지
RaceHistory.java - getRound 제거, lastRound 메서드 추가 - winners가 마지막 라운드 기준으로 동작하도록 수정 - getRounds에서 List.copyOf로 불변성 보장 RaceHistoryTest.java - lastRound 기반 테스트로 변경 - createRoundResult 헬퍼 추가로 중복 제거 RacingGameTest.java - lastRound 사용으로 테스트 단순화
MovePolicy.java - 랜덤 숫자를 기반으로 이동 가능 여부를 판단하는 정책 인터페이스 정의 DefaultMovePolicy.java - MovePolicy 기본 구현체 추가 - FORWARD_THRESHOLD(4) 이상일 경우 전진하도록 정책 구현
Application.java - RacingGame 생성 시 DefaultMovePolicy 주입하도록 수정 Car.java - move 메서드가 MovePolicy를 받아 이동 가능 여부를 위임하도록 변경 Cars.java - moveAll 메서드가 MovePolicy를 함께 전달받도록 수정 RacingGame.java - MovePolicy 필드 추가 및 주입 기반 구조로 변경 - 자동차 이동 시 MovePolicy 사용하도록 수정 RandomNumber.java - canMoveForward 제거, isGreaterThan 메서드로 단순화 테스트 코드(CarTest, CarsTest, RacingGameTest) - MovePolicy를 테스트 더블로 주입하여 정책 분리에 맞게 수정 - 불필요한 RandomNumber 관련 테스트 제거
04-racing-car-winner.md - 이동 정책 기반으로 RacingGame 생성 로직 명시 - 예외 문구와 설명을 통일된 형식으로 수정 - 각 도메인 설명을 간결하고 의미 중심으로 정리
|
commit 히스토리 보셨나 봐요 ㅋㅋ 피드백 반영 과정이동 가능 여부는 RandomNumber의 책임인가?고민한 점피드백 사항을 개선하면서 랜덤 숫자와 이동 조건을 어떻게 도메인 안에서 자연스럽게 표현할지 고민이 있었습니다.
개선방향이동 여부에 대한 판단 책임을 숫자에서 분리하고,
히스토리 저장 방식 관련 검토 및 결정고민한 점요구사항에서 매 라운드 자동차 위치를 모두 출력해야 하므로, 메모리 사용량을 줄이기 위해
하지만 이러한 방식들은
라는 문제들이 있어 적용하지 않았습니다. 질문요구사항 중심으로 판단하여 모든 라운드의 스냅샷을 저장하는 현재 구조를 유지하기로 결정하였지만 그 외 개선사항
리뷰 잘 부탁드립니다! |
RoundResult.java - 클래스를 record로 변경해 불변성 보장 - snapshots() 메서드에서 List.copyOf() 사용으로 외부 변경 방지
Cars.java - 비즈니스적으로 의미 없는 size() 메서드 제거로 캡슐화 강화 CarsTest.java - size() 기반 테스트 제거 - toSnapshots()를 활용해 자동차 생성 및 이동 결과를 검증하도록 수정
- RaceHistory의 size(), lastRound() 제거 - 해당 메서드 의존 테스트 삭제 또는 winners() 기반으로 리팩토링 - 테스트가 내부 구조 대신 비즈니스 결과를 검증하도록 개선
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
이제 정말 마지막 피드백이 될 것 같네요.
이번 미션에서 고민해볼 수 있는 대부분의 피드백은 모두 한 것 같네요. 👍
추가로 피드백 하나 남겼으니 고민해 보고 적용 여부를 판단해 보면 좋겠습니다.
|
|
||
| import racingcar.domain.RandomNumber; | ||
|
|
||
| public interface MovePolicy { |
There was a problem hiding this comment.
이동 정책을 다루는 인터페이스 추가 👍
MovePolicy와 RandomNumberGenerator 두 개의 인터페이스와 구현체가 자동 이동 유무를 판단하는데 관여하고 있음
이를 다음과 같은 구조로 구현하는 것은 어떨까?
public interface MovePolicy {
boolean movable();
}MovePolicy를 구현하는 RandomValueMovePolicy와 같이 Random 값에 따라 자동차의 이동 유무를 결정하도록 구현하는 것은 어떨까?
Application.java - RandomNumber 기반 정책 제거 후 RandomValue 기반 정책 생성 방식으로 변경 - RacingGame 생성자 및 race 호출부 구조 정리 - DefaultRandomValueMovePolicy 생성 팩토리 메서드 추가 Car.java - move() 메서드 시그니처에서 RandomNumberGenerator 제거 - RandomValueMovePolicy 의 moveable() 결과만 활용하도록 단순화 Cars.java - moveAll()에서 generator 인자 제거 - Car.move() 호출 방식 변경 RacingGame.java - race(), executeRounds(), executeRound()에서 generator 제거 - RandomValueMovePolicy 기반 로직으로 전체 흐름 단순화 DefaultMovePolicy.java - 기존 정책 삭제 DefaultRandomValueMovePolicy.java - 새 랜덤 값 기반 이동 정책 추가 - RandomValueGenerator 의 generate() 활용해 이동 가능 여부 판단 MovePolicy.java - 기존 인터페이스 삭제 RandomValueMovePolicy.java - 이동 가능 여부만 제공하는 새 인터페이스 추가 RandomValueGenerator.java - RandomNumberGenerator → RandomValueGenerator 로 이름 변경 SimpleRandomValueGenerator.java - RandomNumberGenerator 구현체 이름 및 시그니처 변경 CarTest.java - move() 호출 방식 변경에 맞게 generator 제거 - RandomValueMovePolicy 람다 기반 테스트 수정 CarsTest.java - moveAll() 테스트에서 generator 제거 - move 정책 람다로 단순화 RacingGameTest.java - race() 호출시 generator 제거 - RandomValueMovePolicy 기반 테스트로 수정
CarTest.java - 랜덤값 기반 표현을 제거하고 Policy 결과 중심으로 테스트명 수정 - move_이동_불가능한_Policy면_정지한다 로 명확하게 의미 전달 - move_이동_가능한_Policy면_전진한다 로 테스트 의도 명시
DefaultRandomValueMovePolicy.java - 코드 스타일 정리(가독성을 위한 공백 추가) DefaultRandomValueMovePolicyTest.java - 랜덤 값이 4 미만일 때 이동 불가 여부 테스트 추가 - 랜덤 값이 4 이상일 때 이동 가능 여부 테스트 추가 - RandomValueGenerator 스텁을 사용해 정책 동작을 명확히 검증
04-racing-car-winner.md - 이동 정책 섹션 추가 - 랜덤 값 생성기를 통한 이동 판단 요구사항 명시 - 전진 기준값 이상일 경우 이동 가능 조건 체크리스트 반영
|
안녕하세요! 피드백 반영했습니다! 개선 과정인터페이스 통합기존에
책임 재분배// Before
car.move(generator, movePolicy);
// After
car.move(movePolicy);
고민했던 점Application 계층에서 Policy 조립이 복잡해져 하지만 현재는 이동 정책이 하나뿐이라 private 메서드로만 정리하자는 판단을 하였습니다. 혹시 추가로 개선할 부분이 있다면 말씀 부탁드립니다! 리뷰 잘 부탁드립니다 |
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
두 개의 인터페이스를 통합하면서 훨씬 더 깔끔해진 느낌이 드네요. 💯
지금까지 정말 많은 피드백을 남겼는데요.
피드백을 반영하면서 빠르게 성장해 가는 느낌을 받았네요.
5단계에서 추가 리팩터링할 부분 있는지 찾아보세요.
5단계 진행하면서 로또 미션 병행해 나가면 될 것 같아요.
| private void validateRandomValue(int randomValue) { | ||
| if (isOutOfRange(randomValue)) { | ||
| throw new IllegalArgumentException("랜덤 값은 0 이상 9 이하이어야 합니다."); | ||
| public void move(RandomValueMovePolicy movePolicy) { |
|
|
||
| import racingcar.random.RandomValueGenerator; | ||
|
|
||
| public class DefaultRandomValueMovePolicy implements RandomValueMovePolicy { |
There was a problem hiding this comment.
| public class DefaultRandomValueMovePolicy implements RandomValueMovePolicy { | |
| public class RandomValueMovePolicy implements MovePolicy { |
인터페이스에 굳이 RandomValue를 드러내지 않아도 되지 않을까?
| @@ -0,0 +1,5 @@ | |||
| package racingcar.policy; | |||
|
|
|||
| public interface RandomValueMovePolicy { | |||
| return game.race(input.roundCount()); | ||
| } | ||
|
|
||
| private static RandomValueMovePolicy createDefaultMovePolicy() { |
There was a problem hiding this comment.
Spring 프레임워크와 비슷한 형태로 구현한다면 객체간의 의존성 조합은 FactoryBean이라는 객체로 분리할 수도 있겠음.
pr 본문 내용에 고민한 내용이 있어 피드백 남겨봐요.
2 participants
자동차 경주(우승자) 과제 제출
안녕하세요!
자동차 경주(우승자) 과제 제출합니다.
구현과정
TDD 전략의 인식
도메인 책임 분리가 가져오는 확장성
도메인 모델
기존 구조를 유지한 채, 자동차의 이름과 상태를 명확히 분리하여 도메인 확장성을 높였습니다.
각 객체가 "무엇을 책임져야 하는가"를 기준으로 역할을 재정의했습니다.
추가된 모델
기존 모델 추가 역할
이번 과제는 “기능을 새로 만드는 것보다, 기존 구조를 어떻게 유지하며 확장할 것인가”에 집중한 경험이었습니다.
작은 단위의 리팩터링과 테스트가 모여 큰 안정성을 만든다는 점을 실감할 수 있었습니다.
리뷰 잘 부탁드립니다!