[우아한테크코스 4기] Level 1 3일차 회고

페어 프로그래밍 - 자동차 경주 완성

오늘은 따로 강의나 팀 활동이 없는 날이어서, 아침에 게더타운을 이용한 데일리 미팅을 진행한 뒤에 연로그와 어제하던 페어 프로그래밍을 이어서 계속할 수 있었다. 어제 자동차 도메인인 Car 클래스에서 자기 자신의 이름과 위치를 나타내도록 출력할 때 쓰일 toString() 메소드를 재정의 하는 부분까지 완료했고, 지금 당장 Car 클래스에서 사용할 로직은 모두 구현했다고 생각했기 때문에 다음 단계로 넘어가기로 했다.

아침 열시 반부터 진행된 페어 프로그래밍은 중간에 한 번의 점심 시간과 잠깐의 쉬는 시간을 거쳐 오후 네 시가 되어서 끝났다. 사실 오늘 저녁 넘어서 까지도 작업을 하는 것을 예상했는데, 페어와 합이 잘 맞는지 생각보다 더 일찍 끝났다. 손댈 곳이 없는 완벽한 코드는 아니었고, 100% 만족스러운 코드도 아니었지만 리팩토링까지 마무리하고 나서 나와 연로그가 지금 알고 있는 지식 내에서 더 리팩토링할 만한 부분은 찾지 못했기 때문에 마감 시한 전까지 더 리팩토링할 부분이 있는지 생각해보기로 하고 작업을 마무리지었다.

프로덕션 코드를 작성하고 리팩토링하는 과정에서 엄청 다양한 디자인 패턴이나 스킬들을 사용한 것은 아니지만, 앞으로도 유용하게 쓰일 만 하고 작성하면서도 만족스러웠던 부분을 몇가지 정리해보려고 한다.

일급 컬렉션의 사용

자동차 클래스인 Car 클래스를 만들었지만, 자동차 경주에 참여하는 자동차가 여러 대인 만큼 자동차들의 목록을 관리해주는 것이 필요했다. 그냥 단순히 생각하기에는 Car 클래스를 List에 담아서 관리하는 것을 생각할 수 있었는데, 이렇게 되면 여러 대의 자동차에 명령을 내려줘야 하는 로직들, 예를 들면 모든 자동차들에게 랜덤 값을 주어서 전진 또는 정지를 결정하는 로직 등이 Car 클래스로 내려가거나 더 위의 Controller나 Service단(계층을 이렇게 나누었을 경우)에서 처리되게 되는 단점이 있어서 좋지 못한 방법이라고 생각했다.

여러 대의 자동차들을 처리하는 방법에서 나와 연로그는 쉽게 의견 일치를 보았는데, 이전에 프리코스에서 자동차 경주 미션을 진행했을 때 둘 다 일급 컬렉션을 사용해서 이 부분을 해결했기 때문이었다. 그래서 이번 미션에서도 Car의 리스트를 wrapping한 클래스 Cars를 추가하기로 결정했다.

Car에 일급 컬렉션을 적용한 부분을 살펴보자면,

public class Cars {
	private final List<Car> cars = new ArrayList<>();

	public Cars(String[] carNames) {
		if (validateDuplicatedName(carNames)) {
			throw new IllegalArgumentException("[ERROR] 중복된 이름이 있습니다");
		}
		for (String carName : carNames) {
			cars.add(new Car(carName));
		}
	}

	private boolean validateDuplicatedName(String[] carNames) {
		long distinctSize = Arrays.stream(carNames)
			.distinct().count();

		return distinctSize != carNames.length;
	}
}

와 같이 Car들을 감싼 일급 컬렉션 객체 Cars를 만들어주었다. 우선 이름 규칙에 대해서 짚고 넘어가야 하는데, 이번 미션에 주어진 이름 규칙은

- 자동차 이름은 쉼표(,)를 기준으로 구분한다.
- 이름은 5자 이하만 가능하다.

였고, 자체적으로 생각하기로는 중복된 이름이나 공백을 가진 이름이 있어서는 안된다는 생각이 들어서 해당 규칙들도 추가했다. 여기서 각각의 자동차에 적용되는 이름 규칙인 5자 이하의 이름, 공백으로 된 이름을 검증하는 로직은 Car에서 구현해서 검증에 실패할 경우 IllegalArgumentException을 던지도록 작성했다.

