[클린코드 5기 손진영] 자동차 경주 미션 STEP 2

[jinyoung234]

안녕하세요 리뷰어님 제가 이번 스탭에서 도전해본 것들은 다음과 같습니다!

도전 내용

테스팅

• jest.fn 사용하지 않고 테스팅 하기
• each 메서드를 통해 여러 테스트 케이스로 검증하여 탄탄한 테스트 코드 만들기

re-export에 대한 고민

무분별한 re-export에 대해 리뷰어님도 언급해주셨던 만큼 다시 한번 더 고민해 보았습니다. 제가 생각한 장점과 단점은 다음과 같습니다.

장점

• 가독성이 좋아진다.

단점

• re-export가 많아지면 트리쉐이킹에 좋지 않다.
• 너무 무분별한 re-export는 가독성에 좋지 않다.

// re-export
import {
 AVALIABLE_RANDOM_NUMBER,
 CAR_MAX_LENGTH,
 CAR_MIN_LENGTH,
 ERROR_MESSAGE,
 INPUT_MESSAGE,
 SEPERATOR_SYMBOLS,
} from '../src/constants';

// no re-export
import { ERROR_MESSAGE, INPUT_MESSAGE } from '../src/constants/message.js';
import { AVALIABLE_RANDOM_NUMBER } from '../src/constants/randomNumber.js';
import { SEPERATOR_SYMBOLS } from '../src/constants/commons.js';
import { CAR_MAX_LENGTH, CAR_MIN_LENGTH } from '../src/constants/validate.js';

장단점을 다시 한번 고려해보았을 때, 역할에 맞게 파일을 네이밍 하는 constants와 utils의 경우 re-export 하는 방식이 가독성이 떨어진다고 생각되어 re-export를 없애는 방식으로 변경을 진행했습니다!

named-export로 변경

model, controller와 같은 class는 default export로 선언했었지만 저번 리뷰 때 리뷰어님의 의견 + 학습 한 내용들을 고려해볼 때 named-export로 통일하는게 맞다고 생각되어 named-export로 통일하였습니다.

static field & method에 대한 고민

위 키워드들에 대해 아티클 디깅이 더 필요할 것 같다고 리뷰어님이 말씀해주셔서 static 키워드를 중점으로 다시 한번 더 고민을 해보았습니다. static 키워드를 사용하는 경우를 정리해봤습니다.

• 인스턴스 내 field를 참조하지 않는 경우
• 인스턴스 내 helper 함수로 사용할 경우

2가지 case로 쪼개어 보았을 때 controller 내 유저의 입력 값을 받는 로직이 helper 함수가 아닌 controller의 역할과 관련되어 있는 로직인 것을 인식했고 view를 controller 내 필드로 추가하여 static 키워드를 사용하지 않는 방향으로 변경했습니다.

validate 책임을 inputView에서 controller, model에서 검증하는 것으로 변경

inputView에서 가장 먼저 입력 값을 먼저 얻어내다보니 그 값에 대한 검증을 inputView에서 하는 것이 변경 된 부분이 외부로 퍼질 위험성이 가장 적다고 생각했지만 관련 영상과 아티클, 다른 분들의 코드를 분석 해보았을 때 controller에서 한번 검증 후 model에서 검증(입력 데이터를 사용하는 레이어에서 한번 더 검증)하는 방식이 더 좋은 방식이라고 생각했고 이유는 다음과 같습니다.

• controller의 경우 view, model을 제어하는 역할로써, view로 부터 받은 데이터를 검증하는 것이 “제어”의 역할에 맞는 책임이다.
• model에서 controller가 넘겨주는 데이터에 대해 다시 한번 검증 하는 것도 필요하다.
• 사용자가 잘못된 입력 값을 작성한 경우 에러 메시지를 보여주고, 다시 입력할 수 있게 한다. 요구사항을 대응하려면 controller를, 테스트 코드를 작성하려면 model에 검증 로직이 필요하다.

위와 같은 이유로 변경을 진행하였습니다!

inputview, outputview를 하나의 view 클래스에서 관리

