[NextStep]눈물의 리팩터링

pengooseDev·2023년 8월 9일
2
post-thumbnail

눈물의 피드백

피드백을 받고 3일 정도 코드를 작성할 수 없었다.
코드를 잘 작성했다고 생각했지만, 내가 작성한 코드는 말 그대로 "코드 덩어리"일 뿐, 객체란 무엇인가?에 대한 충분한 고민이 이루어지지 않았다는 것을 깨닫고 모종의 죄책감이 발생했기 때문이다.

코드 작성을 멈추고, 피드백을 기반으로 3일 내내 "객체"의 의미와 내 코드의 문제점을 분석했다.


객체

객체는 유기체이다. 각 객체는 어느정도의 무결성을 지녀야 하며, 객체는 모여서 각자의 맡은 책임과 역할을 수행해야 한다.

필자의 코드는 그렇지 못했다. 모든 객체가 서로를 도우며, 하나의 유기체가 되어가는 것이 아니라, 그저 잘 작동하는 코드 덩어리에 불과했다.


문제점1 : 도메인 및 Model의 변경을 고려하지 않음.

필자의 코드는 변경에 취약한 코드였다.
다음은 Controller 내부에서 User에게 자동차 이름을 입력받고 RacingGame의 도메인에 이를 넘겨주는 일련의 과정이다.

read => validation => setter로 값 전달


export class GameController {
  #model;
  #view;

  constructor(model, view) {
    this.#model = model;
    this.#view = view;

    this.#readCarName();
  }

  #readCarName() {
    this.#view.readCarName((userInput) => this.#validateCarName(userInput));
  }

  #validateCarName(userInput) {
    try {
      const carNames = splitCarNameToArray(userInput);
      carNames.forEach(validateCarName);

      this.#handleCarNameInput(carNames);
    } catch (error) {
      this.#handleError(error, this.#readCarName.bind(this));
    }
  }

  #handleCarNameInput(carNames) {
    this.#model.setCars(carNames);

    this.#readTotalRound();
  }
  // ... coeds
}

이렇게 유효성 검증이 진행된 값은 아래와 같은 방식으로 RacingGame의 도메인에 할당되었다.


export class RacingGame {
  #cars = RACING_GAME.CARS.DEFAULT_STATE;
  #totalRounds;
  #gameProgress = RACING_GAME.PROGRESS_TITLE;
  #gameResult;

  constructor() {}

  setCars(carNames) {
    this.#cars = carNames.map((carName) => new Car(carName));
  }

  // ...codes
}

위 코드에서 문제점은 다음과 같다.

  • RacingGame에서 Car가 아닌, Bike를 이용한 경주라면?
  • RacingGame에서 탈 것은 동적으로 결정해야 한다면?
  • setter를 제공한다는 것은 예상치 못한 값의 변경으로 위험성이 증가한다는 것.

S1 : 객체는 조금 더 객체답게

각 객체와 도메인의 모든 것이 변할 수 있음을 고려하고 필드명과 메서드명의 네이밍을 변경하고, 추상화를 다시 진행하였다.

Racing Game에서 사용되는 Model(객체)은 해당 Layer에서 할당하는 방식이 아니라, 상위 Layer에서 주입하는 방식으로 구현하여, 문제를 해결할 수 있었다.

export class GameController {
  #model;
  #view;

  constructor(view) {
    this.#view = view;

    this.#setGameConfig();
  }

  async #setGameConfig() {
    const cars = await this.#createCarByUserInput();
    const totalRound = await this.#readTotalRound();
    this.#model = new RacingGame(cars, totalRound);

    this.#printGameResult();
  }
  
  async #createCarByUserInput() {
    try {
      const userInput = await this.#view.readCarName();
      const carNames = splitCarNameToArray(userInput);
      Validation.validateCarNameInput(carNames);
      const cars = carNames.map((carName) => new Car(carName));

      return cars;
    } catch (error) {
      return this.#handleError(error, () => {
        return this.#createCarByUserInput();
      });
    }
  }
  // ...codes
}

setter를 제거하고 생성자의 매개변수로 필요한 값들을 넘겨주는 방식으로 변경하였다.

export class RacingGame {
  #racers;
  #totalRound;
  #gameProgress = RACING_GAME.PROGRESS_TITLE;
  #gameResult;

  constructor(racers, totalRound) {
    this.#racers = racers;
    this.#totalRound = totalRound;

    this.#startRace();
  }
  // ... codes
}

코드는 다음과 같이 변경되었다.

  • 비동기를 제어하는 방식을 콜백이 아닌 Promise 방식으로 변경. (아래에서 다룰 예정)
    "입력"과 "코드의 흐름"에 대한 책임 분리.
  • setter 제거
    예상치 못한 변경을 제거하여 객체 및 코드의 안정성 확보.
  • 각 필드의 네이밍 변경
    더 이상 "자동차 경주 게임"에 국한된 것이 아닌 "경주 게임"으로 존재할 수 있게 되었다.