그런데 자동차 이름이 중복되었는지 검증하는 로직은 Car에서는 불가능하다. 어떤 한 Car 객체가 생성될 때 해당 객체에서 다른 자동차들의 이름을 알아낼 수는 없기 때문이다. 따라서 중복을 체크하는 로직을 Cars에서 처리하도록 했다. 만약 Cars로 래핑하지 않았다면, 중복을 체크하는 로직은 컨트롤러에 들어있었을텐데, 래핑을 해서 관련된 비즈니스 로직을 여기서 처리하면서 더 좋은 코드가 되었다.

그리고 자동차 리스트를 Cars로 래핑하면서, 불변한 객체로 만들었다는 장점도 있다. 만약

public class RacingController {
    private final List<Car> cars;
    ...
}

와 같이 컨트롤러의 멤버 변수로 리스트가 들어있었다고 가정하자. 이 경우에 cars가 final로 되어 있어서 불변한 것이 아니냐 라고 생각할 수 있는데, final은 재할당만 막아줄 뿐이지 해당 멤버 변수 내의 값이 변화하는 것을 막을 수 없다. 당장 cars.add() 메소드가 먹히기 때문이다. 그런데 Cars 클래스로 래핑해주게 되면, Cars 클래스 내에는 최초 객체 생성 시점 이후에 리스트에 자동차를 추가 / 제거 하는 로직이 없기 때문에, 컨트롤러에서 자동차 리스트를 수정하지 못하도록 할 수 있다.

도메인 세분화 - 원시 값을 객체로 분리

이번 미션 레이싱 게임에서는 참가자들의 이름을 입력받은 뒤, 게임을 실행할 시행 횟수를 입력받고 해당 횟수만큼 게임을 진행한 뒤에 우승자를 가린다. 처음에는 시도 횟수만큼 controller가 게임을 진행해야 하기 때문에, 횟수 값을 int 값으로 controller에 멤버 변수로 넣어주었다. 그런데 이 경우에 문제가 생겼던 것이, 시도 횟수 값으로 올바르지 않은 값(예를 들어 문자가 들어오거나 음수가 들어왔을 때)이 들어왔을 때 처리를 하는 로직이 controller나 view에 들어간다는 점이었다. 올바르지 않은 값을 view에서 처리하는 것은 정말 좋지 않은 방법이라고 생각했고, 마찬가지로 controller 단에서는 도메인에서 발생시킨 예외를 처리하는 로직만 있어야지 직접 예외를 발생시켜서 처리하는 로직이 있어서는 안된다고 생각했다.

그러다보니 내가 연로그에게 아예 시도 횟수도 Cars 처럼 래핑해서 분리하는 것이 어떻겠냐는 의견을 냈고, 연로그가 조금 생각해보더니 흔쾌히 동의해서 시도 횟수를 다음과 같이 클래스로 분리하게 되었다.

public class TryCount {
	private final int tryCount;

	public TryCount(String countString) {
		this.tryCount = convertStringToInt(countString);
		validatePositive(tryCount);
	}

	private int convertStringToInt(String string) {
		try {
			return Integer.parseInt(string);
		} catch (NumberFormatException e) {
			throw new IllegalArgumentException("[ERROR] 시도할 횟수는 숫자여야 합니다.");
		}
	}

	private void validatePositive(int count) {
		if (count <= 0) {
			throw new IllegalArgumentException("[ERROR] 시도할 횟수는 자연수여야 합니다.");
		}
	}

	...
}

이렇게 해주니 유효성을 검증하는 로직도 도메인에서 관리할 수 있게 되었고, 앞서 Cars 클래스를 사용할 때 처럼 시도 횟수 값이 새로운 인스턴스를 생성하지 않는 한 불변으로 유지된다는 장점도 생겼다.

시도 횟수를 TryCount로 래핑하고 나니까, Car 클래스에 멤버 변수로 들어있는 name과 position에 대해서도 같은 방법을 적용하면 좋겠다는 생각이 들었다.