이번 step에서 2개의 view 객체를 하나의 view 클래스로 관리하는 방식으로 변경해보았습니다. 변경한 이유는 다음과 같습니다.

• inputview, outputview는 모두 view와 관련된 역할에 엮어있는 클래스들이라고 생각했습니다.
• controller에 새로운 view 객체를 추가해야 하는 상황이 온다면 view 내에서 관리가 가능하여 유연하게 대처할 수 있을거라고 생각했습니다.
• view 내 메서드들을 사용하는 controller 입장에서 view의 구체적인 역할에 신경쓰지 않고 선언적으로 메서드 사용이 가능할 것이라고 생각했습니다.

또한, 이번 도전 내용을 통해 궁금한 점은 다음과 같습니다.

질문

validate의 static 키워드 사용

export class RacingGame {
  constructor(carNames, inputCount) {
    RacingGame.#validateCount(inputCount);
    this.#racingCount = Number(inputCount) || INIT_RACING_COUNT;
    this.#racingCars = new RacingCars(carNames, NumberMaker);
    this.#racingWinners = new RacingWinners();
    this.#racingResult = [];
  }

  static #validateCount(count) { // validate logic }
}

validation과 관련된 로직을 static 키워드로 선언했는데, 아무래도 helper 함수의 성향 이라기 보단 RacingGame 내에서 데이터를 검증한다"는 RacingGame의 역할과 유사하다는 생각이 들어 로직을 작성 하고도 찝찝함이 느껴졌습니다.

export class RacingGame {
  constructor(carNames, inputCount) {
    this.#racingCount = Number(inputCount) || INIT_RACING_COUNT;
    this.#racingCars = new RacingCars(carNames, NumberMaker);
    this.#racingWinners = new RacingWinners();
    this.#racingResult = [];
    this.#validateCount(inputCount);
  }

  #validateCount() { 
       const racingCount = this.#racingCount;
       if (!isNumber(racingCount)) throw new TypeError(ERROR_MESSAGE.AVALIABLE_NUMBER);
   }
 }

이런 방식으로도 코드를 짤 수 있을 거 같지만 그렇게 되면 검증 하기 전 필드 값이 오염되기 때문에 좋지 않은 방법이라고 느껴졌습니다. static 키워드를 사용하는게 옳은 방식인지 아니면 더 좋은 방법이 있는지 궁금합니다.

controller 내 validate 방식

async #getRacingCarNames() {
  return this.#retryOnErrors(async () => {
    const racingCarNames = await this.#view.inputByUser(INPUT_MESSAGE.RACING_CAR);
    GameController.#validateCarNames(racingCarNames);
    return racingCarNames;
  });
}

async #getRacingCount() {
  return this.#retryOnErrors(async () => {
    const racingCount = await this.#view.inputByUser(INPUT_MESSAGE.COUNT);
    GameController.#validateCount(racingCount);
    return racingCount;
  });
}

async #settingRacingGame() {
  return this.#retryOnErrors(async () => {
    const racingCarNames = await this.#getRacingCarNames();
    const racingCount = await this.#getRacingCount();
    this.#racingGame = new RacingGame(racingCarNames, racingCount);
  });
}

각각 자동차 이름, 카운트를 입력받아 반환하는 getRacingCarNames, getRacingCount(입력 값에 대한 예외처리) 그리고 입력값을 토대로 RacingGame을 세팅하는 settingRacingGame(model에서 발생될 수 있는 예외 처리)에 try-catch 문을 사용해서 에러를 처리했습니다. model을 검증하기 위해 settingRacingGame에 try-catch를 사용했는데 이런 과정이 불 필요한 과정 이었는지 여쭤보고 싶습니다..!

또한, getRacingCarNames, getRacingCount이 너무 많은 역할에 종속되어 있는 것도 인지하고 있지만, 이러한 단점 보단 사용자가 잘못된 입력 값을 작성한 경우 에러 메시지를 보여주고, 다시 입력할 수 있게 한다.라는 요구사항을 충족 하기 위해 view에서 검증하거나 controller에서 view에게 값을 받아오는 과정에서 try-catch를 사용해야 하는 것이 최선이라고 생각했고 controller의 원래 책임에 맞게 controller에서 처리하였습니다. 이 방향성 자체도 잘못 된건지 궁금합니다..!