P2 : 단일 책임 원칙에 위배되는 JS의 콜백방식

비동기적으로 유저의 입력을 제어하는 방식에 콜백 함수를 채택하는 것은 단일책임 원칙에 위배된다고 생각했다.

콜백 함수로 제어할 경우, "입력"의 책임 뿐 아니라 "코드 흐름 제어"의 책임까지 담당하기 때문이다. 코드 흐름 관련된 로직이 View에 결합되어 복잡도가 올라가고 코드를 관리하기 어려워진다는 것이다.

물론, 다음 콜백의 매개변수로 이전의 입력값들을 넘겨주는 방법이 있다.
하지만, 입력값이 5개가 넘어간다면, 모든 method에 높은 결합도가 발생한다. 이는 마치 ReactJS의 PropsDrilling와 같다.

우선, 리팩터링을 하기 전. setter를 사용함으로서 해당 문제를 해결했다.


export class GameController {
  #model;
  #view;

  constructor(model, view) {
    this.#model = model;
    this.#view = view;

    this.#readCarName();
  }

  #readCarName() {
    this.#view.readCarName((userInput) => this.#validateCarName(userInput));
  }

  #validateCarName(userInput) {
    try {
      const carNames = splitCarNameToArray(userInput);
      carNames.forEach(validateCarName);

      this.#handleCarNameInput(carNames);
    } catch (error) {
      this.#handleError(error, this.#readCarName.bind(this));
    }
  }

  #handleCarNameInput(carNames) {
    this.#model.setCars(carNames);

    this.#readTotalRound();
  }

  #readTotalRound() {
    this.#view.readTotalRound((userInput) =>
      this.#validateTotalRound(userInput)
    );
  }

  #validateTotalRound(userInput) {
    try {
      validateTotalRounds(userInput);

      this.#handleTotalRoundInput(userInput);
    } catch (error) {
      this.#handleError(error, this.#readTotalRound.bind(this));
    }
  }

  #handleTotalRoundInput(userInput) {
    this.#model.setTotalRounds(userInput);

    this.#startRacingGame();
  }
  // ... codes
}

그 결과 위와 같은 참사가 벌어진다.

  • propsDrilling을 해결하기 위해, model에 setter 추가
  • 코드의 흐름과 state의 높은 결합도 발생
  • setter를 위한 Controller의 상위 Layer에서 Model을 반드시 DI해야함.
  • 위와 같은 Controller 상위 하위 Layer에 부정적인 sideEffect 발생

S2 : Promise 도입

Promise를 도입하여 이러한 책음을 분리하였다.


export class GameController {
  #model;
  #view;

  constructor(view) {
    this.#view = view;

    this.#setGameConfig();
  }

  async #setGameConfig() {
    const cars = await this.#createCarByUserInput();
    const totalRound = await this.#readTotalRound();
    this.#model = new RacingGame(cars, totalRound);

    this.#printGameResult();
  }

  async #createCarByUserInput() {
    try {
      const userInput = await this.#view.readCarName();
      const carNames = splitCarNameToArray(userInput);
      Validation.validateCarNameInput(carNames);
      const cars = carNames.map((carName) => new Car(carName));

      return cars;
    } catch (error) {
      return this.#handleError(error, () => {
        return this.#createCarByUserInput();
      });
    }
  }

  async #readTotalRound() {
    try {
      const userInput = await this.#view.readTotalRound();
      Validation.validateTotalRounds(userInput);

      return userInput;
    } catch (error) {
      return this.#handleError(error, () => {
        return this.#readTotalRound();
      });
    }
  }
  // ... codes
}

변경된 부분들은 다음과 같다.

  • View로부터 코드 흐름 제어 로직 제거
  • DI 받던 Model(RacingGame) 제거 및 Controller 내부에서 할당 방식으로 변경
  • 불필요한 Model의 setter 제거
  • Controller 내부 메서드들의 결합도 제거 및 가독성 개선

Layer별 Validation 재분배

이 또한, 객체의 책임과 맞닿아있는 부분이다.
Controller에 뭉쳐있던 Validation을 잘게 쪼개어 View(공백처리), Car Model(자동차 관련 Validation), Controller(중복된 자동차 이름)에 나누어 처리하였다.


merge! 🥳

아직 CLI 기반으로 TC와 코드를 작성하다보니, View와 관련하여 이해하지 못한 부분이 있는 것 같다. 인성님이 일전 피드백에서 "웹 기반 미션을 진행하면 머리가 반으로 쪼깨질 것"이라고 조언해주셨다 ㅋㅋㅋ

빠르게 웹 기반 Lotto 미션을 진행하고, 다시 고민해보아야 할 것 같다. 😀
많은 가르침을 주신 인성님! 감사합니다!! 😆

0개의 댓글