[그리디] 김민욱 Java:자동차 경주 미션 3,4 단계 제출합니다.#177
[그리디] 김민욱 Java:자동차 경주 미션 3,4 단계 제출합니다.#177hapdaypy wants to merge 66 commits intonext-step:hapdaypyfrom
Conversation
There was a problem hiding this comment.
안녕하세요 민욱님~!
환절기라 그런지 저도 저번 주부터 감기에 꽤 고생하고 있네요 ㅠ
민욱님도 건강 잘 챙기셔요!
이번 미션은 MVC 패턴을 도입하고, 각 객체의 책임을 어떻게 나눌지 고민하는 과정이 중요한 단계였던 것 같은데요
전반적으로 역할이 잘 분리되어 있었고, 코드만 보더라도 각 객체가 어떤 의도로 만들어졌는지 비교적 잘 드러나서 읽기 편했습니다 👍
전체적으로 미션 요구사항도 잘 반영해주신 것 같아요. 고생 많으셨습니다!
아무래도 코드에는 하나의 정답이 있다기보다,
왜 이런 구조를 선택했는지와 각 책임을 어떻게 바라보았는지가 더 중요하다고 생각하는데요.
그런 관점에서 몇 가지 함께 고민해보면 좋을 것 같은 부분들에 코멘트 남겨두었습니다
가볍게 확인해주시고, 민욱님 생각도 같이 나눠주시면 좋을 것 같아요 🙂
질문 남겨주신 부분에 대해 제 생각을 말해 볼게요
1) 유지보수 비용의 기하급수적 증가
• 초기 Car 객체 내부에 있던 repeat("-") 포맷팅 로직을 객체 지향 및 유지보수를 위해 뷰(Output) 계층으로 변경했습니다.
• 만약 OutputView에 대해 콘솔 출력 I/O 기반의 테스트 코드를 작성할 경우, 향후 UI 출력 형식(띄어쓰기, 문구, 개행 등)이 미세하게 변경될 때마다 코드와 테스트 코드를 동기화하여 수정해야 합니다. 이는 유지보수 관점에서 극도의 비효율을 초래하는 것이 아닌지 궁금합니다.
이 부분은, 애초에 출력 형식을 테스트하겠다고 결정했다면
출력 형식이 변경되었을 때 테스트 코드도 함께 수정되는 것은 자연스러운 일이라고 생각합니다
따라서 저는, 출력 형식을 변경했다면 테스트 역시 그에 맞게 수정해서
실제로 변경된 의도대로 잘 출력되는지를 다시 확인하는 과정 자체를 큰 비효율로 보지는 않습니다
왜냐하면 처음부터 내가 출력 형식이 중요하다고 판단해 작성한 테스트라면,
변경 이후에도 그 형식이 의도한 대로 잘 유지되는지를 확인하는 것 역시
그 테스트가 맡고 있는 역할이라고 생각하기 때문이에요
물론 출력은 문구, 공백, 개행처럼 자주 바뀔 수 있는 요소가 많기 때문에
모든 부분을 너무 세세하게 테스트하면 우려하신 것 처럼 테스트 부담이 커질 수 있을것 같습니다
그래서 모든 출력을 촘촘하게 검증하기보다,
요구사항상 중요한 출력 형식이나 실수 가능성이 있는 부분만 선별해서 테스트하는 방향이 더 적절하다고 생각해요
+)추가로 입력과 출력 모두 System.in, System.out 같은 콘솔 입출력에 의존한 방식으로 테스트할 수도 있지만
(InputView에서 System.setIn(new ByteArrayInputStream(...))를 사용하신 것처럼),
실제로 확인하고 싶은 관심사가 출력 포맷팅 규칙이나 입력 검증 규칙이라면 그 책임을 별도의 메서드나 클래스로 분리해 테스트해볼 수도 있을 것 같습니다.
예를 들어 출력은 이동 거리를 ---로 변환하는 로직을, 입력은 값의 유효성을 판단하는 로직을 따로 분리하면
콘솔 I/O 자체에 의존하지 않고도 핵심 규칙을 더 쉽고 명확하게 검증할 수 있을 것 같아요
2) 도메인 검증과의 중복성 (Over-engineering)
• 핵심 비즈니스 로직과 우승자 판별 등 데이터의 무결성은 이미 도메인 계층의 테스트 코드로 완벽하게 검증되었습니다.
• 도메인에서 전달받은 데이터를 단순히 콘솔에 렌더링하는 수동적 계층인 OutputView까지 텍스트 일치 여부로 검증하는 것은 불필요한 자원 낭비이자 중복 검증이 아닐까 싶습니다.
말씀해 주신 것처럼, 도메인 테스트에서 이미 검증한 비즈니스 로직을 OutputView 테스트에서 다시 확인한다면 그것은 중복 검증이 맞다고 생각해요
다만 OutputView 테스트의 관심사는 도메인 로직이 아니라, 전달받은 데이터를 내가 의도한 형식대로 제대로 출력하는지에 있다고 생각합니다
즉, 이 계층에서 검증하려는 것은 비즈니스 규칙이 아니라 출력 형식이라고 보는 편이 더 자연스러울 것 같습니당
| public Cars(String input) { | ||
| String[] names = input.split(","); | ||
| this.cars = new ArrayList<>(); | ||
| for (String name : names) { | ||
| this.cars.add(new Car(name)); | ||
| } | ||
| validateDuplicate(); | ||
| } |
There was a problem hiding this comment.
Cars가 입력 문자열 파싱을 함께 담당하고 있는 것처럼 보이는데요,
도메인에 해당하는 일급 컬렉션이 입력 형식까지 알고 있어야 하는 구조가 자연스러운지 한 번 고민해보시면 좋을 것 같아요!
(지금의 구현에서는 입력 형식이 (구분자) 변경될 경우 Cars가 영향을 받는 구조가 되었네요)
There was a problem hiding this comment.
리뷰어님의 말씀대로 cars 에서 , 로 구분되어 배열로 만드는 메서드를 따로 분리하면 하게 된다면 구분자 형식이 바뀌었을 경우에 리팩토링을 더 하기 수월하다고 생각해요.
지금 과제는 소규모의 초간단 어플리케이션이 때문에 괜찮을 수 있어 보이지만, 기능적으로 분리를 하는 것이 향후 거대한 프로젝트를 수행할 경우의 유지 보수 측면에서 더 올바른 방식이라고 생각합니다 !
정말 생각하지도 못한 부분이에요 !
| public List<Car> getCarList(){ | ||
| return cars; | ||
| } |
There was a problem hiding this comment.
private final List<Car> cars;불변성을 지키기 위해 final을 선언해 주셨군요!
Q1. 그렇다면 외부에서 getCarList 를 호출하여 자동차 리스트를 가져온 뒤,
carList.add(new Car(~)) 한다면 값이 추가가 될까요??
List<Car> carList = cars.getCarList();
carList.add(new Car(~));Q2. 위처럼 getter로 반환된 컬렉션이 외부에서 수정될 수 있다면,
이 getter 메서드에서 값을 전달해 줄때 어떤 방식으로 컬렉션의 불변성을 보장할 수 있을까요?
There was a problem hiding this comment.
외부에서 자동차 리스트를 가져온 뒤에 자동차 객체를 선언하여 ~ 라는 자동차를 추가한다면 자동차 리스트에 ~ 가 추가가 됩니다.
그럴 경우에는 인스턴스인 리스트를 반환하는 것이 아니라 그냥 주소만 반환을 할 수도 있고 아니면 자동차 리스트를 복사해서 복사된 리스트를 반환하게끔 코드를 수정해보겠습니다!
| @Test | ||
| @DisplayName("공동 우승자를 정확히 판별한다.") | ||
| void getWinners_Multiple() { | ||
| Cars cars = new Cars("pobi,woni,jun"); | ||
| Race race = new Race(cars, 1); | ||
|
|
||
| race.moveAll(() -> true); | ||
|
|
||
| List<String> winners = race.getWinners(); | ||
| assertThat(winners).containsExactly("pobi", "woni", "jun"); | ||
| } |
There was a problem hiding this comment.
공동 우승 테스트에서 containsExactly()를 쓰고 있는데,
해당 메서든 winners 컬렉션의 순서까지 검증을 하게 됩니다
현재 요구사항에서 공동 우승자의 순서까지 중요하게 봐야 하는지는 한 번 더 고민해보셔도 좋을 것 같아요 !
(예를 들어 순서가 중요하지 않은 지금의 요구사항에서는 "pobi", "jun", "woni" 순으로 공동우승자를 반환했다면 이 결과는 유효한 성공 케이스인데, containsExactly() 를 사용하게 되면 해당 케이스는 실패하게 되는 상황이 있을수 있어요)
There was a problem hiding this comment.
저는 3명이 들어온 순서대로 공동 우승을 반환해야 한다고 생각했는데 공우승인 경우인데 순서가 다르다고 틀렸다고 할 수 있으니까 해당 부분에 대해서 오류가 있네요.
| public void moveAll(MoveStrategy moveStrategy) { | ||
| for (Car car : cars.getCarList()) { | ||
| if (moveStrategy.isMovable()) { | ||
| car.move(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Race에서 cars.getCarList()로 Cars의 내부 컬렉션을 꺼내 외부에서 반복을 수행하고 있고
Cars 클래스를 보면 중복된 자동차 이름 유효성 검사 정도를 하고 있어서
Cars가 단순히 컬렉션을 감싸는 역할에 그치고 있는 것처럼 보여요
자동차들의 이동과 같은 동작을 Cars 내부로 옮기는 방향도 가능해 보이는데, 이에 대해서는 어떻게 생각하시나요?
There was a problem hiding this comment.
자동차를 움직여 주는 것은 자동차 매니저들(Cars 리스트)이 해야하는 역할인가 싶고
아니면 race 의 경기에 따라서 car들을 race 가 옮겨주는 것인가 싶습니다.
race에서도 진행할 수 있는 기능을 Cars를 호출해서 전진을 시키는 것은 간편하게 갈 수 있는 길을 살짝 우회를 해서 돌아가는 느낌이라 생각합니다!
어떤 것이 더 효과적인지는 아직까진 잘 모르겠습니다!
There was a problem hiding this comment.
말씀하신 것처럼 설계에 따라 Race가 자동차들을 직접 움직이게 할 수도 있고,
Cars가 자동차들의 집합으로서 그 역할을 맡을 수도 있다고 생각합니다
다만 현재 구조에서는 Cars는 컬렉션을 보관하고,
실제 상태 변화는 Race가 처리하고 있어서 두 객체의 책임 경계가 조금 모호하게 느껴져서 코멘트를 남기게 되었어요
예를 들어 Cars는 자동차들을 움직이는 책임을, Race는 한 라운드 진행이나 최종 우승자 판별의 책임을 갖도록 나누어 볼 수도 있을 것 같습니다
이렇게 나누면 Cars는 자동차 집합을 다루는 객체로서 역할이 조금 더 분명해지고, Race도 경기의 흐름과 결과를 다루는 책임에 더 집중할 수 있을 것 같아요 ~!
P.S 클래스를 설계를 할 때, 각 객체가 무슨 책임을 가질 지를 중심으로 고민하면서 설계를 해 보는 것을 추천드립니다 !!
There was a problem hiding this comment.
race 도메인에서 cars를 호출하는 것이 과연 역할 분리가 잘 되는것인가? 라는 의문이 들었었습니다.! 하지만 Cars가 내부 요소를 직접 순회하며 명령을 전달하는 구조가 객체 지향의 'Tell, Don't Ask' 원칙에 부합하는 검증된 구조임을 깨닫게 되었고
Race가 Cars의 리스트를 강제로 꺼내어 직접 반복문을 돌리는 것이야말로 두 객체 간의 결합도를 높이고 캡슐화를 파괴하는 설계임을 학습했습니다!
제 코드를 섬세하게 봐주셔서 감사합니다 !!!!!
Race의 책임: 총 라운드 수 관리, 현재 라운드 진행 가능 여부 판단, Cars에게 '이번 라운드 진행' 지시.
Cars의 책임: 내부 자료구조(List) 캡슐화 보호, 소속된 모든 Car 요소에 대한 일괄적인 메시지 전파.
이런식으로 각 객체별로의 역할을 정한뒤에 리팩토링을 진행하였습니다.
| public static void println(){ | ||
| System.out.println(); | ||
| } |
There was a problem hiding this comment.
개행 출력을 하는 메서드인 것으로 보여지는 데요, 이걸 따로 메서드로 분리하신 이유가 있을까요?
There was a problem hiding this comment.
System.out을 하는 메서드를 작성해서 호출하는 것이 MVC 패턴을 적용해서 가독성을 올리는 방법이라고 생각했습니다.
하지만 저도 적으면서 굳이? 라는 생각이 들었습니다.
어떤식으로 리팩토링을 하면 좋을까요 ?
고수님의 방식은 어떠한지 제가 참고를 하고싶습니다 ㅜ
There was a problem hiding this comment.
제가 생각하는 MVC의 핵심은 Controller, Model, View의 관심사를 분리해서
각 영역의 변경의 영향이 서로의 영역으로 번지지 않게 하는 데 있다고 생각해요
그런 관점에서 보면 println()을 public 메서드로 열어 두는 것은,
개행이라는 출력 형식의 일부를 View 외부에서 직접 제어할 수 있게 만든다는 점에서
출력 책임이 View에 충분히 응집되지 못하는 구조처럼 느껴졌습니다.
개행 위치나 출력 순서가 바뀌면 Controller에서도 그 흐름을 함께 수정해야 할 수 있기 때문이에요
예를 들어 지금은 라운드 결과를 출력한 뒤 한 줄을 비우고 있지만,
나중에 출력 요구사항이 바뀌어 라운드 시작 전에도 매번 개행이 필요해진다면,
View 내부가 아니라 Controller등에서 움직이기 전에 개행을 한번 더 찍는 등
호출 순서까지 함께 수정해야 할 가능성이 생깁니다
이런 상황은 출력 형식의 변경이 View -> Controller까지 전파되는 구조라고 볼 수 있을 것 같습니다
그래서 저라면
public static void printRoundResult(List<Car> cars) {
for (Car car : cars) {
System.out.println(car.getName() + " : " + "-".repeat(car.getPosition()));
}
System.out.println();
}개행의 출력을 printRoundResult() 같은 메서드 안에서 함께 처리해서 책임을 View 내부에 더 응집시켜 볼 것 같아요 ~
There was a problem hiding this comment.
View 내부를 더 응집을 시켜야 한다는 아이디어는 정말 생각도 못했습니다 !
리팩토링에 반영하겠습니다 !
| @Test | ||
| @DisplayName("자동차 이름 형식을 검증한다.") | ||
| void validateCarNameFormat_Exception() { | ||
| provideInput("pobi,,woni\n"); | ||
| assertThatThrownBy(InputView::inputCarName) | ||
| .isInstanceOf(IllegalArgumentException.class) | ||
| .hasMessageContaining("[ERROR]"); | ||
| } |
There was a problem hiding this comment.
저는 개인적으로 테스트의 네이밍에는 어떤 행동이 주어졌을때 어떤 결과를 기대하는지를 적는 것이
직접 코드를 읽지 않고 메서드 네이밍만 보고 어떤 테스트인지 구체적으로 알 수 있는것 같아서 선호하곤 하는데요
예를 들어 자동차 이름 입력에 쉼표가 연속되면 예외가 발생한다 이런식으로 네이밍을 바꾸면 해당 메서드가 실패 했을때 어떤 케이스에서 실패하게 되었는지 실행 결과만 보고 알 수 있어서 편한 것 같더라구요!
(다른 메서드는 이런 식으로 잘 작성해 주셨는데, 일부는 다른 컨벤션으로 작성해 주셔서 일관성 있게 작성하면 더욱 좋을거 같아요~~)
There was a problem hiding this comment.
리뷰어님처럼 네이밍을 하는것이 더 직관적이여서 그 방식대로 리팩토링을 하고 싶습니다!
테스트의 네이밍 형식과 컨벤션을 잘 지키면서 리팩토링을 진행해보겠습니다.
| public boolean isMovable() { | ||
| return random.nextInt(RANDOM_RANGE) >= MOVE_THRESHOLD; | ||
| } | ||
| } |
There was a problem hiding this comment.
RandomMoveStrategy가 생성자를 통해 Random을 선택적으로 주입받도록 구현해주신 것 같은데요,
랜덤값에 대한 의존성을 분리해서 테스트 가능성을 열어두신 점은 아주 좋은 것 같아요!!
다만 RandomMoveStrategy라는 이름의 객체가 외부에서 주입되는 값에 따라 더 이상 랜덤하지 않은 방식으로도 동작할 수 있다면,
이 객체를 여전히 “랜덤 이동 전략”이라고 할 수 있을까? 라는 생각이 들었습니다
이미 이동 전략을 인터페이스로 잘 추상화해두신 만큼,
테스트를 위해 Random 자체를 주입받기보다는
MoveStrategy를 구현하는 별도의 전략을 만들어 사용하는 방식도 가능해 보이는데 이 방법에 대해서는 어떻게 생각하시나요??
예를 들어 항상 전진하는 전략이나 항상 정지하는 전략처럼 테스트 전용 전략을 분리해두면,
각 전략의 이름과 역할도 더 자연스럽게 맞아떨어질 것 같습니다.
또 이런 전략 구현체를 Race나 Cars 내부에서 직접 결정하기보다
생성자로 외부에서 선택해 주입받는 구조라면, 전략 교체도 더 유연해질 수 있을 것 같아요
There was a problem hiding this comment.
해당 부분은 제가 이해하기 위해서 더 시간이 필요합니다!
제가 이해가 되는대로 바로 답변을 하겠습니다.!
There was a problem hiding this comment.
제가 설명을 구체적이지 못하고 받아 들이는 입장에서는 너무 알쏭달쏭하게 했던 것 같네요 😭
이러한 디자인 패턴을 전략 패턴이라고 하는데, 이와 관련된 잘 정리된 블로그가 하나 있어 남겨 놓을게요!
한 번 읽어 보시면 감이 오실거에요 👍
| private static void validateCarNameFormat(String input) { | ||
| if (input.isBlank()) { | ||
| throw new IllegalArgumentException("[ERROR] 입력값이 없습니다."); | ||
| } | ||
| String noSpaceInput = input.replace(" ", ""); | ||
| if (noSpaceInput.contains(",,")) { | ||
| throw new IllegalArgumentException("[ERROR] 쉼표가 연속으로 입력되었습니다."); | ||
| } | ||
| if (input.startsWith(",") || input.endsWith(",")) { | ||
| throw new IllegalArgumentException("[ERROR] 입력값의 시작이나 끝에 쉼표가 있습니다."); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
여기 자동차 이름 입력을 검사하기위해 if 문으로 여러 케이스를 검증해 주시고 계신데요,
여기서도 Car 도메인 내부에서 자동차 이름을 검증했을때 처럼 정규표현식을 쓸 수 있었을 것 같은데,
이렇게 if 문을 나누어서 처리하게 된 이유가 있을까요??
There was a problem hiding this comment.
view에서 검증하는 것과 domain에서 금증을 해야하는 기능을 분리했습니다!
inputView 같은 경우에는 숫자와 , 로 잘 되어있는지와 같은 입력된 형식에 대해서만을 검증을 하고 car 도메인에서는 정규식을 통해서 올바른 자동차의 이름을 검증하기 위해
다른 클래스로 분리하여 검증을 시도했습니다 !
| public interface MoveStrategy { | ||
| boolean isMovable(); | ||
| } |
There was a problem hiding this comment.
민욱님 저번 리뷰에 대한 답변과, 몇 가지 추가 리뷰를 남겼어요!!
이번 미션 정말 고생하셨어요 👏👏👏
MVC 패턴을 처음 적용해 보셨을텐데, 생각보다 구조를 잘 잡아주셨더라고요!
그래도 항상 "왜 이런 패턴을 쓰라고 하는 거지?" 라는 고민은 항상 하시면서 코드를 작성하는 것을 추천해요.
왜냐하면 패턴 자체를 외우는 것보다, 그 패턴이 해결하려는 문제를 이해하는 게 훨씬 더 오래 남고, 상황에 맞게 응용할 수 있게 되거든요
코드라는 것은 정답이 없기 때문에 뭔가 직접적으로 알려드리기 보다는 질문 형태로 스스로 고민해 볼 수 있는 방향으로 리뷰를 남겼었는데, 이 부분 때문에 좀 어렵게 느껴지셨을 수도 있을 것 같아요. 혹시 리뷰 보시다가 의도가 잘 안 잡히는 부분이 있으면 편하게 여쭤봐 주세요!
P.S 지금 이 PR 타겟 브랜치가 next-step:main으로 되어있었군요... (지금 알았네요 ㅠ) 이 부분 next-step:민욱님 브랜치로 바꿔 주세요
| package racing.domain; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import racing.domain.Car; | ||
|
|
||
| public class CarParser { | ||
| public static List<Car> parse(String input){ | ||
| return Arrays.stream(input.split(",")).map(Car::new).collect(Collectors.toList()); | ||
| } | ||
| } |
There was a problem hiding this comment.
CarParser는 문자열을 Car 리스트로 변환하는 역할을 하고 있어서,
도메인 로직이라기보다는 입력을 해석하는 역할에 가까워 보입니다
도메인 패키지보다는 컨트롤러 계층이나 별도의 파싱/유틸 영역으로 분리해보는 것도 좋을 것 같아요!
There was a problem hiding this comment.
추가로 현재 CarParser는 문자열을 Car 객체 리스트로까지 변환하고 있는데,
개인적으로는 문자열을 나누는 역할까지만 담당하고, Car 객체 생성은 컨트롤러에서 수행하도록 나누는 방향도 고려해볼 수 있을 것 같습니다
예를 들어 CarParser는 "pobi,crong" → ["pobi", "crong"]처럼 문자열 리스트만 반환하고,
컨트롤러에서 해당 값을 기반으로 Car 객체를 생성하면 객체 생성 흐름이 조금 더 명확하게 드러날 수 있을 것 같아요 🙂
P.S 이렇게 문자열을 나누는 역할의 유틸리티성 클래스라면,
도메인에 종속적인 CarParser보다는 InputParser나 StringSplitter처럼 조금 더 범용적인 네이밍을 사용하는 것도 좋아 보여요~~
There was a problem hiding this comment.
저는 String으로 입력만을 담당하는 기능을 한다면 View 계층에 있어야 한다고 생각했고 String을 List 의 형태로 바꿔주는 것은 domain의 역할이라고 생각했습니다.
하지만 view 에서 리스트를 만들어준 다음에 domain에 리스트 값을 넘겨준다면 view 와 domain 사이에 역할 분리가 훨씬 더 잘 분리가 될 수 있겠구나 생각이 들었습니다 !
| public class Cars { | ||
| private final List<Car> cars; | ||
|
|
||
| public Cars(List<Car> cars) { | ||
| this.cars = cars; | ||
| validateDuplicate(); | ||
| } |
| // 불변성 보장: 외부에서 리스트를 수정하지 못하도록 읽기 전용 뷰 반환 | ||
| public List<Car> getCarList() { | ||
| return Collections.unmodifiableList(cars); | ||
| } |
| public class Race { | ||
| private final Cars cars; | ||
| private final int totalRounds; | ||
| private int currentRound; | ||
| private final MoveStrategy moveStrategy; | ||
|
|
||
| public Race(Cars cars, int totalRounds, MoveStrategy moveStrategy) { | ||
| if (totalRounds <= 0) { | ||
| throw new IllegalArgumentException("[ERROR] 시도 횟수는 1 이상이어야 합니다."); | ||
| } | ||
| this.cars = cars; | ||
| this.totalRounds = totalRounds; | ||
| this.currentRound = 0; | ||
| this.moveStrategy = moveStrategy; | ||
| } |
There was a problem hiding this comment.
MoveStrategy 전략을 Race내부가 아니라 외부에서 주입 받을 수 있도록 수정해 주셨군요!! 👏👏👏
| @Test | ||
| @DisplayName("생성된 난수가 4 이상이면 참을 반환하지 못할경우 예외를 발생시킨다.") | ||
| void isMovable_True_WhenRandomIs4OrMore() { | ||
| // 통제된 난수 4를 반환하는 익명 객체 주입 | ||
| Random controlledRandom = new Random() { | ||
| @Override | ||
| public int nextInt(int bound) { | ||
| return 4; | ||
| } | ||
| }; | ||
| RandomMoveStrategy moveStrategy = new RandomMoveStrategy(controlledRandom); | ||
|
|
||
| assertThat(moveStrategy.isMovable()).isTrue(); | ||
| } |
|
민욱님, 고생 많으셨습니다~! 이번 미션에서는 MVC 패턴에 대해 많이 고민해보신 만큼, P.S. 현재 충돌이 발생해서 머지를 바로 하지는 못하는 상황인데요, 이 부분이 해결되면 머지할 수 있을 것 같습니다 😢 |
3 participants
안녕하세요! 리뷰어님!
귀한 시간을 내주셔서 제 코드에 대한 리뷰를 남겨주셔서 정말 감사합니다!
자동차 경주 1,2 주차 미션에서 리팩토링을 통해 3,4주차 미션을 수정 및 보완했습니다!
날씨가 많이 풀렸네요! 환절기에 감기 조심하셔요!!
감사합니다 😄
🛠 개선 사항 (Refactoring & Improvements)
1. Application과 Controller의 역할 분리
Application)과 실행 흐름 제어(Controller)의 책임을 분리하여 구조적 명확성 확보.2. 도메인 핵심 로직 단위 테스트 코드 작성
Car,Cars,Race,RandomMoveStrategy3. RandomMoveStrategy 의존성 역전 및 격리
Controller및 도메인 내부에서 하드코딩된Random인스턴스 의존성 제거.Race실행 시 테스트 더블(Test Double) 주입을 가능하게 하여, 100% 통제 가능하고 결정론적인(Deterministic) 단위 테스트 환경 구축.4.
validateTrialCount내부 캡슐화 적용trialNumber상태 관리 및 유효성 검증 책임을 해당 데이터를 소유한 객체 내부로 캡슐화.🤔 논의 사항 (Discussion Point)
OutputView 단위 테스트 작성의 실효성 고찰
현재 핵심 도메인 로직에 대한 검증은 완료되었으나, 출력(OutputView) 영역에 대한 단위 테스트의 필요성에 대해 의문이 있습니다. 다음 두 가지 관점에서 비효율이 크다고 판단됩니다.
1) 유지보수 비용의 기하급수적 증가
Car객체 내부에 있던repeat("-")포맷팅 로직을 객체 지향 및 유지보수를 위해 뷰(Output) 계층으로 변경했습니다.2) 도메인 검증과의 중복성 (Over-engineering)