[igy95]

안녕하세요 진영님~ 1단계에 비해 PR에 코드 작성 의도, 이에 관련된 질문을 세심히 작성해주셔서 저도 좀 더 집중적으로 볼 수 있었습니다. 👍 한가지 말씀드리고 싶은 부분은, 대체적으로 수정방향을 살펴 보았을 때 어떤 개념에서 A vs B로 대치되는 부분이 있을 때 그 중 하나로만 통일하시려는 기조가 있으신 것 같아요.

예를 들어 모든 모듈을 named export로 바꿔주셨는데, 편의성 + semantic의 이유로 하나의 모듈에서 주요한 모듈 하나만 export하는 경우에 default export를 사용하기도 합니다. (다른 라이브러리를 사용할 때 import path를 보며 어떤 모듈은 어떻게 가져오는지 참고해보시는 것도 추천드려요)

그래도 지금처럼 개념을 깊게 파보며 코드에 적용하려는 자세는 계속 가져가시면 좋을 것 같습니다 😀 질문에 대한 답변 남기고 이만 코멘트 마치도록 할게요~

validate의 static 키워드 사용

올려주신 예시 코드에서 메서드의 순서를 바꾸면 적절한 동작을 수행하지 않을까 싶네요. 그리고 검증이 실패했을 때 에러를 던질 것인지, fallback 값을 채워줄 것인지 하나를 선택하는 것이 좋을 듯 합니다!

export class RacingGame {
  constructor(carNames, inputCount) {
    this.#validateCount(inputCount);
    this.#racingCount = Number(inputCount) || INIT_RACING_COUNT;
  }

  #validateCount(count) { 
       if (!isNumber(count)) throw new TypeError(ERROR_MESSAGE.AVALIABLE_NUMBER);
   }
}

controller 내 validate 방식

input에 대한 validate 흐름, 함수를 나눈 부분에 대해서는 크게 어색하지 않다고 생각합니다. 다만 함수 역할에 비해 getSomething의 네이밍은 조금 좁게 느껴지네요! 유저의 input으로부터 값을 가져오는 역할을 담당하는 함수임을 알 수 있도록 이 부분만 수정해보면 어떨까 싶습니다~

[igy95]

현재는 자동차 경주라는 하나의 도메인을 가진 작은 앱을 만드는 미션이었기 때문에 폴더 구조의 확장성을 크게 신경쓰지 않아도 되어 굳이 코멘트를 남기지 않았지만, 나중에 여유가 되신다면 이에 대한 고민도 한번 해보시면 좋을 듯합니다. 만일 어떠한 프로젝트에서 두 개 이상의 도메인을 다루어야 한다면 현재의 폴더와 파일 구조는 확장이 용이할까요? 예를 들어 domainA, domainB, 두 도메인에서 공통으로 사용하는 validator가 모두 따로 있다면 현재의 파일 구조에서 어떤 식으로 코드를 작성할 수 있을까요?

[jinyoung234]

리뷰어님의 질문에 대해 mvc 패턴을 동일하게 사용한다는 전제 하에, javascript에서 mvc를 사용하는 nest.js의 폴더 구조를 살펴보며 답을 찾아보려고 했습니다!

📦src
 ┣ 📂domainA
 ┃ ┣ 📂 view
 ┃ ┣ 📂 model
 ┃ ┣ 📂 controller
 ┃ ┗ 📂 validator
 ┣ 📂domainB
 ┃ ┣ 📂 view
 ┃ ┣ 📂 model
 ┃ ┣ 📂 controller
 ┃ ┗ 📂 validator
 ┗  // ..

