[코드리뷰] Level 1 -자동차 경주

우테코 첫 과제로는 프리코스때 진행 했던 자동차 경주를 페어프로그래밍 하는 것이었다.
초반에 구조를 잡지 않고 시작해서 중간에 많이 애먹었지만 구현을 목적으로 코딩하여 시간 안에 완성할 수 있었다.

Step1 코드리뷰

• 기존 클래스 명확하게 수정하기
  기존에 Move 라는 클래스에 자동차의 이름 정보, 전진 정보를 담고 있어 모두 포함하는 네이밍(MoveCarInfo)으로 수정
• 테스트를 할 때 모킹함수를 최대한 작성하지 않도록 도메인 코딩하기

	  move() {
    if (Random.randomNum() >= 4) {
      return this.#moveTrace.push(true);
    }
    return this.#moveTrace.push(false);
  }

->

  move(random) {
    if (random >= 4) {
      return this.#moveTrace.push(true);
    }
    return this.#moveTrace.push(false);
  }

move 라는 함수를 테스트 하기 위해 randomNum 이라는 함수를 모킹해야했는데, 그렇게 하기 위해 *번거로운 작업을 거쳤다. 모킹함수를 없애기 위해 random 을 인수로 넘겨 모킹하지 않도록 수정했다.

• *Random.randomNum = jest.fn();
  이 부분에서 randomNum is readOnly 라는 에러 발생한 이유가 무엇일까?(해당 함수를 클래스로 수정 후 내부에 static 함수로 만들어주어 겨우 테스트를 통과시킬 수 있었다..)
  ->
  jest 에서 esm 환경에서 함수를 모킹하기 위해서 Random.randomNum = jest.fn(); 와 같이 특정 함수만 모킹함수로 대체하는 방법은 허용하지 않는다고 함.

  😊 💬 모킹하기 위해서는 import 해주는 모듈 전체를 함께 모킹해야 동작할 것 같아요
  stackoverflow 참고하기!

• 단위 테스트에서 입출력 테스트까지는 할 필요 없다.
  입출력이(UI부분이) 단위 테스트가 필요 없는 이유에는 단순히 사용자에게 입력받는 부분, 혹은 콘솔로 찍어내는 부분이라 임의에 따라 변할 수 있기도 하고, 단위테스트 자체가 주요 기능(도메인 부분)이 잘 돌아가는지 확인하기 위함 이라서 도메인과 분리되어있는 view 부분을 테스트하지 않아도 된다고 생각한다.

  😊 💬 입출력, 특히나 콘솔기반으로 입력을 받고 출력을 하는 부분은 자바스크립트 노드 환경 자체의 기능이기 때문에, 테스트를 하는 의미가 없어 보임. 입출력의 책임을 온전히 노드 환경에 맡기려면 입출력(UI)과 도메인의 책임이 완전히 분리되어야 할 것.따라서 개발자가 테스트해야 할 대상은 개발자가 직접 작성한 도메인 영역 뿐이라고 생각함.
  -> 외부환경과 내가 작성한 코드의 분리!!내가 작성한 코드만 테스트!!

• 테스트코드는 이 코드가 어떤 기능을 하는지 최대한 구체적으로 작성하기. 어떤 상황에서 어떤 결과가 일어날지 적기!

• 경계값 테스트
  테스트 할 때는 범위 내의 숫자 모두를 사용하기보다 경계값만 테스트 해줘도 충분하다고 생각.
  

• 추후 확장 가능성이 있는 부분은 함수로 나눠주기~

• (페어가 받은 리뷰) map은 배열의 각 요소를 변환하여 새 배열을 생성할 때 사용함. 그래서 단순 순환을 할 때는 forEach 사용하기!

step2 코드리뷰

• 패키지 매니저 통일하기.
  기존에 설치되어 있던 패키지 매니저는 yarn 이었는데 npm 을 사용해서 섞여있었음. -> packagelock.json 지우고 yarn 으로 통일
  yarn, npm , pnpm 비교하기
  

• boolean 값이 반환되는 함수의 경우 앞에 is, has, should, can 등의 접두사 붙이기

• 리스트 네이밍 고민해보기

ㄴ 멘토님이 보여주신 토스페이먼츠 테크피드에서 올라온 논의 내용
내가 네이밍 한 것을 보면 -List 와 -s 둘 다 사용하고 있어서 하나로 통일해야겠고, api 만큼 타입 변경이 큰 영향을 미치지는 않겠지만 도메인 관점에서 네이밍을 하는게 좋다 라는게 인상깊었던것같다.

• input 파일 안에서 car 인스턴스를 생성 후 validation 을 해줬었는데, 리뷰어님께서 validation 같은 기능들은 컨트롤러가 하는 기능 인 것 같다 라는 조언을 받았다.

  😊 💬 현재는 해시의 input 클래스가 컨트롤러의 역할까지 일부분 부담하고 있는 것 같아보입니다.
  저는 컨트롤러의 역할은 말그대로 UI 와 도메인을 연결해주는 역할이라고 생각하고 UI 클래스들의 역할은 단순히 사용자 인터페이스와 관련된 역할(입,출력)에만 제한되어야 한다고 생각합니다.
  view 에서 도메인 하위 모듈에 접근하고 있는 부분도 잘못된 방향의 참조인 것 같고 , 어찌보면 input 의 유효성 검증도 도메인 영역과 관련된 것이기 때문에, 유효성 검증 처리는 외부에서 해주도록 리팩토링 해보면 좋을 것 같다고도 생각이 들어요.

그래서 input 에 있던 유효성검사를 raceController 의 input 메소드에서 검사할 수 있도록 수정해주었다.

// input 클래스
class Input {
  static carNameInput = async () => {
    const carList = await ReadLine.readLineAsync(INPUT_CAR_NAMES);
    return carList;
  };

  ...
  
  
// raceController 클래스 
  async race() {
    const { cars, tryNumber } = await this.#input();
    this.#controlCarMove(cars, tryNumber);
    const raceWinner = new RaceWinner(this.#carsMoveInfoList).getRaceWinner();
    this.#print(tryNumber, raceWinner);
  }

  async #input() {
    const cars = await this.#carsInput();
    const tryNumber = await this.#tryNumberInput();
    return { cars, tryNumber };
  }

  async #carsInput() {
    try {
      const carList = await Input.carNameInput();
      const cars = this.#carsValidation(carList);
      return cars;
    } catch (error) {
      console.log(error.message);
      return this.#carsInput();
    }
  }