public class Car {
	private String name;
	private int position;

	public Car(String name) {
		validateName(name);
		this.name = name;
		this.position = 0;
	}
    
    public void go(int random) {
		if (random >= 4) {
			this.position++;
		}
	}
    
    private void validateName(String name) {
		if (name == null || name.isBlank() || name.length() > 5) {
			throw new IllegalArgumentException("[ERROR] 올바르지 않은 이름 입력입니다.");
		}
	}
   
	...
}

처음 클래스를 만들었을 때 Car 클래스는 String name 값과 int position 멤버 변수를 가지고 있었다. 그래서 인스턴스가 생성되는 타이밍에 생성자에 인자로 받은 값을 name으로 할당하고, position 값을 초기 위치인 0으로 초기화 해주는 과정을 거쳤다. name과 position을 분리하게 되면, 각각의 검증 과정을 분리한 클래스에 일임하고 Car 클래스는 생성시에 입력받은 이름을 가지고 이름 인스턴스를 생성하고, 위치 역시 인스턴스를 생성해서 멤버 변수에 할당해주기만 하면 된다. 또한, position을 증가 시키는 비즈니스 로직을 직접 처리하지 않고 위치 객체에 "한 칸 전진해라" 라는 메시지만 주고, position 값을 건드리지 않게 된다.

public class Name {
	private static final int MAX_LENGTH = 5;

	private final String name;

	public Name(String name) {
		this.name = name;
		validate();
	}

	@Override
	public String toString() {
		return this.name;
	}

	private void validate() {
		validateNotEmpty();
		validateBoundary();
	}

	private void validateNotEmpty() {
		if (name == null || name.isBlank()) {
			throw new IllegalArgumentException("[ERROR] 이름은 빈 값으로 입력할 수 없습니다.");
		}
	}

	private void validateBoundary() {
		if (name.length() > MAX_LENGTH) {
			throw new IllegalArgumentException("[ERROR] 너무 긴 이름입니다.");
		}
	}
}

이렇게 분리하고 봤더니 이름의 유효성을 검사하는 로직이 Name 클래스에 들어가게 되어서 도메인의 책임도 조금 더 명확해지는 것 같고(기존에는 Car 클래스에서 이름의 유효성 검증의 책임을 가졌는데, 생각해보니 Car클래스지 Name 클래스가 아닌데 생성 시 유효성 검증에 대한 책임을 진다는 점이 이상했다!) 코드 자체도 더 깔끔해진 것 같았다.

그리고 원래 Car 클래스에서 이름 유효성을 검사할 때는 validateName()이라는 하나의 메소드로 공백 검사와 길이 검사를 모두 했는데, 연로그의 의견대로 공백을 검사하는 메소드 따로, 길이를 검사하는 메소드를 validate() 메소드의 하위 메소드로 분리했다. 분리하고 나니까 각각의 검증 메소드가 의미하는 바가 조금 더 명확해지고, 클린한 코드가 된 것 같다.

public class Position {
	private static final String DISTANCE_MARK = "-";
	private static final int INITIAL_POSITION = 0;

	private int position;

	public Position() {
		this.position = INITIAL_POSITION;
	}

	public void move(int distance) {
		this.position += distance;
	}

	public int toInt() {
		return this.position;
	}

	@Override
	public String toString() {
		return DISTANCE_MARK.repeat(Math.max(0, this.position));
	}
}

자동차의 위치 또한 마찬가지었다. Position 이라는 클래스를 새로 작성해주고, Car의 인스턴스가 생성될 때 마다 새 Position 객체를 생성해서 멤버 변수로 할당되도록 해주었다. move()에 관한 부분에서 나와 연로그의 의견이 다른 부분이 있었는데, 처음 내가 Position 클래스를 작성할 때는 random한 값을 인자로 받아서 값이 전진 혹은 정지를 결정하는 기준에 해당하는지 판단하는 로직을 Position에 넣어주었던 반면, 연로그는 애초에 Car에서 기준에 해당하는지 판단한 후에, 전진해야 하는 경우면 Postion.move()를 호출해 주는 것이 맞는 것 같다는 의견을 냈다. 생각해본 결과 위치 값은 그냥 계속 증가만 시켜주면 되는 건데 전진 여부를 판단하는 로직이 Position에 들어있도록 작성한 것은 내 판단미스라는 생각이 들어서 연로그의 의견을 따라 수정했다.