도메인을 나눈다는 것은 규모가 커짐에 따라 하나의 최상위 폴더로는 감당할 수 없기 때문에 나누는 것이라고 이해했습니다. 따라서, 하나의 validator로는 서로 다른 도메인의 책임 들을 담아내기에 부족할 것이라고 생각했고, 도메인에 대한 별도의 validator를 추가하거나 validator 없이 model 내에서 자체적으로 검증할 수 있도록 코드를 작성 해볼 것 같습니다.

이러한 형태의 폴더 구조를 만들었을 때 이점은 도메인 별로 mvc를 관리하기 때문에 관심사의 분리가 확실히 이뤄졌다고 생각했으며, 유지보수 하기도 편할 것 같다는 생각입니다!

[igy95]

넵 좋은 의견인 것 같습니다. 물론 오버 엔지니어링이라고 판단되면 실제 문제가 닥쳤을 때 구조를 변경하는 방향도 생각해볼 수 있겠으나, 시간이 지남에 따라 수정이 명백한 지점이라면 처음부터 확장이 용이한 구조로 작성하는 것도 좋을 것 같아요. 그런 방법론들이 쌓여야 전반적으로 더 빠르고 좋은 코드를 작성할 수 있고 그것이 곧 개발자의 실력이 되는 것이라고 생각합니다!

[igy95]

이와 관련해서는 modern frontend architecture라는 키워드로 구글링해보시거나 아래의 아티클을 참고 삼아 읽어보시는 것도 추천드립니다

https://ahnheejong.name/articles/package-structure-with-the-principal-of-locality-in-mind/

[jinyoung234]

아티클 공유 감사 합니다 👍 꼭 읽고 정리해보겠습니다 !!

[igy95]

어떤 코드의 집합을 클래스로 만든다는 것은, 1) 여러 인스턴스가 필요하고 2) 인스턴스 내에서 공유해야할 상태 혹은 3) 인스턴스 간 공유해야할 메서드가 필요한 상황을 기대해볼 수 있겠습니다. 그렇다면 View라는 클래스를 예시로 들면 이것을 클래스로 두는데 얻는 이점은 무엇이 있을까요?

또한 어느정도 규모가 있는 프로덕트 내의 view는 쉽게 몇 백개가 넘어갈 수 있을 텐데 현재와 같은 구조가 된다면 View의 크기는 금방 비대해지지 않을까요?

[jinyoung234]

제가 View를 클래스로 두려고 했던 이유는 다음과 같습니다!

1. 관심사를 명확히 분리 하여 controller 내 InputView, OutputView가 아닌 다른 View가 필요할 것이라고 할 때 controller에서 변경(case 1) 하는 것이 아닌 View 내에서 변경(case 2)할 수 있게 하였습니다.

// case 1
export class GameController {
  constructor() {
    this.#inputView = InputView;
    this.#outputView = OutputView;
    this.#specificView = SpecificView;
  }
}


// case 2 
export class View {
  constructor() {
    this.#inputView = InputView;
    this.#outputView = OutputView;
    this.#specificView = SpecificView;
  }
}

export class GameController {
  constructor() {
    this.view = new View();
  }
}

view의 새로운 의존성을 controller에 주입하는 방식(DI)를 고려(case 3)해보기도 했지만, 그렇게 해도 App 클래스에서 추가하기 때문에 이것 보단 View 에서 추가해주는 것이 맞을거 같다고 생각했습니다.

// case 3
export class GameController {
  constructor(inputView, outputView) {
    this.#inputView = inputView;
    this.#outputView = outputView;
  }
}

export class App {
  constructor() {
    this.controller = new GameController(InputView, OutputView);
  }
}

2. View가 대체로 도메인에 따라 달라 지는 것이 아닌 모든 도메인에서 공통적으로 사용하는 클래스 라고 생각했습니다. 그래서 View가 몇백개 까지 생길것이라고 생각하지 못했던거 같습니다. 만약 cli 기반이 아닌 gui 관련 애플리케이션이 된다면 충분히 비대해 질 수 있을 것 같습니다.

[igy95]

