[우아한 테크코스] - 자동차 경주 미션 step 2 회고

jiny·2024년 2월 19일
우아한테크코스자동차 경주자바스크립트프런트엔드
0

우아한테크코스 6기

목록 보기
2/6
post-thumbnail

Intro

step 2가 끝나고 다시 회고 하게 되었다.

미션의 요구 사항은 크게 많지 않았지만, step 1에서 구현에 있어 아쉬웠던 점들을 리팩터링했고, step 2에서 제시하는 키워드 들을 스스로 확인해보며 리뷰어 님께 확인하는 형태로 진행하게 되었다.

모듈 네이밍 고려

step 1에서 대부분의 파일 네이밍이 directory/module.js의 형태로 전개 되었다.

그러자 리뷰어님께 이러한 리뷰를 받게 되었다.

directory/module.js로 전개한 것에 대한 특별한 이유는 없었고, import path가 /car/car.js로 되는 형태가 너무 이질적이라고 생각했기 때문이었다.

그래서, 리뷰어 님께 아래와 같은 질문을 하게 되었다.

리뷰어님이 OK 사인을 보내셨고, 나 또한 다시 생각해보았을 때 리액트의 폴더구조만 살펴봐도 컴포넌트에 대해 /directory/Directory.tsx와 같은 형태로 작성했었기 때문에 큰 고민 없이 변경할 수 있었다.

이런 과정을 통해 파일 네이밍을 할 땐, 일반적인 네이밍을 좀 더 찾아봐야 겠다는 생각을 하게 되었던거 같다.

무분별한 Object.freeze 줄이기

프로젝트에서 Object.freeze를 적용 할 때 기준이 따로 없이 객체라면 마구잡이로 사용했었다.

이유는 외부에서의 변경은 인위적으로 언제든지 발생할 수 있다고 생각했다. (예를 들면 특정 개발자가 아무 생각 없이 코드를 수정하는 경우 .. )

// App.js
const App = Object.freeze({
  async start() {
    await RacingGameController.run();
  },
});

특히 App의 경우 자동차 경주가 본격적으로 시작되는 코드 이기 때문에, 만약 외부에서 조작할 경우 앱이 아예 실행 조차 되지 않는 상황이 발생할 수 있다고 생각했다.

그래서 리뷰어님께 다음과 같은 리뷰를 받게 되었다.

나는 리뷰어님의 리뷰 중 중요한 정보에 포커스를 맞췄고 애플리케이션의 중요한 정보라면 비즈니스 로직이라고 생각했기 때문에, model layer 라고 생각해서 다음과 같이 질문 했고 OK 사인을 받을 수 있었다.

리뷰어 님의 말씀을 듣고, 조금 더 현실적으로 코드를 변경하는 case 들을 떠올려보게 되었다.

일반적으로, 리팩터링을 진행하게 된다면 아래와 같은 항목들을 확인하게 된다.

  1. 코드의 품질을 개선 한다.
  2. 요구 사항의 추가 혹은 변경이 발생했다.

model에 존재하는 비즈니스 로직들은 대부분 1, 2에 해당되기 때문에 Object.freeze를 통해 외부의 변화를 막아야 할 의무가 존재한다.

하지만, App의 경우, 컨트롤러의 메서드 네이밍을 수정해야 할 경우 아니면 거의 수정하지 않을 확률이 높다.

만약 수정한다고 해도, 내부 로직을 수정할 확률이 높지 외부에서 조작할 일은 거의 없을 것이다.

그래서, 리뷰어 님의 리뷰를 통해 어떤 상황에서 외부 변경을 방지하기 위한 방법 들을 고민하여 리팩터링 해볼 수 있었다.

단수형 모듈 VS 복수형 모듈

// Cars.js
import CarEngine from '../CarEngine/module.js';

class Cars {
  #racingCarDetails;

  constructor(racingCarNames) {
    this.#createInitRacingCarDetails(racingCarNames);
  }