아, 그리고 미션 규칙에는 매 차례마다 랜덤 값이 기준을 만족하면 한 칸씩 전진하도록 하라고 되어 있어서 처음에는 move() 메소드가 인자를 받지 않고 멤버 변수 값을 1씩 증가시키는 코드를 짰는데, 이 부분 역시 연로그가 만약 전진하는 칸 수가 달라져도 적용하기 쉽도록 전진하는 값을 애초에 인자로 받아서 그만큼 전진시키자는 의견을 내서 적용하였다.

최종적으로는 Car 객체가 다음과 같이 훨씬 깔끔하게 리팩토링되었다.

public class Car {
	private static final int MOVING_DISTANCE = 1;
	private static final int STANDARD_VALUE = 4;

	private final Name name;
	private final Position position;

	public Car(String nameString) {
		this.name = new Name(nameString);
		this.position = new Position();
	}

	public void goOrStop(int random) {
		if (random >= STANDARD_VALUE) {
			position.move(MOVING_DISTANCE);
		}
	}

	public String getName() {
		return this.name.toString();
	}

	public int getPosition() {
		return this.position.toInt();
	}

	@Override
	public String toString() {
		return name + " : " + position;
	}

}

3일차 페어 프로그래밍 후기

어느 정도 적응되고 나니까 불편한 점은 크게 없고 좋은 점만 가득했다. 불편한 점이라면 인텔리제이의 Code With Me 플러그인이 온갖 버그로 말을 듣지 않아서 고생했다는 점 정도...?

어제 회고에서 이미 페어 프로그래밍의 장점들을 여럿 언급 했기 때문에 그 부분에 대한 언급은 넘어가고, 페어 프로그래밍을 진행하면서 특히나 좋았던 점 하나를 꼽자면 실수가 줄어든다는 점인 것 같다. 단순히 오타나 import 실수 같은 부분만을 말하는 것이 아니다. 예를 들어 앞서 말했듯이 Position 클래스에 전진 여부를 판단하는 로직이 포함되도록 하는 등 비효율적이거나 납득되지 않는 코드를 작성하는 것도 어느 정도 방지해 줄 수 있다. 덕분에 프리코스 때보다 훨씬 빠른 시간에 더 좋은 코드를 작성할 수 있었다.

페어랑 의견 차이가 심하거나 성격이 안맞고 했으면 힘든 부분도 있었을텐데 그런 점이 정말 하나도 없어서 좋았던 것은 덤이다. 어제는 5분씩, 오늘은 10분씩 드라이버와 내비게이터를 바꿔가면서 페어 프로그래밍을 진행했는데 매번 5분, 10분이 진짜 빠르게 지나갔고, 그러다보니 어느새 한 시간 두 시간이 훌쩍 지나가 있었을 정도로 몰입해서 코딩을 할 수 있었다. 페어를 이뤄서 좋은 합을 보여줬던 연로그에게 감사할 따름!

개인적으로 반성할 부분은 정적 팩토리 메소드를 적용해보고 싶어서 얘기를 했었는데 연로그의 "그런데 여기에 그 패턴을 적용하면 어떤 이점이 있는거에요?" 라는 물음에 말문이 턱 막혔던 점이다. 내가 어떤 패턴의 장단점에 대해 잘 고민하지 않고 그냥 남들이 좋다고 하니까 써야지 하는 생각을 가지고 있었던 것 같다. 메소드 하나를 분리할 때에도 그 행위로 인해 얻는 이점과 잃게되는 부분을 고민해봐야겠다는 생각이 들었다.

그리고 내일은 자동차 경주 미션에 대한 첫번째 코드리뷰가 있는데, 내 눈에는 보이지 않았지만 리뷰어들의 눈에는 얼마나 많은 리팩토링할 부분이 보일지, 그를 통해 얼마나 많은 것을 얻어갈 수 있을지 기대된다.

작성한 코드 링크