• 1번에 대한 이유는 단순히 Controller와 View의 레이어를 나누는 것만으로도 해결이 될 수 있을 것 같아 클래스를 사용해야만 하는 이유로 보기엔 완전히 납득되지는 않네요..! 다만 해당 방향이 맞다고 판단하셨다면 일단은 그대로 유지하면서 클래스와 객체를 번갈아서 많이 써보시길 바랍니다.
• 2번에 대한 의도였다면, 현재와 같은 파일 구조를 유지해도 괜찮을 것 같다는 생각이 드네요 👍

[igy95]

이미 하위에서 모두 에러를 잡아 처리하는 것 같은데, 이 단계에서는 어떤 에러를 잡게 되나요?

[jinyoung234]

controller와 model 모두 같은 에러를 처리하고 있습니다..! 코드가 중복 됨에도 두 곳에서 처리하려고 한 이유는 아래와 같습니다.

1. model 에서만 검증하게 되면 count와 carNames를 settingRacingGame에서 모두 검증하기 때문에 carNames를 제대로 입력 후 count를 실패하게 되면 다시 carNames 부터 입력 받게 되어 사용자가 잘못된 입력 값을 작성한 경우 에러 메시지를 보여주고, 다시 입력할 수 있게 한다.의 요구사항에 어긋난다고 생각했습니다.

2. controller 에서만 검증하게 되면 테스트 코드에서 readline 모듈에 결합된 모듈이다보니 mocking을 할 수 밖에 없다고 생각했습니다. 다만, step3에서 jest.fn 사용을 못하기 때문에 mocking을 해도 다시 변경될 것이라고 생각했고, model에서 한번 더 검증하면 테스트 코드 짜기 용이할 것이라고 생각했습니다.

3. validator를 통해 코드 중복을 방지할 수 있었지만, validator를 사용하는 클래스 들이 많아질 수 있는 클래스이며, 변경이 자주 일어날 수 있는 모듈이라 생각해 결합도가 높아질 것이라고 생각했기 때문에 validator를 사용하지 않았습니다.

결과 적으로 코드 중복 보단 요구 사항을 처리하는 것이 올바른 트레이드오프 라고 생각 들어 다음과 같이 진행했지만 100% 확신이 들진 않는 상태입니다..ㅜㅜ validator를 사용하는 것이 맞는 선택일지, 아니면 다른 선택지가 있을지 궁금합니다 !!

[igy95]

요구사항 처리는 다른 방도가 없다면 어쩔 수 없지만, 테스트코드 작성을 염두에 두고 소스 코드를 작성하는 것은 약간 주객전도인 것 같습니다. 그보다는 오히려 명확한 레이어 구분이 중요한 우선순위라고 생각하는데요, 하나의 코드 베이스에 여러 사람의 의도가 들어가게 되면 컨벤션과 레이어를 통해 개발자로 하여금 코드 설계와 작성 방식을 어느정도 강제하는 것이 필요하기 때문입니다.

하지만 이 코드를 보았을 때 처음으로 들었던 생각은 '에러 처리를 하는 레이어가 어디인지 잘 모르겠다' 였는데요. 이러한 부분도 한번 고려해보시면 좋겠습니다.

[jinyoung234]

@GwangYeol-Im
안녕하세요 리뷰어님 🙇‍♀️ 리뷰어님 덕분에 새로운 관점에서 고민해볼 수 있었던 것 같습니다.

약간의 리팩터링을 진행하며 작업 했던 내용은 다음과 같습니다!

작업 내용

• RacingGame 클래스 내 race 함수에서 우승자를 업데이트 하는 것이 아닌 race가 끝나고 우승자를 업데이트 하도록 변경했습니다.
• 입력 관련 getRacingCarNames, getRacingCarCount의 prefix를 input으로 변경했습니다.
  • input은 입력 받는 과정이 포함되어있으므로, 재 입력 받을 책임과 검증할 수 있는 책임 모두 포함 시킬 수 있을 것이라고 생각했습니다.
