자동차 경주 2단계 회고

리팩터링 요구사항

• 핵심 비즈니스 로직을 가지는 객체를 domain 패키지, UI 관련한 객체를 view 패키지에 구현한다.
• MVC 패턴 기반으로 리팩터링 한다.
• 테스트 가능한 부분에 대해 단위 테스트를 진행한다.
  
  

리팩터링

원시값 포장

1단계에서 나는 자동차 위치에 대해서는 원시값 포장을 했지만 자동차 이름과 시도 횟수에 대해서는 원시값 포장을 하지 않았다. 1단계 미션에서는 자동차 위치를 업데이트 시켜주는 로직이 필요했기 때문에 Position 클래스를 만들어 이 곳에서 위치 이동 기능을 구현했다.

2단계 리팩터링할 것들을 찾아보니 자동차 이름을 나타내는 필드가 Car 객체 안에 원시값 그대로 정의되어 있는 것을 보았다.

// Car 객체 내부
private final String name;
private final Position currentPosition;

public Car(String name, int startPoint) {
    validateName(name);
    this.name = name;
    this.currentPosition = new Position(startPoint);
}

자동차 위치는 원시값 포장이 되어있는데, 자동차 이름은 포장되어있지 않아 이상함을 느꼈다. "원시값 포장"에 대해 좀 더 자세히 알아보았고, 자동차는 순전히 자동차 관련 역할만 해야 하는데 자동차 이름에 대한 유효성 검사도 하고 있었다는 것을 알아챌 수 있었다. CarName이라는 클래스를 만들었고, 자동차 객체의 역할을 줄일 수 있었다.

자동차뿐만 아니라 controller에서도 원시값 포장을 하지 못한 변수를 찾았다. 자동차 이동 진행 횟수를 저장하고 있는 인스턴스 변수인데, 아래 코드와 같이 그저 인스턴스 변수로 이동 횟수를 저장하고 있었다.

// controller 내부
public void run() {
    Cars cars = initCars();
    int tries = initTries();
    OutputView.printResultMessage();
    race(cars, tries, numberGenerator);
    showFinalStatus(cars);
    prizeWinner(cars);
}

tries도 남은 이동이 몇 회인지 알려주고, 이동 후에 이동했다고 표시해주는 기능이 있는 적지 않은 역할을 하는 변수로 보였다. 이 부분 또한 객체로 만들어주면 좋겠다고 생각하여 바로 클래스로 빼주어 역할을 나누었다.

글을 작성하는 이 시점에서는 이 이유뿐만 아니라, controller의 역할때문에도 분리해야한다고 생각한다. controller는 view와 domain을 연결시켜주는 역할만 해야하는데, 직접 변수를 두어 몇 번 남았는지 세는 기능도 두었던 것이다. 다음 미션부터는 MVC도 확실하게 분리하도록 노력해야겠다.

final 키워드로 값의 변경 보호

매서드에서 다른 매서드를 호출할 때 인자로 보내는 값은 보호받아야 한다. 다른 매서드에서 의도치않게 값을 변경할 수도 있기 때문이다. 그걸 위한 것이 바로 final인데, 이 키워드를 사용하는 것을 잊었었다. 실수를 인지하고 값의 변경을 막아야하는 부분에 대해서는 바로 final 키워드를 붙였다.

피드백

static은 충분히 생각하고 사용하자

나는 우승자를 구하는 클래스는 공용으로 사용되어도 아무 문제 없고, 필드 값도 없고, 그거 우승자 추출기 기계처럼 보아 모든 메서드를 static으로 구현했다. 아래는 내가 남긴 코멘트이다.

저는 우승자를 구하는 클래스만을 보았을 때 단점은 단점으로 보이지 않았습니다. 첫째로, 인스턴스 변수가 존재하지 않기 때문에 캡슐화를 고려할 필요가 없다고 생각했습니다. 또, 우승자를 구하는 객체는 단 하나의 인스턴스만 생성이 됩니다. 인스턴스를 생성하지 않고, static 객체 그대로 사용해도 크게 문제가 되지 않는다고 생각했습니다.
인스턴스를 생성하지 않음으로써 코드를 조금 줄일 수 있고, 속도도 더 빠를 것 같아서 static을 사용하게 되었습니다. 같은 이유로 InputView와 OutputView를 static으로 관리했습니다!
구글링을 통해 static이 interface를 생성하지 못하게 한다.라는 정보를 추가적으로 배웠습니다. 우승자를 구하는 클래스가 interface까지 고려할만한 클래스라는 생각은 들지 않아서요... 혹시 어떤 부분 때문에 static 사용에 대해 말씀하셨는지 이야기해주실 수 있을까요? 그리고 혹시 static을 지양해야 한다면 이걸 싱글턴 패턴으로 적용해도 괜찮을까요?

그리고 리뷰어께서 "객체 지향적인가?"라는 질문을 던져주셨다. 질문을 듣자마자 나는 객체 지향을 공부하는 사람인데, 객체 지향 원칙을 무시하고 효율성을 따진 사람처럼 느껴졌다. 우리같은 객체 지향 개발자들은 작은 효율보다 유지와 보수에 더 집중해야한다는 것을 이번 미션을 통해 다시 가슴에 새겼다.

오버 엔지니어링

미래에 프로그램이 더 확장하게 될 때, 어떻게 코드를 작성해야 업데이트하기 편할지 고민하며 미션을 진행했다. 내 딴에는 유지ㆍ보수가 편하도록 현재는 필요하지 않은 기능들도 고려하며 코드를 작성했다.

2단계 코드애서 자동차 객체를 생성할 때 자동차의 시작 위치도 생성자 파라미터로 넣어주었다. 리뷰가 왔을 때, 현재 프로그램에서 모든 자동차의 시작점은 0으로 동일한데 왜 생성자 파라미터로 넣었는지 질문받았다. 나중에 핸디캡 기능이 생겨서 자동차마다 시작점이 다를 수도 있을 것 같아 생성자 파라미터로 넣어주었다고 대답했는데, 리뷰어께서 오버 엔지니어링인 것 같다고 조심스럽게 말씀해주셨다.

추가적으로 다른 개발자들이 내 코드와 같은 형태의 생성자를 보았을 때 "왜 이 생성자에 출발 위치를 넣지?"와 같이 헷갈릴 수 있다고 하셨다. 함께 개발해가는 것이기 때문에 지나치게 미래 지향적인 것보다는 현재 요구사항에 충실하고 단순하게 개발하는 것이 의사소통에 도움이 되지 않을까? 라는 의견을 내주셨다. 리뷰어님의 말씀을 듣고 그 의견에 동의했다.

하지만 여전히 "확장성을 고려한 개발"과 "오버 엔지니어링"의 기준이 확실하게 잡히지는 않는다. 어디까지가 확장성이고 어디부터가 오버 엔지니어링일까?

경험으로 알 수 있는 부분이라고 생각하고, 나머지 미션들에서 열심히 생각해보아야겠다.

github

• 코드 : https://github.com/shb03323/java-racingcar/tree/step2
• PR : https://github.com/woowacourse/java-racingcar/pull/627

마무리

구현 단계보다 리팩터링 단계에서 배운 것이 더 많은 것 같다. 리팩터링을 진행하면서 리뷰어님의 의견에 대해 계속 고민하고 리뷰어님의 의견이 납득될 때까지 자료를 찾아봐서 그런 것 같다. 덕분에 생각할 수 있는 범위가 한 층 넓어진 것 같고, 고려해야 하는 부분들에 대해 배울 수 있었다.