  #createInitRacingCarDetails(racingCarNames) {
    this.#racingCarDetails = racingCarNames.map((carName) => ({ carName, moveCount: 0 }));
  }

  #updateMoveCounts(randomMoveCounts) {
    const racingCarDetailsCopy = this.#racingCarDetails.map((detail) => ({ ...detail }));

    randomMoveCounts.forEach((randomMoveCount, index) => {
      racingCarDetailsCopy[index] = CarEngine.triggerMove(racingCarDetailsCopy[index], randomMoveCount);
    });

    return racingCarDetailsCopy;
  }

  moveCars(randomMoveCounts) {
    this.#racingCarDetails = this.#updateMoveCounts(randomMoveCounts);

    return this.#racingCarDetails;
  }
}

export default Cars;

Cars 모듈은 아래와 같은 로직을 수행한다.

  1. 2차원 배열(시도 횟수, 자동차 이름 크기)의 랜덤 값을 받는다.
  2. 각 turn 마다 자동차를 이동 시킨다.
  3. 업데이트 된 각 자동차 정보가 담긴 배열을 반환 한다.
// CarEngine.js
const CarEngine = Object.freeze({
  triggerMove(carDetail, randomMoveCount) {
    if (randomMoveCount >= MIN_MOVABLE_VALUE) {
      carDetail.moveCount += CAR_MOVE_COUNT;
    }

    return carDetail;
  },
});

이 때, 각 자동차를 이동 시킬 땐 CarEngine의 triggerMove를 통해 조건에 부합되는 자동차만 이동 시킨다.

const RacingGame = Object.freeze({
  startRace({ racingCarNames, tryCount, randomMoveCounts }) {
    const cars = new Cars(racingCarNames);

    const racingResult = Array.from({ length: tryCount }, (_, index) => cars.moveCars(randomMoveCounts[index]));

    return racingResult;
  },
});

RacingGame에서는 각 turn의 결과를 총 tryCount 만큼 지속하여 그 결과를 반환함으로서 자동차 이동 기능을 수행하게 된다.

나는 이 모듈 들을 작성하며, Cars(각 turn 마다 자동차를 이동)와 RacingGame(시도 횟수 만큼의 레이싱 결과 반환)의 역할이 명확하다고 생각했고, 실제로 하나의 메시지 만을 가지기 때문에 테스트 코드도 매우 명확하게 나온 편이었다.

하지만 리뷰어님에게 복잡도를 이유로 단수형 모듈을 만드는 것을 제안해주셨다.

그 이유를 확인해보고 싶어 관련 자료를 검색해보았고 Classes naming: singular or plural? 라는 옛날 자료에서 확인해볼 수 있었다.

요약하자면 데이터 모델링을 할 때 복수형 보단 단수형을 사용하는 것이 더 직관적이라는 것이었다.

나 또한 이러한 답변 들을 읽으며, 데이터를 모델링 할 때 보통 사람들이 아닌 사람으로 모델링 하는 것이 맞다는 생각을 하게 되었다.

그러므로 비슷한 맥락으로 자동차들이 아닌 자동차로써 모델링 하는 것이 더 직관적이라고 느껴졌다.

// Car.js
class Car {
  static #MIN_MOVABLE_VALUE = 4;

  static #CAR_MOVE_COUNT = 1;

  #carDetails;

  constructor(carName) {
    this.#carDetails = {
      carName,
      moveCount: 0,
    };
  }

  move(randomMoveCount) {
    this.#updateMoveCount(randomMoveCount);

    return { ...this.#carDetails };
  }

  #updateMoveCount(randomMoveCount) {
    if (this.#isMovable(randomMoveCount)) {
      this.#carDetails.moveCount += Car.#CAR_MOVE_COUNT;
    }
  }

  #isMovable(randomMoveCount) {
    return randomMoveCount >= Car.#MIN_MOVABLE_VALUE;
  }
}

export default Car;
// RacingGame.js
import Car from '../Car/Car.js';

class RacingGame {
  #tryCount;

  #cars;

  constructor({ racingCarNames, tryCount }) {
    this.#tryCount = tryCount;
    this.#cars = racingCarNames.map((carName) => new Car(carName));
  }

  startRace(randomMoveCounts) {
    const racingResult = Array.from({ length: this.#tryCount }, (_, racingTurn) =>
      this.#updateRacingResult(racingTurn, randomMoveCounts),
    );

    return racingResult;
  }

  #updateRacingResult(racingTurn, randomMoveCounts) {
    const partialRacingResult = this.#cars.map((car, carIndex) => {
      const randomMoveCount = randomMoveCounts[racingTurn][carIndex];

      const updatedCarDetail = car.move(randomMoveCount);

      return updatedCarDetail;
    });

    return partialRacingResult;
  }
}