• 입력 값에 대해 model이 아닌 controller만 검증하도록 하였으며, 테스트 코드 작성 및 올바른 요구 사항 반영을 위해 다시 Validator 사용을 도입했습니다.
  • model이 검증 해야 하는 것과 controller가 검증해야 하는 요소가 다르다고 생각하는데, 현재 검증하고 있는 것들이 "사용자 입력"과 관련된 데이터를 검증하다 보니 controller가 검증하는 것이 맞다고 생각했습니다.
  • 만약 "입력"과 다른 데이터를 검증(ex: 데이터 베이스)한다면 model에서 검증할 수 있을 거라 생각하여 model 내 validate 하는 로직은 제거 하였습니다.
  • Validator를 이전에 추가했다 제거했던 이유는 Validator가 비대해질 때 Validator를 사용하는 클래스가 많아지면 결합도가 높아져서 변경에 매우 취약한 설계가 될 거라 생각하여 제거했지만, 비대해질 때 다른 Validator를 추가하여 그 Validator에 의존하게 한다면 상관 없을 것 같다는 생각으로 인해 다시 도입했습니다. (리뷰어님이 말씀해주신 도메인이 여러 개일 때 validator의 위치에 대해 생각해보며 힌트를 얻었습니다!)

작업 내용 이외의 리뷰어 님이 말씀해주신 View과 관련된 내용에 대해서도 추가적으로 생각해보고 고민해보았습니다.

클래스로써의 View

우선, 제가 View를 설계 할 땐 이 앱이 아닌 브라우저로 변환되는 경우는 고려해보지 못한 채 cli 관점에서 설계했습니다.

그래서, 브라우저 관점도 고려해보게 되었고 Bridge Pattern(구현부에서 추상층을 분리하여 각자 독립적으로 변형이 가능하고 확장을 가능하게 만든다.)을 통해 UIView와 CliView를 추상화하는 View 클래스(abstract)를 만들어서 OCP도 준수하며 컨트롤러에서 CLIView 대신 추가해도 메서드의 기능적인 면에서 동일하기 때문에 문제 없이 변경과 재 사용성에 유연하게 설계가 가능하지 않을까 생각 했습니다.

// abstract class인 View
export class View {
  async inputByUser(message) {
    throw new Error('Not implemented');
  }

  print(message) {
    throw new Error('Not implemented');
  }
}

// View를 구현하는 CLIView
export class CLIView extends View {
  constructor() {
    this.inputView = InputView;
    this.outputView = OutputView;
  }

  async inputByUser(message) {
    const userInput = await this.inputView.input(message);
    return userInput;
  }

  print(message) {
    this.outputView.print(message);
  }
}

// View를 구현하는 UIView 
export class UIView extends View {
  constructor(inputView, outputView) {
    super();
    this.inputView = inputView;
    this.outputView = outputView;
  }

  async inputByUser(message) {
    // ...
  }

  print(message) {
    // ...
  }

하지만, 리뷰어님이 말씀해주신 "규모가 있는 프로덕트 내의 view는 쉽게 몇 백개가 넘어갈 수 있을 텐데 현재와 같은 구조가 된다면 View의 크기는 금방 비대해지지 않을까요?"에 대해 고민해보았을 때 gui 기반에서 View는 상태 관리 기반의 수 많은 Component중 하나로써 동작할 것이라고 생각했기 때문에 cli 기반 애플리케이션과 gui 기반 애플리케이션의 아키텍처 구조 자체가 달라질 것이라고 생각했습니다.

참고한 내용

• Vanilla Javascript로 웹 컴포넌트 만들기

아키텍처가 달라진다면 위 예시와 같이 설계 할 이유가 없을 거라 생각해서 따로 작업 하지는 않았습니다.

하지만 따로 리팩터링도 진행하지 않았는데, 관심사 측면에서 View를 변경하는 건 controller가 아닌 View 에서 전체적으로 관리할 수 있도록 하는게 이상적일 것이라 생각했기 때문입니다.

이상입니다 🙇‍♀️

[igy95]

안녕하세요 진영님~ 추가 코멘트 확인 부탁드리며 해당 미션은 이만 머지하도록 하겠습니다. 첫번째 미션 고생 많으셨습니다 ! 🙌