우아한테크코스 레벨1 자동차 경주 미션을 진행하며 배운 내용과 함께 피드백 내용을 정리한다.
1단계 - 자동차 경주 구현
리뷰어: 제이
고민한 내용
랜덤한 값에 대한 테스트 코드 작성
자동차는 랜덤한 값 이상인 경우에는 전진을 하고, 그렇지 않은 경우에는 제자리에 머무는 방식으로 동작한다.
따라서 자동차가 제대로 전진하는지 여부에 대한 테스트를 작성하기 위해서 다음과 같은 코드를 작성하여 테스트하였다.
@Test
@DisplayName("자동차 반복 진행시 정상 동작 확인")
public void 자동차_반복_진행_테스트() throws Exception {
//given
Car car = new Car("woo");
int count = 5;
//when
for (int i = 0; i < count; i++) {
car.progress();
}
//then
assertThat(car.getPosition())
.isGreaterThanOrEqualTo(0)
.isLessThanOrEqualTo(5);
}하지만 이렇게 될 경우, 만약 자동차가 랜덤한 값 이상이어도 전진하지 않는다 해도 해당 테스트 코드는 통과하게 되고, 결국 내가 원하는 기능(랜덤한 값이 어떤 기준값 이상이면 전진한다. 즉, Car내부의 position값이 증가한다.)에 대한 제대로 된 테스트를 할 수 없게 된다.
따라서 테스트 코드에서 어떤 랜덤한 값을 고정해서 줄 수 있다면 제대로 된 테스트가 가능해지게 될 것이다. 하지만 이를 위해서는 Production 코드에 대한 변경이 필요하게 되므로 어떤게 보다 좋은 선택일지에 대한 고민을 하였다.
당시에는 static 메소드인 getRandomNumber()를 제공하는 Util 클래스를 두었고, 이 값을 progress()의 인자로 전달하는 방법을 생각하였다. 즉, 외부에서 값을 전달해주는 방법을 생각하였다.
Getter에 대한 질문
이번 미션의 요구사항뿐 아니라 프리코스의 미션 요구사항에서 Getter를 사용하지말라는 말을 많이 들었다.
하지만 이해가 가지 않았다.
내가 생각하기에 무분별하게 Setter를 두는 것은 해당 객체의 값을 의도하지 않은 곳에서 빈번하게 변경할 위험이 있기 때문에 자제하는 것이 좋다는 것을 생각했지만, Getter는 해당 객체의 인스턴스 변수 값을 변경하지 않기 때문에 필요한 곳에서 사용할 수 있도록 열어두어도 된다고 생각하였다.
예를 들어 다음과 같은 코드가 있다.
private void printProgress() {
for (Car car : carList) {
String carName = car.getName();
int position = car.getPosition();
OutputView.printProgress(carName, position);
}
}위 코드는 carList를 순회하면서 자동차의 이름과 현재 위치를 꺼내오고 이를 View에 전달하여 출력하고 있는 코드이다.
왜 Getter 사용을 지양해야 할까?
피드백 내용
질문에 대한 답
- Getter를 사용해야할 때도 분명히 존재한다. getter의 사용을 지양하라는 것은 절대 사용하지 말라는 것이 아니고, 객체의 상태값을 꺼내서 무엇을 하기보다는 상태값을 가지고 있는 객체가 스스로 일하도록 만드는 것의 중요성을 이야기 하는 것이다.
해당 내용에 대한 보충으로 어떤 객체의 상태에 해당하는 인스턴스 변수의 값을 밖으로 꺼내와서 어떤 비즈니스 로직을 처리하기 보다는 해당 객체에게 메시지를 보내서 요청하라라는 의미에서 Getter 사용을 지양해야하는 것이다. 이는 객체의 역할과 책임과도 연관된다. 어떤 인스턴스 변수(데이터)를 가지는 객체가 있다면 이 데이터에 대한 처리는 해당 객체의 책임이라고 볼 수 있다.
다음은 해당 내용과 관련된 내 생각이다.
또한 질문시 올렸던 코드의 경우 Car에 대한 DTO를 만들어 View로 전달해줌으로써 Car의 Getter를 제거해줄 수 있다.
DTO는 이와 같이 계층간 데이터를 전달해주기 위한 객체이므로 당연히 Getter를 가져도 무방하며, 이렇게 DTO를 사용함으로써 Car의 Getter를 제거해줄 수 있게 되는데, 이렇게 되면 Car의 Getter를 위 예시의 로직(printProgress) 이외에서 사용하는 것을 방지해줄 수 있다.
Q. 자동차의 순위를 판단하는 것은 각각의 자동차에 있어야 할까? 판단자에 있어야 할까?
판단을 하는 쪽에서 정렬 기준을 넣어주면(오름차순, 내림차순) 관리가 수월할 것 같다.
하지만 정렬 조건이 Car에 있다면 판단을 하는 쪽에서 안으로 들어와서 어떤 기준으로 정렬이 되는지를 확인해야 한다. 이에 대한 나의 생각은?
public class Car implements Comparable<Car> {
...
@Override
public int compareTo(Car other) {
return other.getPosition() - this.position;
}
}A. 만약 요구사항이 추가되어 가장 position이 큰 자동차 뿐 아니라 가장 position이 작은 자동차에 대한 판단도 해야 한다고 가정해보자.(혹은 가장 작은 자동차 다음의 자동차 등) 그렇게 된다면 Car 내부에 정렬조건이 있는 기존 코드는 OCP를 지키지 못하고 기존 코드에 대한 수정을 요구하게 된다. 하지만 만약 getMaxPosition() 과 같은 기능에 대한 책임을 가지는 쪽에서 정렬조건을 결정한다면 새로운 요구사항에 맞는 정렬 조건에 따라서 정렬만을 새로 해주면 되므로 기존 코드에 대한 변경을 요구하지 않게 된다. 따라서 Car 객체보다는 이에 대한 정렬(판단)을 하는 쪽에서 정렬 기준(조건)을 제공하는 것이 적절해보인다.
또한 "Car의 정렬의 기준이 바뀌는 것에 의해 Car 객체가 바뀌어야한다."는 어색하다.
Q. 다음 생성자를 사용하는 경우 new Car(""); 혹은 new Car(null); 과 같이 Car 인스턴스를 생성하는 경우 어떻게 될까?
public Car(String name) {
this.name = name;
}A. 자동차 생성시 이름(String)에 대한 검증이 없기 때문에 방생하는 문제이다.
이를 방지하기 위해서 다음과 같은 형태로 개선해볼 수 있다.
public Car(String name) {
vlidateCarName(name);
this.name = name;
}Q. scanner 객체를 잘 사용해주었는데, 학습차원의 질문으로 static하게 field를 선어하면 그렇지 않을 때와 어떤 차이가 있을까요?
public class InputView {
private static final Scanner scanner = new Scanner(System.in);
...
}A. static으로 선언하게 되면 클래스 변수가 되고, 이 경우 생성자를 통해서 객체가 생성되는 시점이 아닌 클래스 로딩 시점에 메모리(메소드 영역)에 올라가게 됩니다. 또한 클래스당 하나가 생성되어 해당 클래스로부터 생성되는 인스턴스 간에 공유하게 된다고 알고 있습니다. 여기서는 InputView의 모든 메소드가 static 이므로 시간적 특성을 고려하여 static 선언을 해주었습니다.
Q. 다음의 3가지 객체가 아래와 같이 선언되었을 때 발생할 수 있는 문제는 어떤게 있을까요?
List에 따라서 List의 승자를 계속해서 만들어 낼 수 있지 않을까요?
private List<Car> carList = new ArrayList<>();
private List<String> winnerNameList = new ArrayList<>();
private int totalAttemptCount;A. carList를 통해서 구할 수 있는 winnerNameList는 인스턴스 변수로 둘 필요가 없다.
즉, 다른 인스턴스 변수를 통해서 생성할 수 있는 값을 굳이 인스턴스 변수로 둘 필요가 없다.
Q. 질문주셨던 내용과 같이 "정확히 3번 진행시키고 테스트하면 잘 테스트해볼 수 있겠다"는 생각의 방향이 너무 좋아요! 그대로 테스트하기 위해서 활용할 수 있는 것에 대한 고민을 해볼 수 있을까요?
@Test
@DisplayName("자동차 반복 진행시 정상 동작 확인")
public void 자동차_반복_진행_테스트() throws Exception {
//given
Car car = new Car("woo");
int count = 5;
//when
for (int i = 0; i < count; i++) {
car.progress();
}
//then
assertThat(car.getPosition())
.isGreaterThanOrEqualTo(0)
.isLessThanOrEqualTo(5);
}A.Car 클래스의 progress 메소드에서 인자로 (랜덤한) 숫자를 받도록 수정함으로써 테스트해볼 수 있다.
Q. util성 클래스에서 view 단의 메소드를 호출하고 있다.
클래스의 책임과 역할을 분명히하고, 관계를 잡아보면 어떨까요?
A. 컨트롤러 역할을 하는 RacingGame에서 carNames와 attempt를 넘겨주도록 하여 Util에서 View의 의존을 끊어주었습니다.
추가적으로 단순 기능만을 제공하는 Util 클래스가 좋은 객체지향 설계인지에 대한 고민을 하게끔 하였다. 처음에 Util 클래스로 구현한 기능들, 특히 InitUtil과 같이 객체의 초기화를 위해서 만든 클래스의 경우, '사용자 입력으로 부터 객체를 생성해 List로 반환해준다.' 라는 역할과 책임을 Controller 쪽으로 이동(view와 domain 사이의 처리(변경))시켜줄 수 있기 때문에 Util클래스를 둘 이유가 없다.
Q. 이 부분이 생성되는 것을 보면 InitUtil에서 생성로직을 담당하고 있죠?
이렇게 어떠한 곳에서 생성을 담당하게 된다는 것은 저 List로 묶여있는 존재들이
일정한 규칙을 가져야한다는 것을 의미할 수 있어요.
현재는 List에서 중복되는 자동차 이름이 존재하면 안되는거죠.
즉, List로 묶여 있는 자료구조가 일종의 로직을 갖게 된다는 것입니다.
2단계 미션을 진행하면서 일급컬렉션에 대해 공부해보고, 적용해보면 좋을 것 같아요!
제가 드린 링크는 참고 정도로만 사용하시고 다른 글들도 많이 참고해보셔도 좋을 것 같아요!
private void initRacingCarGame() throws IllegalArgumentException {
String carNames = getCarNames();
carList = initCar(carNames);A. 어떤 컬렉션이 일정한 규칙을 가지고 생성되어야 한다. 즉, 컬렉션으로 묶여 있는 자료구조가 일종의 로직을 가진다. 이를 Wrapping함으로써 검증과 같이 어떤 특정 조건을 가진 자료구조를 하나의 클래스로 묶음으로써 해당 검증이 필요하지 않은 곳으로부터 분리해낼 수 있을 뿐 아니라, 불변을 보장할 수 있다.
Q. 아래 쪽 List에 대한 내용과 비슷하게, totalAttemptCount도 물론 int이지만, 기존의 int와는 조금 다른 int죠. 양수만 가능한 int입니다.
해당 로직을 넣은 객체로 포장해보는건 어떨까요??
A. 원시타입을 그대로 사용하는 것이 아닌 VO(Value Object)를 사용함으로써 해당 데이터의 특징을 살릴 수 있다.(생성시에 검증도 가능)
예를 들어 totalAttemptCount는 양수만이 가능하다는 조건을 가지는데, 이를 단순 원시 타입인 int가 아닌 VO를 사용함으로써 명시적으로 totalAttemptCount를 사용할 수 있을 뿐 아니라 생성시에 검증할 수도 있게된다.
또한
while (totalAttemptCount --> ZERO) {
...
}와 같은 코드를 다음과 같이 해당 VO객체에게 메시지를 보내도록 할 수 있다.
while (gameTotalCount.isContinue()) {
...
}이렇게 함으로써 얻는 장점은 이전 코드에서는 'totalAttemptCount가 0이 되어야 while이 빠져나오는 구나.'를 totalAttemptCount를 사용하는 코드에서 알아야하며 해당 조건(0이 되는 조건)도 totalAttemptCount를 사용하는 곳에서 직접 조작해주어야 한다.
하지만 이렇게 메시지를 보냄으로써 사용자 코드는 gameToatalCount에게 메시지를 보내고 진행 가능 여부만 알면 되며, totalAttemptCount의 데이터를 직접 조작할 필요가 없어진다.
- Collections.sort
이전까지는 정렬을 해야 하는 경우 다음과 같은 형태로 정렬을 수행하였다.
Collections.sort(carList, (o1, o2) -> o2.getPosition() - o1.getPosition());하지만 List에서 제공하는 sort() 메소드를 활용함으로써 코드의 가독성을 높일 수 있다.
(더군다나 Collections의 sort() 내부에서 list의 sort()를 호출하는 것으로 볼 때, 성능에는 차이가 없어 보인다.)
carList.sort((o1, o2) -> o2.getPosition() - o1.getPosition());Q.이러한 VerificationUtil을 따로 만들었을 때 어떤 장점과 단점이 존재할까요?
A. 검증에 대한 책임을 유틸성 클래스에 위임할 수 있다는 것이 장점이라고 생각합니다! 따라서 요구사항(글자가 10글자까지 허용되도록)이 변경되어도 해당 클래스에서만 변경이 일어날 것이라고 생각합니다! 하지만 해당 검증이 필요하지 않은 도메인에서도 사용 가능하다는게 단점으로 작용할 수도 있다고 생각합니다..!! (네이밍이 명확하지 않은 경우 다른 검증 메소드를 불러 호출할 수도 있습니다!)
위와 같이 답변을 달았었다.
하지만 VerificationUtil 클래스를 자세히 들여다보자.
자동차 이름에 대한 검증, 그리고 자동창 이름 중복에 대한 검증 크게 두가지 기능을 제공하는 클래스이다.
그렇다면 자동차 이름에 대한 검증은 자동차 생성 시점에 일어나야하며 이는 자동차(Car) 객체의 책임이 아닐까?
더 나아가서는 CarName이라는 VO 객체의 책임이 아닐까?
또 자동차 이름 중복에 대한 검증은 Cars 라고 하는List<Car>를 관리하는 일급컬렉션의 생성 시점에 검사되어야 하는 하는 것이 아닐까?
또한 해당 검증이 필요하지 않은 도메인에서도 사용 가능하다는 것은 그렇게 좋지 못한 생각인 것 같다. (오용으로 이어질 수 있음)
마지막으로 '검증에 대한 책임을 유틸성 클래스에 위임할 수 있다는 것이 장점'이라고 이야기 하였었는데, 요구사항(글자가 10글자까지 허용되도록)이 변경된다면 이는 다른 verification에 대한 책임도 가지고 있는 VerificationUtil의 변경이 아닌 "자동차 이름" 이라고 하는 CarName에만 변경이 일어나도록 하는 것이 보다 적절한 설계이라고 생각한다.
2단계 - 자동차 경주 리팩터링
리뷰어: 제이
고민한 내용
MVC 패턴 적용
리팩터링 요구사항
- 핵심 비지니스 로직을 가지는 객체를 domain 패키지, UI 관련한 객체를 view 패키지에 구현한다.
- MVC 패턴 기반으로 리팩터링해 view 패키지의 객체가 domain 패키지 객체에 의존할 수 있지만, domain 패키지의 객체는 view 패키지 객체에 의존하지 않도록 구현한다.
변경 및 고려 사항
- 기존 코드에서 핵심 비지니스 로직을 가지는 객체를 domain 패키지에, UI 관련 객체를 view 패키지에 분리하였으므로 첫번째 요구사항을 만족하여 별도의 수정을 하지 않았다.
- 기존의 RacingGame이 Controller의 역할을 하고 있으면서
view 패키지의 객체가 domain 패키지의 객체에 의존할 수 있지만, domain 패키지의 객체는 view 패키지 객체에 의존하지 않도록 구현하였으므로 이 또한 별도의 리팩토링을 수행하지 않았다.
하지만view 패키지에서 domain 객체에 직접 의존이라는 점에서 DTO의 적절한 사용에 대한 고민을 하였다.
피드백 내용
-
테스트는 하나의 좋은 문서가 되어야 한다. 따라서 '엣지 케이스'에 대한 테스트를 하는 것이 좋다.
예를 들어 자동차 경주 게임에서 이동 조건이 '4이상인 수일 때 이동' 이라고하면 엣지 케이스는 해당 조건을 만족시키는 최소의 수인 4와 해당 조건을 불만족 시키는 제일 큰 수인 3이 될 것이다.private static final int MINIMUM_MOVE_CONDITION = 4; private static final int MAXIMUM_STOP_CONDITION = 3;만약 위 두 상수의 값이 0과 10이라고 하면 '10이 되어야 이동하는 구나', '0이 되어야 멈추는구나'라고 인식할 수 있다. 즉, 적절한
문서의 역할을 할 수 없다. -
매직넘버에 대한 상수를 사용한다고 했을 때, 단순히
int ZERO = 0과 같은 상수 네이밍 보다는 해당 상수 '0'이 의미하는 것이 무엇인지를 고민하고 해당 의미를 상수의 이름으로 사용하는 것이 더 좋다. -
Car에서 단순히 랜덤한 값을 progress() 메소드의 매개변수로 받는 것이 아니라, 자동차의 진행을 결정하는 로직을 인터페이스로 분리해서 구현해볼 수 있다.
즉, 움직임에 대한 적략을 가지는 interface를 정의하고 이에 대한 Concrete Class에서 각 움직임 전략에 따른 구현체를 정의할 수 있다.
이를 통해서 우리는테스트 간으한 부분과 테스트하기 힘든 부분을 분리해 테스트 가능한 부분에 대해서만 테스트가 가능해지게 된다. -
다음의 코드는
현재 진행 중인 상태를 묻는 것과상태를 변화시키는 기능이 하나의 메소드에 함께 존재하고 있다. 이것으로 인한 사이드 이펙트는 없을까?public boolean isContinue() { if (totalAttemptCount == GAME_END_CONDITION) { return false; } totalAttemptCount = totalAttemptCount -1; return true; }전체 시도 횟수에 대한 로직이 변경 -> 수정
진행 조건이 변경 -> 수정
즉, 변경의 원인이 여러개 이다.
예를 들어서 전체 시도횟수에 대한 로직이 다음과 같이 바뀌면 isContinue()를 변경해야한다.
(VIP는 1씩 감소하고 GENERAL은 2씩 감소한다.)
따라서 위와같은 메소드는현재 진행 중인 상태를 묻는 메소드와상태를 변화시키는 메소드두 가지로 분리하는 것이 좋다.
이와 관련하여 명령과 조회의 분리라는 CQRS도 참고해볼 수 있다.
참고해볼만한 블로그
-
정적 팩토리 메소드
public GameTotalCount(String attempt) { initTotalAttempt(attempt); }GameTotalCount에서 가장 중요한 생성 규칙은 0보다 큰 수를 만든다는 것이다. 따라서 이를 생성자(Constructor)의 핵심 로직으로 두고, 문자열 등 다른 입력 값으로 만드는 것을 별도의 정적팩토리 메소드로 구현할 수 있다.
생성자 보단 정적 팩토리 메소드private GameTotalCount(int totalAttemptCount) { validatePositiveNumber(totalAttemptCount); this.totalAttemptCount = totalAttemptCount; } public static GameTotalCount createGameTotalCount(String attempt) { int number = translateInteger(attempt); return new GameTotalCount(number); } -
DTO를 사용하는 이유는 다양하다.
앞서 고민했던 내용 중 하나인view 패키지에서 domain 객체에 직접 의존이라는 점에서도 DTO를 적용함으로써 view에서 domain에 대한 직접적인 의존을 끊어낼 수 있다.for (int i = 0; i < cars.getSize(); i++) { Car car = cars.getCar(i); String carName = car.getName(); int position = car.getPosition(); OutputView.printProgress(carName, position); }view패키지에서 domain 객체에 직접 의존함으로써 발생할 수 있는 문제 상황을 보이는 코드이다.
만약 자동차에 대한 데이터가 더 많아지고, 이를 view에 전달해야한다면 어떻게 될까? 코드가 엄청나게 길어질 것이다. 왜 그럴까? 그 이유는 view가 domain 객체에 직접 의존하고 있기 때문이다. 단순히 domain을 view로 넘겨야만이 직접 의존하는 것이 아니다. 지금 코드를 보면 Car 객체의 데이터를 직접 전달해주고 있다. 이는 view에 car를 전달하고 내부에서 getter를 사용하는 것과 차이가 없다.
이를 해결해주기 위해서 DTO를 사용할 수 있다. Car로부터 view에 필요한 데이터만을 가지는 CarDto를 만들어 넘김으로써 이를 해결해줄 수 있다.
(이 때, domain은 DTO 존재에 대해서 알 필요가 없다.) -
배열 대신
java collection을 사용하라.
java collection에서 제공하는 다양한 api는 강력하며 단순 배열을 사용하는 것보다 효율적이다. 따라서 내가 내린 결론은, 이런 api가 불필요한 경우에도 통일성을 위해서 모두 collection을 사용하는게 좋아보인다. 왜냐하면 배열 대신 java collection을 쓴다고 해서 별도의 비용이 드는 것이 아니기 때문이다.