export default RacingGame;

실제로 구현을 해볼 때, 코드를 읽기 더 수월하다고 느껴졌으며, 코드 복잡도도 더 낮아졌다는 생각이 들었다.

또한, 자동차들은 이동해서 결과를 반환한다. 단, 각 자동차는 조건에 따라 이동한다. 보다 각 자동차 들은 조건에 맞게 이동하여 그 결과를 반환한다. 라는 맥락으로 변경된다는 느낌이 들어 더 자연스럽다고 느껴졌다.

Shuffle 내부 로직 변경

shuffle(array) {
  return array.sort(() => Math.random() - 0.5);
}

리뷰어님 께서 shuffle 로직에 대한 피드백을 주셨다.

사실 요구 사항에 대한 고민을 더 하고 싶었기 때문에, shuffle의 경우 @woowacourse/mission-utils의 내부 로직을 아무 생각 없이 사용했었다.

피드백을 받고 셔플 내부 로직을 찾아보니 다양한 방식의 셔플 알고리즘이 있었다.

사용했던 셔플 알고리즘은 random sort 방식으로, sort 내 콜백이 양수라면 오름차순으로, 음수를 리턴하면 내림차순으로 정렬되가며 셔플 되는 로직이다.

random sort의 문제점은 sort 메서드 자체가 이러한 목적으로 설계된 함수가 아니다 보니 예상치 못한 결과를 낳을 수 있었다.

const arr = [1, 2, 3];
 
function shuffle(array) {
  array.sort(() => Math.random() - 0.5);
}
 
let count = {
  '123': 0,
  '132': 0,
  '213': 0,
  '231': 0,
  '321': 0,
  '312': 0
};
 
for (let i = 0; i < 1_000_000; i++) {
  let array = [1, 2, 3];
  shuffle(array);
  count[array.join('')]++;
}
 
/**
123: 375285,
132: 62554,
213: 125270,
231: 62429,
312: 62557,
321: 311905
*/

실제로 random sort를 돌려보았을 때 결과가 균일하지 않고 한 곳에 몰리는 것을 알 수 있었다.

또한, sort의 시간 복잡도는 O(n log n)이기 때문에 느린 축에 속한다.

shuffle(array) {
  for (let currentIndex = array.length - 1; currentIndex > 0; currentIndex -= 1) {
    const randomIndex = Math.floor(Math.random() * (currentIndex + 1));

    swap(array, currentIndex, randomIndex);
  }

  return array;
}

그래서 피셔-에이츠 셔플 알고리즘으로 수정하였다.

피셔-에이츠 알고리즘의 로직은 아래와 같이 전개 된다.

  1. 1 ~ N까지 수 중 임의의 숫자를 선택한다.
  2. 1과 선택되지 않고 남아 있는 수 사이의 임의의 수 k를 선택한다.
  3. k를 선택되지 않은 마지막 숫자와 자리를 바꾼다.
  4. 1부터 N-1까지의 수 중 임의의 숫자를 선택한다.
  5. 더 이상 선택할 숫자가 없을 때까지 2~4번의 과정을 반복한다.

해당 알고리즘은 O(n)의 시간 복잡도를 가지기 때문에 성능을 개선할 수 있었고, 편향되지 않는 빈도로 shuffle도 가능했다.

또한, 라이브러리 내 로직을 사용할 땐 생각 없이 사용하기 보단 항상 더 나은 방법이 있는지를 고민해야겠다고 느꼈다.

끝으로

드디어 온보딩 미션인 자동차 경주가 끝이 났고, 내일 부터 로또 미션을 진행한다.

로또 미션은 TDD와 리팩터링이 핵심 키워드인 만큼 효과적인 TDD와 리팩터링에 대해 살펴보고 적용해봐야겠다.

profile
jiny
FE
이전 포스트

[우아한 테크코스] - 자동차 경주 미션 step 1 회고

다음 포스트

[우아한 테크코스] - 로또 미션 step 1 회고

0개의 댓글