Skip to content

Conversation

@woowahan-pjs
Copy link
Owner

@woowahan-pjs woowahan-pjs commented Dec 31, 2023

미션을 진행하면서 웹 프론트엔드 교육 분야의 콘텐츠에 대한 피드백뿐만 아니라, 모바일 안드로이드 교육 분야의 콘텐츠에 대해서도 다시 살펴보는 시간이 되었습니다. 물론 수업을 들으면 어렵지는 않겠지만, 초보자들이 문서만 봐도 이해할 수 있도록 좀 더 친절하게 안내해 줬으면 좋았을 부분이 있었습니다.

  • Promise 기반 API가 아닌 콜백 및 동기화 API를 사용하였습니다. 콜백 및 동기화 API를 최대한 개선한 다음, 다음 단계에서 Promise 기반 API로 전환할 예정입니다.
  • 같은 이유로 ESM이 아닌 CJS를 먼저 적용하였습니다.
  • index.js의 코드 시작 부분과 폴더 구조에 대한 힌트가 있으면 좋겠다고 생각하였습니다.
    • 예를 들어, 이번 미션에서부터 미션 유틸리티 라이브러리를 사용하지 않으므로 입력 및 출력에 대한 예제 코드가 있으면 좋을 것 같습니다. (https://nodejs.org/api/readline.html)
    • 이미 프리코스를 거쳤지만, 프로그램을 실행하는 방법에 대한 언급이 다시 있으면 좋을 것 같습니다. (yarn install && yarn start)
  • 그 외 콘텐츠 관련 피드백은 별도로 전달해 드리겠습니다.

@@ -1,0 +1,49 @@
const Car = require("../src/Car");

describe("자동차는", () => {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS Code의 Jest 플러그인을 사용하면 IDE에서 테스트를 쉽게 실행할 수 있으며, describe()로 포장하면 여러 테스트를 동시에 쉽게 실행할 수 있습니다. 하지만 여러 개의 describe()가 있는 경우, 어떤 방법이 IDE에서 테스트를 실행하기 더 쉬울까요? (yarn test 제외)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS Code의 jest-runner 플러그인을 사용하면 describe 단위로 실행한 값을 터미널로 보여줍니다! 저는 이 방법을 사용하고 있어요.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Jest Runner보다 Jest 플러그인이 사용하기 더 쉽다고 생각합니다. 그리고 어떤 플러그인을 사용하든 '미션 수행을 위한 환경 설정'에도 이것을 추가하면 좋을 것 같아요.
  • 하나의 파일에서 여러 개의 describe()를 동시에 실행할 방법이 있는지 물어보려고 했습니다. Java나 코틀린에서는 하나의 클래스 안에 여러 개의 describe()를 넣고 클래스별로 테스트를 실행할 수 있으므로 VS Code에서도 가능한지 물어본 거예요!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제이슨 말씀해주신 게 예를 들어 하나의 파일 내에 describe()가 3 그룹 있으면 그 중 2개만 실행할 수 있는지가 맞을까요? 🙂
맞다면 저도 특별히 다른 방법은 모르고^_ㅜ, 플러그인을 설치한 상태에서 describe() 단위로 실행을 하고는 있습니다.
(tmi. 엄청나게 많이 쓰이는 플러그인은 아닌 것 같은데 최근에 Jest Run It이라는 플러그인을 써봤는데 딱 간단하게 실행 및 디버그 기능만 제공해주는 게 괜찮아서 써보고 있어요ㅋㅋㅋ)

VSCode는 기본적으로 (IntelliJ 대비로는)IDE 자체에서 모든걸 지원해주기보단 알고 계신 것처럼 오픈소스 플러그인들을 활용하는 경우가 많은데요, IntelliJ에 익숙하시다면 WebStorm이 IDE 자체에서 지원해주는 기능은 더 많은 편이라 맘에 드시는지 한번 살펴보셔도 좋을 것 같아요ㅋㅋ 커리큘럼에서는 VSCode도 실제로 널리 쓰이고 있고 라이센스가 필요하다는 점에서 WebStorm을 공식으로활용하고 있지는 않습니다.
+) 이것도 아마 원하시는 방법은 아닐 것 같지만 공유차ㅋㅋ 각 플러그인에서는 UI 기능 외에도 커스텀 커맨드들을 같이 지원해주는 경우가 많은데요. cmd+shift+ p에서 설치한 플러그인에서 제공해주는 커맨드도 찾아보실 수 있긴 합니다. 파일 전체를 실행하는 등의 단축키는 있을 거에요.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woowapark

  • 하나의 파일을 어떻게 테스트할 수 있는지, describe() 함수가 3개라면 어떻게 세 개를 모두 실행할 수 있는지 물어봤어요. 덕분에 ⌘;F 단축키에 대해 알게 되었네요. 앞으로 ⌘;F⌘;A가 매우 유용할 것 같아요!
    {
      "key": "cmd+; f",
      "command": "testing.runCurrentFile",
      "when": "editorTextFocus"
    }
    {
      "key": "cmd+; a",
      "command": "testing.runAll"
    }
  • 실제로 IntelliJ IDEA와 WebStorm은 큰 차이가 없습니다. IntelliJ IDEA에 필요한 플러그인을 설치하면 WebStorm과 동일해져요.
  • IntelliJ IDEA에는 유료 버전도 있지만 Community Edition이라는 무료 버전도 있어서 다른 두 교육 분야에서 사용하고 있습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오프라인에서 잡담하면서 같이 공유드렸지만... 기록 차원에서 남겨둡니당 :)
맞아요 실제로 IntelliJ IDEA 및 WebStorm 무료 버전을 사용하는 크루들도 많습니당
다만 VSCode도 꼭 무료여서 커리큘럼 내 공식 도구(?)로 강의 때 사용하고 있다기 보다는

  • 실제 실무 개발팀에서 일할 때에도 IntelliJ IDEA(혹은 WebStorm), VSCode 가 모두 사용되는 편
  • TypeScript가 사실상 표준처럼 사용되면서, TypeScript의 관리 주체인 MS가 함께 관리하고 있는 VSCode에서 TS관련 지원이 빠르게 적용되는 점

도 고려 대상일 것 같습니다. 그냥 주변에서 본 케이스들을 주관적으로 나누어보자면ㅋㅋ IDE 자체에서의 리팩터링 기능 등을 조금 더 많이 활용하고 싶어하시는 분들은 JetBrain 계열을, TS관련으로 넘어왔다가 오픈소스로 제공되는 각종 플러그인들을 설치해서 세팅하기 선호하시는 분들은 VSCode를 많이 쓰시는 것 같았어요.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이러한 학습 테스트는 수업 시간에 퀴즈 형식으로 진행하면 좋을 것 같아요. 실제로 수업에 활용하고 있습니다.

it.each([0, NaN, "", null, undefined])(
"조건 비교에서 거짓인 특수값 (%s)",
(parameter) => {
expect(!!parameter).toBe(false);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분을 배우려면 어떤 책을 읽어야 하나요? 자바라고 하면 '이펙티브 자바'를 읽어 보면 되는데, 자바스크립트에 관해서는 어떤 책을 읽어야 할지 궁금합니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 모던 자바스크립트 Deep Dive에서 비교 연산자를 다루는 곳에서 확인하실 수 있습니다.
    • 모던 자바스크립트 Deep Dive 저자가 운영하는 웹 사이트인 PoiemaWeb에서도 비교 연산자를 소개하고 있습니다.
  2. 자바스크립트는 왜 그 모양일까? Chapter 6. 불(bool) 타입에서도 해당 내용을 다루고 있습니다.
  3. 그 밖에 Modern Javascript Tutorial에서도 비교 연산자에 관해 잘 소개해 주고 있습니다!

Copy link
Collaborator

@woowahan-cron woowahan-cron Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자바스크립트가 얼마나 유연하면 거짓인 특수 값과 비교를 할 수 있는지 그리고 조건 비교에서 어떤 값이 나오는지는 알고 지나가야 한다고 생각해요. 그렇지만, 이를 바탕으로 프로그래밍에 적용하라는 의도는 아닙니다. 그래서 유연한 자바스크립트 언어에서는 프로그래머가 어떤 의도로 비교했는지 보다 명확하고 간결하게 작성해야 하는지를 시사하는 테스트라고 생각해요. 그래서 이것을 알고 실무에서 무조건 적용하기보다는 위험성을 인지하고 비교 의도를 명확히 드러낼 수 있는 방법을 탐구하면 좋을 것 같습니다🙏

(제이슨을 위한 피드백은 아니고.. 호기심에 지나치는 크루들이 많을 것 같다는 생각에 남기고 지나갑니다 👀)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비교 연산자 같은 기본적인 문법이 아닌 !!를 이용한 유효성 검사를 기법으로 봤기 때문에 '이펙티브 자바' 같은 자바스크립트 구문을 효율적으로 사용하는 방법을 알려 주는 책이 있는지 여쭤봤습니다!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 자바스크립트는 유연한 언어이므로 강력한 유효성 검사보다는 다양한 유형을 허용하는 것이 더 낫다는 뜻일까요?
  2. 자바스크립트로 코딩할 때 타입 등 각종 유효성 검사는 언제 추가되나요? 아니면 모든 것을 'try..catch'로 포장해야 할까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

벨로퍼트와 함께하는 모던 자바스크립트 Truthy and Falsy를 확인하시면 도움이 되실 것 같습니다! 그런데 이와 같은 기법을 언급한 책은 제가 아직 확인을 못해서요 위 링크로 갈음하겠습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오! 질문의 의도는 "!!"이었는데 제가 비교 연산자에 관한 것으로 범위를 확장해서 답변드렸던 점 먼저 말씀드려야 할 것 같아요.
2가지 질문을 주신 부분에 관련해서 답을 드릴게요!

1. 자바스크립트는 유연한 언어이므로 강력한 유효성 검사보다는 다양한 유형을 허용하는 것이 더 낫다는 뜻일까요?

  유연하게 사용한다면 강력한 유효성 검사가 맞지 않을까요? 그런데 자바스크립트가 유연한 언어인만큼 때때로 함수가 어떤 유형을 반환하는지 혼동할 수 있습니다. 저의 경우 반환 타입이 객체였는지 string이었는지 자주 헷갈렸어요. 따라서 이 현상을 놓고 본다면 유효성 검사를 강화하는 것이 맞겠죠. 하지만, 근본적으로는 자바스크립트의 유연함을 최대한 활용하기보다는 간결하고 명확한 의도를 코드에 드러내는 것이 바람직하다는 것이 제가 드리고 싶은 말씀이었습니다.

2. 자바스크립트로 코딩할 때 타입 등 각종 유효성 검사는 언제 추가되나요? 아니면 모든 것을 'try..catch'로 포장해야 할까요?

  각종 유효성 검사라면, 컴파일 시점에 수행하는 검증을 말씀하시는 것으로 이해했습니다. 이해한 내용을 바탕으로 결론부터 말씀드리면 말씀하신 유효성 검사는 타입스크립트로 보완할 수 있습니다. 그리고 자바스크립트 자체에서 이런 유효성 검사를 추가하는 것은 어렵거나 시간이 오래 걸릴 것 같아요. 왜냐하면, 자바스크립트는 웹 브라우저에서 태동한 언어이기 때문에 다른 언어와는 다르게 접근해야 한다고 생각하기 때문인데요. 자바스크립트는 초기 웹 브라우저에서 타입을 엄격하게 비교해서 계산하는 것보다는 유연하게 계산할 수 있도록 설계하는 것이 사용자 경험 측면에서 이롭다고 생각하지 않았을까 합니다(ex: null+2). 이런 기조와 함께 자바스크립트가 웹 브라우저 세상 밖에 나오기 전에는 호환성이 굉장히 중요했을 거예요. 자바스크립트의 문법에 변화가 생겼다고 하더라도 이전에 작성된 웹 페이지가 깨지게 될 경우 많은 경우 문제가 될 수 있으니까요. 그런데 제가 제이슨의 질문 의도를 제대로 이해했는지는 잘 모르겠네요🥲.

Copy link

@woowapark woowapark Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Exception을 낼 법한 상황에서도 맘대로 형을 변환해서 어떻게든 결과를 내주려는 JS의 특이한허술한 특성을 조금이나마 코드를 간결하게 쓰는데 써먹어보려는 시도가 아닐까..? 하고 있습니당. 그 중 일부는 언어 버전이 업데이트 되면서 최신 기능으로 대체하는 것을 더 권장하는 경우도 있고요.

크론이 이미 언급해주신 책을 포함해서 캠퍼스에 있는 책 중에서는 아래 책들이 순서대로 볼 만 하실 것 같아요
각 책에서 boolean, 혹은 타입 변환에 관한 섹션을 참고해보시면 도움이 되시지 않을까 싶습니다.
(1) <자바스크립트는 왜 그 모양일까?>
(2) <모던 자바스크립트 Deep Dive> / <You Don't Know JS> 둘 다 Truthy/Falsy와 논리 연산자에 대한 조금 더 원론적인 이야기들을 같이 설명해주고 있습니다. 말씀해주신 이펙티브 시리즈와 유사한 느낌이라면 1번 책이 더 가까울 것 같긴 해요.
(3) <자바스크립트 코딩의 기술> 도 인지도가 높은 책은 아니지만 이펙티브 시리즈처럼 아이템 단위로 간결하게 정리된 책이라 볼 만 합니당 :)

개인적으로는 간결하게 표현되기는 하지만 어쨌든 암묵적인 형변환에 기대고 있고 런타임에서는 의도한 바와 다르게 동작할 수 있다고 알고 있어 그냥 구체적인 값들로 비교해서 방어처리를 하는 게 안전하다고 생각해요. (정말 확실한 경우는 제외!)
유사하게 숫자가 아닌 값 앞에 +를 붙여 숫자 형변환을 시켜 활용하기도 하는데, 요것도 가독성 측면을 포함해 선호하지는 않아요.

!와는 좀 다른 활용이지만 논리 연산자 ||를 기본값 방어처리를 위해 사용하던 때도 있었는데요.

function someFunc(value) {
     const someValue = value || "default value";
} 
// 혹은 클래스 생성자에서 유사하게 사용

ECMAScript6에서 default parameter 문법이 추가되면서, 대체할 수 있는 경우에는 최신 문법을 쓰는 것이 권장되기도 합니다.
(참고: https://joshua1988.github.io/vue-camp/es6+/default-parameter.html#%E1%84%89%E1%85%A5%E1%86%AF%E1%84%86%E1%85%A7%E1%86%BC)

+) 더 최신 문법상으로는 ||와 함께 널 병합 연산자 ??&&와 함께 옵셔널 체이닝 ?. 도 같이 봐두셔도 좋을 것 같아요 🙂

src/Car.js Outdated
Comment on lines 20 to 28
if (
typeof condition === "number" &&
condition >= MINIMUM_MOVEMENT_CONDITION
) {
this.#position++;
}
if (typeof condition === "function" && condition()) {
this.#position++;
}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else를 사용하지 말라고 해서 사용하지 않았는데 어떻게 하면 코드를 더 깔끔하게 만들 수 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move 메서드의 condition은 숫자이거나 숫자를 반환하는 함수가 들어오는 것으로 이해했습니다. 이 설정을 바탕으로 else를 지양한 방법은 어떤 것이 있는지 간단하게 떠올려 보자면요...!

제1안: 삼항연산자

  • 해결할 수 있는 방법 중 하나의 선택지로 볼 수 있으나 권장되지 않는 방법입니다. Airbnb 스타일 가이드에 위배되기 때문입니다.
    typeof condition === "number" && condition >= MINIMUM_MOVEMENT_CONDITION
      ? this.#position++
      : typeof condition === "function" && condition()
      ? this.#position++
      : null;

제2안: 조건문 통합

move(condition) {
  const isNumberAndMeetsCondition =
    typeof condition === "number" && condition >= MINIMUM_MOVEMENT_CONDITION;
  const isFunctionAndReturnsTrue =
    typeof condition === "function" && condition();

  if (isNumberAndMeetsCondition || isFunctionAndReturnsTrue) {
    this.#position++;
  }
}

제3안: 함수 객체 만들어 사용

move(condition) {
   const func = {
      number: () => typeof condition === "number" && condition >= MINIMUM_MOVEMENT_CONDITION,
      function: () => typeof condition === "function" && condition(),
    };

    if (func[typeof condition]()) {
      this.#position++;
    }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그런데 저는 코드를 개선하기 이전에 move라는 메서드에서 받는 값을 제한하고 싶어요. 자바스크립트는 유연하기 때문에 자칫 유연한 처리가 가독성을 망칠 수 있다고 생각하기 때문인데요. 그래서 저는 개인적으로 받을 수 있는 타입을 좁힐 수 있다면 좁히는 것이 더 좋지 않을까 생각합니다.

  • 혹시 move 메서드에서 함수를 받으려고 하신 의도가 있으셨다면 이유가 무엇인지 궁금합니다.
  • move 메서드의 경우 number만 받게 되었을 경우 문제가 될 것으로 우려하시는 점이 있으실까요?

Copy link
Owner Author

@woowahan-pjs woowahan-pjs Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1 (comment) 의 질문과 함께 답변해 드리겠습니다!

  • 숫자와 함수를 모두 받도록 move()를 설계함으로써 동적 타이핑이라는 자바스크립트 기능을 활용하려고 했습니다.
    • 동적 타이핑을 활용한다고 하지만, 내부에서 타입 검사를 하는 것을 보고 약간 회의적이었지만요.
    • 실제 Java나 코틀린이었다면 함수만 받도록 리팩터링했을 거예요.
  • 자동차를 단순히 숫자로 움직이는 것보다 더 다양한 방식으로 움직이도록 확장하는 것에 대해 생각하기 시작하면 고차 함수, 전략 패턴 등 함수형 프로그래밍과 객체지향 프로그래밍 모두에서 고민해야 할 다양한 문제가 생길 거예요. 예를 들어, '숫자로' 움직일지를 결정하는 대신 '움직일지를 결정하는' 역할 자체에 초점을 맞추게 됩니다.
    • move() 내부에 무작위를 사용하기 때문에 테스트하기 어려운 문제를 이미 해결한 리뷰이에게는 도전 과제로 제공합니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"자동차의 다양한 움직임"을 위한 관점 때문이라면 �최종적으로는 자바스크립트에서도 숫자와 함수가 아닌 함수만 받도록 해도 될 것 같습니다. 그런데 숫자와 함수를 둘 다 받게 하신 것은 자동차의 숫자로 움직이는 과정에서 다양하게 움직이기 위한 과정으로 확장하기 위한 의도이셨겠군요!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

흐음.... 숫자와 함수를 모두 받아야만 한다면 크론이 말씀해주신 2안 정도로 조건식을 별도 변수로 빼내서 조건문 자체는 한 번 사용하는 정도일 것 같아요. 더 분리한다면 isNumberAndMeetsCondition || isFunctionAndReturnsTrueshouldMove() 같은 별도의 함수로 한번 더 빼내거나..?
말씀해주신 것처럼 결국은 타입 검사가 필요해서 🤔🤔🤔
연습 차원에서 전략 패턴을 쓸 수는 있겠으나, 자동차 경주에서는 약간 배보다 배꼽이 커지는 느낌이긴 하네요ㅋㅋ

혹은 switch문으로 대체하는 건 어떠신가요?

  shouldMove(condition) {
    switch (typeof condition) {
      case "number":
        return condition >= MINIMUM_MOVEMENT_CONDITION;
      case "function":
        return condition();
      default:
        return false;
    }
  }

  move(condition) {
    if (this.shouldMove(condition)) {
      this.#position++;
    }
  }

switch문을 사용한 구문은 경우에 따라 객체 리터럴로 대체할 수 없는지 검토해보기도 합니다.

  shouldMove(condition){
    const conditionType = typeof condition;
    const conditionMap = {
      "number": () => condition >= MINIMUM_MOVEMENT_CONDITION,
      "function": () => condition(),
    };

    return (conditionMap[conditionType] && conditionMap[conditionType]()) || false;
  }

그런디 개인 취향이지만 이 경우는 객체 리터럴로 하면 오히려 가독성이 떨어지는 느낌이긴 하네요ㅋㅋ
switch문 정도까지는 고려해보지 않을까 싶습니다. 혹은 오히려 if 문을 그대로 두거나요.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

강의 자료에서 else를 사용하지 말라는 언급은 빼면 좋을 것 같아요! else를 사용하지 않는다는 것은 switch를 사용하지 않는다는 의미이기도 합니다.
https://developerfarm.wordpress.com/2012/01/27/object_calisthenics_3

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else를 이용한 것이 명시적일 수 있는 경우도 있다고 생각해서 else를 굳이 사용하지 말라는 언급은 빼도 될 것 같습니다.
그런데 이전에 else를 사용하지 않는다 또는 지양하라는 의미는 가독성을 해치지 말라는 의도로 이해했었어요.

  1. Early Return 패턴을 이용한 가독성 개선
  2. 아래와 같은 복잡한 코드 구성을 지양
    function foo() {
        if (조건1) {
            if (조건2) {
                if (조건3) {
                    if (조건4) {
                    } else {
                        throw new Error();
                    }
                } else {
                    throw new Error();
                }
            } else {
                throw new Error();
            }
        } else {
            throw new Error();
        }
    }

그런데 2번의 경우 프로그래밍 요구사항에서 함수 들여쓰기 수준을 제한하고 있어서 굳이 금지할 필요가 없겠다? 싶습니다.
혹시 제가 생각하는 의도로 금지하셨던 이유가 아니었다면? else 사용금지 언급 여부와 무관하게 그 이유도 제가 알면 좋을 것 같아요 ㅎㅎ

src/Racing.js Outdated
Comment on lines 22 to 23
const { name, position } = car;
return { name, position };
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처음에는 return ({ name, position } = car);로 작성하여 약간의 삽질을 하였습니다. 정확한 원인은 무엇일까요? 변수가 선언되지 않았기 때문일까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const { name, position } = car;

22번째 줄의 경우 객체 구조 분해 할당 문법 이라고 부릅니다. 그런데 자바스크립트에서 구조 분해 할당과 동시에 return하는 것은 유효하지 않습니다.

한 줄에 걸쳐 사용하고 싶다면 아래와 같이 개선할 수 있습니다.

return { name: car.name, position: car.position };

또는 21번째 줄이 없다고 가정한다면 아예 메서드 선언부에서 구조 분해를 할 수 있습니다.

#move({ name, position }, condition) {
 return { name, position };
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체 리터럴을 사용하자는 제안이 마음에 드는데, 제가 생각하던 방향이었습니다! 값을 복사해야 하는데 추가 클래스를 만들고 싶지 않았거든요.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 파일 이름의 규칙은 무엇인가요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문의 의도를 잘 이해하지 못했어요.

그렇지만, 파일명에 대한 리뷰를 드리자면요. cars라고 하면 Car객체의 배열이라고 암시하는 경우가 많다고 생각했어요. 그래서 이름을 개선해 보는 것이 좋지 않을지 먼저 제안해 드리고 싶습니다. 지금은 자동차 객체를 담은 배열을 받아서 처리한 값을 반환하고 있으니 carsUtils.js 라거나 carArrayUtils.js와 같이 명명한다면 이 파일은 무슨 내용을 담고 있는지 명확하게 파악할 수 있지 않을까요?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Java에서는 StringUtils와 같이 끝에 'Util'을 추가하곤 했습니다. 코틀린에서는 최상위 함수를 다룰 때는 클래스가 아닌 이러한 최상위 함수가 포함된 파일 이름에 'Util'과 같은 의미 없는 단어를 사용하지 말라고 합니다.
  • 제 질문은 자바스크립트 클래스가 있는 파일 이름은 대문자 카멜 케이스를 사용하고, 하나의 공개 함수가 있는 파일은 소문자 카멜 케이스를 사용하는데, 둘 이상의 공개 함수가 있는 파일의 이름을 짓는 규칙이 무엇인지 궁금했습니다.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실, Airbnb 스타일 가이드에서 자바스크립트에서 보편적으로 파일명을 정해야 한다고 가이드하는 규칙은 따로 없는 것으로 알고 있습니다. 프로젝트 내에서 보편적인 파일 이름 정하는 규칙을 정립하여 일관되게만 사용하면 좋을 것 같아요. 이러한 차원에서 위와 같이 파일에 든 함수가 어떤 역할을 수행하는지를 이름에서 드러내면 좋을 것 같아 제안드린 것이기도 합니다!

다만, Google Style Guide에서는 영소문자, 하이픈(-), 밑줄(_) 문자만 허용하고 있으며 구두점도 허용하지 않습니다.

File names must be all lowercase and may include underscores (_) or dashes (-), but no additional punctuation. Follow the convention that your project uses. Filenames’ extension must be .js.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참고로 아래 글도 공유해 드려요!
https://jojoldu.tistory.com/671

src/index.js Outdated
Comment on lines 10 to 23
(function askCarNames() {
rl.question(
"경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).\n",
(answer) => {
try {
const cars = createCars(answer);
askAttemptCount(cars);
} catch (e) {
console.error(`[ERROR] ${e.message}`);
askCarNames();
}
}
);
})();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

애플리케이션을 어떻게 시작해야 할지, 콜백 구조를 어떻게 개선해야 할지 고민입니다.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

콜백 및 동기화 API를 최대한 개선한 다음, 다음 단계에서 Promise 기반 API로 전환할 예정입니다. 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

즉시실행함수를 통해 askCarNames() 함수를 실행하셨군요! 제 생각에는 index.js 파일에서 프로그램의 진입 부분을 어떻게 호출하면 좋을지를 고민하시다가 즉시실행함수를 사용하셨다고 생각했습니다. 그런데 이 경우에는 시작 메서드를 별도로 정의하시고(play라거나.. run이라거나..) 그 메서드를 호출해도 충분할 거 같아요! 즉시실행함수를 사용하는 용도는 자바스크립트 문법이 정립되기 이전(ES6)에 스코프의 오염을 방지하기 위해서 사용했던 기법이라서요. 지금처럼 webpack 모듈 번들러를 사용하는 경우에는 굳이 사용할 필요는 없다고 생각합니다.

제가 추천드리는 접근법은 먼저 readline 인터페이스를 별도로 빼서 관리해 보는 것은 어떠세요? 자바스크립트 mission-utils의 구조를 참고해 보시면 좋을 것 같아요!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참고로 해당 라이브러리도 제가 만든 구조입니다... 😂

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

구체적으로 고민되는 부분이 있으신가요~?
해보시다가 필요하면 호출해 주셔도 좋습니다 🙂

@woowahan-pjs woowahan-pjs marked this pull request as ready for review December 31, 2023 13:48
Copy link
Collaborator

@woowahan-cron woowahan-cron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

24년 목표 중 하나가 ‘웹 프론트엔드 미션 해보기’라고 하셨는데 저의 리뷰가 최대한 도움이 되셨다면 좋겠습니다. Polyglot이신 제이슨이 자바스크립트 세계를 처음 탐험하실 때 필요하다고 생각하신 부분이 무엇일지 리뷰를 바탕으로 정리해 봤습니다. (저는 Polyglot 아니에요. 그래서 저의 추론이 틀렸을 수도 있습니다)

  • 자바스크립트 언어의 이해와 히스토리(스코프, 모듈시스템)
    • 문법 (구조분해 할당과 동시에 return 불가 등)
    • 메서드의 매개 변수를 어떻게 받아서 처리할 것인가?
    • 즉시실행함수라는 기법이 존재하지만 지금은 사용되지 않는 이유
  • 우아한테크코스에서 권장하는 컨벤션을 준수하기 위해 필요한 것
    • 절차 for문(Iterator) 대신 배열 메서드(Array.from) 활용하기

이미 자동차 경주 미션은 자바나 코틀린을 통해서 수백 번 코드를 짜 보셨기 때문에 프로그래밍 요구사항 구현이라거나 접근법, 테스트 코드 작성에 관해서는 굳이 리뷰를 드리지 않아도 부족함이 없으셔서 skip했습니다.

제이슨의 자바스크립트 정복 응원합니다👍

it.each([0, NaN, "", null, undefined])(
"조건 비교에서 거짓인 특수값 (%s)",
(parameter) => {
expect(!!parameter).toBe(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 모던 자바스크립트 Deep Dive에서 비교 연산자를 다루는 곳에서 확인하실 수 있습니다.
    • 모던 자바스크립트 Deep Dive 저자가 운영하는 웹 사이트인 PoiemaWeb에서도 비교 연산자를 소개하고 있습니다.
  2. 자바스크립트는 왜 그 모양일까? Chapter 6. 불(bool) 타입에서도 해당 내용을 다루고 있습니다.
  3. 그 밖에 Modern Javascript Tutorial에서도 비교 연산자에 관해 잘 소개해 주고 있습니다!

src/Car.js Outdated
Comment on lines 20 to 28
if (
typeof condition === "number" &&
condition >= MINIMUM_MOVEMENT_CONDITION
) {
this.#position++;
}
if (typeof condition === "function" && condition()) {
this.#position++;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move 메서드의 condition은 숫자이거나 숫자를 반환하는 함수가 들어오는 것으로 이해했습니다. 이 설정을 바탕으로 else를 지양한 방법은 어떤 것이 있는지 간단하게 떠올려 보자면요...!

제1안: 삼항연산자

  • 해결할 수 있는 방법 중 하나의 선택지로 볼 수 있으나 권장되지 않는 방법입니다. Airbnb 스타일 가이드에 위배되기 때문입니다.
    typeof condition === "number" && condition >= MINIMUM_MOVEMENT_CONDITION
      ? this.#position++
      : typeof condition === "function" && condition()
      ? this.#position++
      : null;

제2안: 조건문 통합

move(condition) {
  const isNumberAndMeetsCondition =
    typeof condition === "number" && condition >= MINIMUM_MOVEMENT_CONDITION;
  const isFunctionAndReturnsTrue =
    typeof condition === "function" && condition();

  if (isNumberAndMeetsCondition || isFunctionAndReturnsTrue) {
    this.#position++;
  }
}

제3안: 함수 객체 만들어 사용

move(condition) {
   const func = {
      number: () => typeof condition === "number" && condition >= MINIMUM_MOVEMENT_CONDITION,
      function: () => typeof condition === "function" && condition(),
    };

    if (func[typeof condition]()) {
      this.#position++;
    }
}

src/Car.js Outdated
Comment on lines 20 to 28
if (
typeof condition === "number" &&
condition >= MINIMUM_MOVEMENT_CONDITION
) {
this.#position++;
}
if (typeof condition === "function" && condition()) {
this.#position++;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그런데 저는 코드를 개선하기 이전에 move라는 메서드에서 받는 값을 제한하고 싶어요. 자바스크립트는 유연하기 때문에 자칫 유연한 처리가 가독성을 망칠 수 있다고 생각하기 때문인데요. 그래서 저는 개인적으로 받을 수 있는 타입을 좁힐 수 있다면 좁히는 것이 더 좋지 않을까 생각합니다.

  • 혹시 move 메서드에서 함수를 받으려고 하신 의도가 있으셨다면 이유가 무엇인지 궁금합니다.
  • move 메서드의 경우 number만 받게 되었을 경우 문제가 될 것으로 우려하시는 점이 있으실까요?

src/Racing.js Outdated
Comment on lines 22 to 23
const { name, position } = car;
return { name, position };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const { name, position } = car;

22번째 줄의 경우 객체 구조 분해 할당 문법 이라고 부릅니다. 그런데 자바스크립트에서 구조 분해 할당과 동시에 return하는 것은 유효하지 않습니다.

한 줄에 걸쳐 사용하고 싶다면 아래와 같이 개선할 수 있습니다.

return { name: car.name, position: car.position };

또는 21번째 줄이 없다고 가정한다면 아예 메서드 선언부에서 구조 분해를 할 수 있습니다.

#move({ name, position }, condition) {
 return { name, position };
}

src/index.js Outdated
Comment on lines 10 to 23
(function askCarNames() {
rl.question(
"경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).\n",
(answer) => {
try {
const cars = createCars(answer);
askAttemptCount(cars);
} catch (e) {
console.error(`[ERROR] ${e.message}`);
askCarNames();
}
}
);
})();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

즉시실행함수를 통해 askCarNames() 함수를 실행하셨군요! 제 생각에는 index.js 파일에서 프로그램의 진입 부분을 어떻게 호출하면 좋을지를 고민하시다가 즉시실행함수를 사용하셨다고 생각했습니다. 그런데 이 경우에는 시작 메서드를 별도로 정의하시고(play라거나.. run이라거나..) 그 메서드를 호출해도 충분할 거 같아요! 즉시실행함수를 사용하는 용도는 자바스크립트 문법이 정립되기 이전(ES6)에 스코프의 오염을 방지하기 위해서 사용했던 기법이라서요. 지금처럼 webpack 모듈 번들러를 사용하는 경우에는 굳이 사용할 필요는 없다고 생각합니다.

제가 추천드리는 접근법은 먼저 readline 인터페이스를 별도로 빼서 관리해 보는 것은 어떠세요? 자바스크립트 mission-utils의 구조를 참고해 보시면 좋을 것 같아요!

expect(car.position).toBe(0);
});

it("숫자 이외의 다양한 조건으로 움직일 수 있다", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제이슨이 추가하신 나름의 규칙일까요? 2023년도 자동차 경주 기준으로는 숫자 이외의 다양한 조건으로 움직여야 한다는 요구사항이 없습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문의 의도를 잘 이해하지 못했어요.

그렇지만, 파일명에 대한 리뷰를 드리자면요. cars라고 하면 Car객체의 배열이라고 암시하는 경우가 많다고 생각했어요. 그래서 이름을 개선해 보는 것이 좋지 않을지 먼저 제안해 드리고 싶습니다. 지금은 자동차 객체를 담은 배열을 받아서 처리한 값을 반환하고 있으니 carsUtils.js 라거나 carArrayUtils.js와 같이 명명한다면 이 파일은 무슨 내용을 담고 있는지 명확하게 파악할 수 있지 않을까요?

src/Racing.js Outdated
Comment on lines 13 to 17
const results = [];
for (let index = 0; index < this.#attemptCount.value; index++) {
results.push(this.#cars.map((it) => this.#move(it, condition)));
}
return results;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래와 같이 한 줄로 개선할 수 있습니다!
Airbnb 컨벤션에서 for-loop의 사용을 지양하라고 했기 때문에 아래와 같이 개선해 보면 좋을 것 같아요!

Suggested change
const results = [];
for (let index = 0; index < this.#attemptCount.value; index++) {
results.push(this.#cars.map((it) => this.#move(it, condition)));
}
return results;
const results = Array.from({ length: this.#attemptCount.value }, () =>
this.#cars.map((it) => this.#move(it, condition))
);
return results;
  • Array.from 정적 메서드를 이용해서 배열을 생성할 수 있습니다! 자바스크립트에서 Array.from은 이 코드의 상황과 유사한 경우에 자주 사용됩니다.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it.each([0, NaN, "", null, undefined])(
"조건 비교에서 거짓인 특수값 (%s)",
(parameter) => {
expect(!!parameter).toBe(false);
Copy link
Collaborator

@woowahan-cron woowahan-cron Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자바스크립트가 얼마나 유연하면 거짓인 특수 값과 비교를 할 수 있는지 그리고 조건 비교에서 어떤 값이 나오는지는 알고 지나가야 한다고 생각해요. 그렇지만, 이를 바탕으로 프로그래밍에 적용하라는 의도는 아닙니다. 그래서 유연한 자바스크립트 언어에서는 프로그래머가 어떤 의도로 비교했는지 보다 명확하고 간결하게 작성해야 하는지를 시사하는 테스트라고 생각해요. 그래서 이것을 알고 실무에서 무조건 적용하기보다는 위험성을 인지하고 비교 의도를 명확히 드러낼 수 있는 방법을 탐구하면 좋을 것 같습니다🙏

(제이슨을 위한 피드백은 아니고.. 호기심에 지나치는 크루들이 많을 것 같다는 생각에 남기고 지나갑니다 👀)

#value;

constructor(value) {
value = parseInt(value);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

number 타입으로 변환하는 방법으로는 parseInt(value) vs Number(value)가 있는데 결과에 차이가 있습니다.

Copy link

@woowapark woowapark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

화이팅입니당~~~💪🚀💪🚀💪🚀
필요하신 건 아무때나 호출해주세요~!ㅋㅋ

@@ -1,0 +1,49 @@
const Car = require("../src/Car");

describe("자동차는", () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제이슨 말씀해주신 게 예를 들어 하나의 파일 내에 describe()가 3 그룹 있으면 그 중 2개만 실행할 수 있는지가 맞을까요? 🙂
맞다면 저도 특별히 다른 방법은 모르고^_ㅜ, 플러그인을 설치한 상태에서 describe() 단위로 실행을 하고는 있습니다.
(tmi. 엄청나게 많이 쓰이는 플러그인은 아닌 것 같은데 최근에 Jest Run It이라는 플러그인을 써봤는데 딱 간단하게 실행 및 디버그 기능만 제공해주는 게 괜찮아서 써보고 있어요ㅋㅋㅋ)

VSCode는 기본적으로 (IntelliJ 대비로는)IDE 자체에서 모든걸 지원해주기보단 알고 계신 것처럼 오픈소스 플러그인들을 활용하는 경우가 많은데요, IntelliJ에 익숙하시다면 WebStorm이 IDE 자체에서 지원해주는 기능은 더 많은 편이라 맘에 드시는지 한번 살펴보셔도 좋을 것 같아요ㅋㅋ 커리큘럼에서는 VSCode도 실제로 널리 쓰이고 있고 라이센스가 필요하다는 점에서 WebStorm을 공식으로활용하고 있지는 않습니다.
+) 이것도 아마 원하시는 방법은 아닐 것 같지만 공유차ㅋㅋ 각 플러그인에서는 UI 기능 외에도 커스텀 커맨드들을 같이 지원해주는 경우가 많은데요. cmd+shift+ p에서 설치한 플러그인에서 제공해주는 커맨드도 찾아보실 수 있긴 합니다. 파일 전체를 실행하는 등의 단축키는 있을 거에요.

expect(car.position).toBe(0);
});

it.each([0, null, undefined])(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JS는 너무 관대한 나머지 계산 결과가 잘못되어도 Exception을 내지 않고 NaN 을 돌려주기 때문에 요 값도 종종 방어처리를 같이 하곤 하는데요.
이 테스트 케이스에 추가해도 이미 방어처리가 잘 되었을 것으로 보이지만, 한 세트로 봐두시면 좋을 것 같아 남겨둡니당 :)

it.each(["", null, undefined])(
"parseInt는 숫자가 아니면 NaN을 반환한다",
(parameter) => {
const actual = parseInt(parameter);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseInt()의 두 번째 인자인 radix는 optional이기는 기본값이 10진수 고정이 아니기 때문에 특별히 다른 의도가 있는 게 아니라면 10을 같이 써주기를 권장하고 있습니당 🙂
https://velog.io/@mukeunzi/JavaScript-parseInt%EC%97%90%EC%84%9C-radix%EB%A5%BC-%EC%83%9D%EB%9E%B5%ED%95%B4%EB%8F%84-%EB%90%A0%EA%B9%8C

);

// https://inpa.tistory.com/entry/📚-null-undefined-NaN
it.each([0, NaN, "", null, undefined])(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 NaN이 이미 쓰였군요!ㅋㅋㅋ👍


// https://inpa.tistory.com/entry/📚-null-undefined-NaN
it.each([0, NaN, "", null, undefined])(
"조건 비교에서 거짓인 특수값 (%s)",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참고 정보) 원하신다면 toBeTruthy(), toBeFalsy() 등의 메서드를 사용해 보실 수도 있습니당
https://jestjs.io/docs/using-matchers#truthiness

src/cars.js Outdated
}

module.exports.maxByPosition = maxByPosition;
module.exports.filterByPosition = filterByPosition;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES6 이후부터는 단축 속성명을 사용할 수 있어 요 export 구문에도 동일하게 적용할 수 있습니다.
요런 방식은 어떠신가요? 🙂

Suggested change
module.exports.filterByPosition = filterByPosition;
module.exports = {
maxByPosition,
filterByPosition
};

// https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Math/random
function getRandomIntInclusive(min, max) {
min = Math.ceil(min);
max = Math.floor(max);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재할당해서 사용하는 경우 의도하지 않은 부수 효과가 생길 수 있을 것 같아요.
이름을 굳이 다시 붙여야 하는 점은 좀 별로지만..ㅋㅋ 새로 할당해서 사용하면 어떨까요?

Suggested change
max = Math.floor(max);
const ceilMin = Math.ceil(min);
const floorMax = Math.floor(max);

Copy link
Owner Author

@woowahan-pjs woowahan-pjs Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위의 링크에서 코드를 그대로 복사하였습니다.

src/index.js Outdated
Comment on lines 10 to 23
(function askCarNames() {
rl.question(
"경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).\n",
(answer) => {
try {
const cars = createCars(answer);
askAttemptCount(cars);
} catch (e) {
console.error(`[ERROR] ${e.message}`);
askCarNames();
}
}
);
})();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

구체적으로 고민되는 부분이 있으신가요~?
해보시다가 필요하면 호출해 주셔도 좋습니다 🙂

src/Car.js Outdated
Comment on lines 20 to 28
if (
typeof condition === "number" &&
condition >= MINIMUM_MOVEMENT_CONDITION
) {
this.#position++;
}
if (typeof condition === "function" && condition()) {
this.#position++;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

흐음.... 숫자와 함수를 모두 받아야만 한다면 크론이 말씀해주신 2안 정도로 조건식을 별도 변수로 빼내서 조건문 자체는 한 번 사용하는 정도일 것 같아요. 더 분리한다면 isNumberAndMeetsCondition || isFunctionAndReturnsTrueshouldMove() 같은 별도의 함수로 한번 더 빼내거나..?
말씀해주신 것처럼 결국은 타입 검사가 필요해서 🤔🤔🤔
연습 차원에서 전략 패턴을 쓸 수는 있겠으나, 자동차 경주에서는 약간 배보다 배꼽이 커지는 느낌이긴 하네요ㅋㅋ

혹은 switch문으로 대체하는 건 어떠신가요?

  shouldMove(condition) {
    switch (typeof condition) {
      case "number":
        return condition >= MINIMUM_MOVEMENT_CONDITION;
      case "function":
        return condition();
      default:
        return false;
    }
  }

  move(condition) {
    if (this.shouldMove(condition)) {
      this.#position++;
    }
  }

switch문을 사용한 구문은 경우에 따라 객체 리터럴로 대체할 수 없는지 검토해보기도 합니다.

  shouldMove(condition){
    const conditionType = typeof condition;
    const conditionMap = {
      "number": () => condition >= MINIMUM_MOVEMENT_CONDITION,
      "function": () => condition(),
    };

    return (conditionMap[conditionType] && conditionMap[conditionType]()) || false;
  }

그런디 개인 취향이지만 이 경우는 객체 리터럴로 하면 오히려 가독성이 떨어지는 느낌이긴 하네요ㅋㅋ
switch문 정도까지는 고려해보지 않을까 싶습니다. 혹은 오히려 if 문을 그대로 두거나요.

it.each([0, NaN, "", null, undefined])(
"조건 비교에서 거짓인 특수값 (%s)",
(parameter) => {
expect(!!parameter).toBe(false);
Copy link

@woowapark woowapark Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Exception을 낼 법한 상황에서도 맘대로 형을 변환해서 어떻게든 결과를 내주려는 JS의 특이한허술한 특성을 조금이나마 코드를 간결하게 쓰는데 써먹어보려는 시도가 아닐까..? 하고 있습니당. 그 중 일부는 언어 버전이 업데이트 되면서 최신 기능으로 대체하는 것을 더 권장하는 경우도 있고요.

크론이 이미 언급해주신 책을 포함해서 캠퍼스에 있는 책 중에서는 아래 책들이 순서대로 볼 만 하실 것 같아요
각 책에서 boolean, 혹은 타입 변환에 관한 섹션을 참고해보시면 도움이 되시지 않을까 싶습니다.
(1) <자바스크립트는 왜 그 모양일까?>
(2) <모던 자바스크립트 Deep Dive> / <You Don't Know JS> 둘 다 Truthy/Falsy와 논리 연산자에 대한 조금 더 원론적인 이야기들을 같이 설명해주고 있습니다. 말씀해주신 이펙티브 시리즈와 유사한 느낌이라면 1번 책이 더 가까울 것 같긴 해요.
(3) <자바스크립트 코딩의 기술> 도 인지도가 높은 책은 아니지만 이펙티브 시리즈처럼 아이템 단위로 간결하게 정리된 책이라 볼 만 합니당 :)

개인적으로는 간결하게 표현되기는 하지만 어쨌든 암묵적인 형변환에 기대고 있고 런타임에서는 의도한 바와 다르게 동작할 수 있다고 알고 있어 그냥 구체적인 값들로 비교해서 방어처리를 하는 게 안전하다고 생각해요. (정말 확실한 경우는 제외!)
유사하게 숫자가 아닌 값 앞에 +를 붙여 숫자 형변환을 시켜 활용하기도 하는데, 요것도 가독성 측면을 포함해 선호하지는 않아요.

!와는 좀 다른 활용이지만 논리 연산자 ||를 기본값 방어처리를 위해 사용하던 때도 있었는데요.

function someFunc(value) {
     const someValue = value || "default value";
} 
// 혹은 클래스 생성자에서 유사하게 사용

ECMAScript6에서 default parameter 문법이 추가되면서, 대체할 수 있는 경우에는 최신 문법을 쓰는 것이 권장되기도 합니다.
(참고: https://joshua1988.github.io/vue-camp/es6+/default-parameter.html#%E1%84%89%E1%85%A5%E1%86%AF%E1%84%86%E1%85%A7%E1%86%BC)

+) 더 최신 문법상으로는 ||와 함께 널 병합 연산자 ??&&와 함께 옵셔널 체이닝 ?. 도 같이 봐두셔도 좋을 것 같아요 🙂

}

function main() {
askCarNames();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promise 기반 API로 자연스럽게 이어질 수 있도록 피드백 강의로 다루면 좋을 것 같습니다.
https://joshua1988.github.io/web-development/javascript/javascript-asynchronous-operation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async/await로 가기 전 중간 단계로 프로미스 체이닝을 같이 다뤄봐도 좋겠네요 :)
https://ko.javascript.info/promise-chaining
(비동기 키워드 피드백에 유사한 내용들이 이미 있긴 합니당 - 근데 요건 약간 레벨 후반부의 이야기네요)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그러고 보니

  • 5기: 프리코스와 자동차경주 모두 콜백 방식으로 진행 but 자동차 경주를 콜백 방식으로 진행하라는 별도의 제한을 두지 않았었던 것으로 기억해요.
  • 6기: 프리코스부터 async-await 방식으로 진행했으니 이번 자동차 경주에서도 콜백이 아니라 async-await 방식으로 진행하는 편이 좋을지, 어떻게 Promise API와 함께 비동기 강의를 구성해 보면 좋을지도 고민이네요!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woowahan-cron
프리코스가 레벨1 커리큘럼에 또 영향을 주었네요! 1단계에서는 콜백 및 동기화 API를, 2단계에서는 Promise 기반 API를 사용하게 하면 좋을 것 같아요. 어느 쪽이든 교육생들이 스스로 필요성을 느끼고, 우리가 그 서사를 잘 설명하는 것이 중요하다고 생각합니다.

Comment on lines +12 to +13
rl.close();
callback(answer);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woowapark 의 설명을 기다리고 있습니다.

Suggested change
rl.close();
callback(answer);
callback(answer);
rl.close();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(글로 적다 포기한 흔적)
오프라인에서 설명드리겠습니다!ㅋㅋ

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

설명 완료!ㅋㅋ

Copy link

@woowapark woowapark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타입이 없는 세계에서 고군분투하시는 제이슨 화이팅입니다ㅋㅋ
promise나 async/await 문법을 사용하여 개선하는 것, 모듈 시스템 변경 등은 다음 단계를 진행하면서 단계적으로 고민해보신다고 하여, 수정하신 부분들에 대한 질문 및 참고 자료 등만 남겨두었습니다 :)
한번 확인만 부탁드려요! 💪💪💪💪💪 필요하다면 오프라인으로 이야기 나누어도 좋습니다~!


function renderCar(car) {
return `${car.name} : ${"-".repeat(car.position)}`;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createCars, randomMovementStrategy 와 같은 함수와 입출력을 처리하는 함수가 한 파일에 함께 있으니 도메인 로직과 입출력 로직이 분리되지 않은 것으로 보이는 것 같은데 어떻게 생각하시나용?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참고로 createCars()는 다른 파일로 이동하였습니다. index.js가 컨트롤러 역할을 하므로, 정확히 말하면 흐름 제어 로직과 입출력 로직이 서로 다른 파일로 분리되어 있지 않다고 할 수 있습니다. 굳이 분리할 필요성을 느끼지 못했지만, 다른 두 교육 분야는 2단계에서 MVC 디자인 패턴 적용을 요구하고 있고, 그 요구 사항에 따라 분리할 수 있다고 생각합니다!

// https://mathieularose.com/main-function-in-node-js
if (require.main === module) {
main();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅋㅋㅋjava가 아니라 kotlin에서도 main()을 사용하는 것이 일반적인가요?-?
링크로 남겨주신 부분은 저도 처음 보았습니다...! 🤔
알고 계시듯이 js에서는 언어 자체에서 애플리케이션의 시작점이 되는 main 메서드 등을 정의해두지 않고 있고,
애플리케이션의 시작점이 되는 메서드 혹은 함수에 대한 특정 컨벤션이 있는 것도 아니라고 알고 있는데요.
익숙하기로는 run(), init(), start() 등을 사용하는 것 같아 의견만 남겨둡니당
js에서도 main()을 못본 건 아니지만 보통은 좀 더 엔진 레벨에 가까운 구현을 하는 코드에서 보았던 것 같아요.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

파이썬을 제외하면 대부분 언어에서는 main 함수를 요구 사항이나 관례로 사용하는 것 같아요. 찾아보니 파이썬에도 현재 모듈이 가장 먼저 실행되는 모듈인지 확인하는 방법이 있더라고요.
https://wikidocs.net/84414

src/Car.js Outdated
@@ -0,0 +1,36 @@
const MAXIMUM_NAME_LENGTH = 5;
Copy link

@woowapark woowapark Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

언어와 무관하게 의견이 궁금해서 코멘트 남겨둡니다.
js에서도 정적 프로퍼티를 쓸 수 있는데요,

class Car {
    static MAXIMUM_NAME_LENGTH = 5;
}

혹시 js 문법상 지원이 되는 것을 알고 계시는데 사용하지 않고 class 외부의 상수로 따로 선언하신 거라면 이유가 궁금합니다!
이 부분도 개발자 개인의 기준에 따르면 된다고 생각하는데요.
다만 저는 요 경우에는 MAXIMUM_NAME_LENGTH을 딱히 따로 export 해서 외부에서 다른 의미로 사용하고 있는 것도 아니고, Car에만 사용될 숫자 기준이니 static으로 선언해서 Car.MAXIMUM_NAME_LENGTH처럼 쓰는 것도 (이후에 다른 곳에서 자동차의 이름 길이 기준을 가져다 쓰게 된다면) 선호하는 편이어서 의견 교류 차 남겨보아요 🙂
물론 지금 상태도 좋습니다 :)
-> 근데 쓰다보니 다른 모듈에서 사용할 게 아니라면 '굳이...?' 라는 생각이 드네요ㅋㅋㅋ 다른 곳에서 사용하게 되는 경우에는 의미를 드러내기 더 좋다는 생각이 들어서 남겨보았었습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호.. 아마 이 부분에서 스타일이 갈릴 것 같아요.
저는 상수의 경우 외부에서 참조하지 않는 한, class 내부의 static을 사용하지 않고 외부로 빼는 편을 더 선호합니다.
지금은 문제되지 않는 수준이지만, 종종 인스턴스 변수와 함께 있는 것보다는 어느 정도 물리적인 거리가 있는 편이 가독성에 더 좋기 때문이라고 생각해서요! 😄

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상수를 최상위 수준에서 선언할지, 아니면 클래스 내부에서 선언할지는 코틀린에서도 논쟁의 여지가 있는 영역입니다. 저는 상수를 선언할 위치를 결정할 때 다음과 같은 기준을 사용합니다.

  1. 해당 클래스 내에서만 사용하는 경우 클래스 내에서 비공개로 선언합니다.
  2. 같은 파일에 있는 여러 클래스에서 참조하는 경우 파일 내에서 비공개로 선언합니다.
  3. 클래스 외부에서 사용되지만, 컨텍스트를 전달해야 하는 경우(=외부에서 더 쉽게 참조할 수 있도록) 클래스 내에서 공개로 선언합니다.
  4. 클래스 외부에서 사용되지만, 컨텍스트를 전달할 필요가 없는 경우 파일 내에서 공개로 선언합니다.

이 기준에 따르면 위의 경우는 1번에 해당하므로 @woowapark 의 제안을 따라야 하는데, name.length > MAXIMUM_NAME_LENGTH의 코드가 name.length > Car.#MAXIMUM_NAME_LENGTH로 바뀌게 되더라고요. 왜 더 보기 흉할까요? 🤔

src/index.js Outdated
const retryAsking = require("./utils/retryAsking");

const CAR_NAME_SEPARATOR = ",";
const MINIMUM_MOVEMENT_CONDITION = 4;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참고용)
디렉토리를 나눈다면 constants/ 혹은 한 파일에 다 모은다면 constants.js와 같이 나누어서 상수들을 따로 관리하는 방식도 자주 쓰입니다 :)

리액트 기반으로 개발할 때에도 흔히 사용하는 폴더 구조라서 함께 남겨두어요.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹은 이 경우에는 randomMovementStrategy를 위한 값이니 class 파일 내에 관련 상수를 두신 것처럼 randomMovementStrategy도 상수와 세트로 분리하는 방법도...ㅋㅋㅋㅋㅋ

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +12 to +13
rl.close();
callback(answer);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(글로 적다 포기한 흔적)
오프라인에서 설명드리겠습니다!ㅋㅋ

module.exports = {
createCars,
filterByPosition,
maxByPosition,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 딱히 언어와 무관하게...의견이 궁금하여 남깁니당.
다시 읽다보니 filterByPosition, maxByPositionRacing 이 가지고 있어도 이상하지 않다는 생각이 들었어요.
Racing 외에서는 사용되지 않고, 그렇다고 도메인과 상관없는 유틸성 함수라고 보기도 어려운 것 같아서요.
Racing 내 private 메서드로 두는 건 어떻게 생각하시나요?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • filterByPosition()maxByPosition()Racing으로 옮기는 것은 괜찮다고 생각하지만, Racing에서만 사용되기 때문에 비공개로 선언되어 테스트하기가 더 어려워질 수 있습니다. 이 문제를 어떻게 극복할 수 있을까요? 예를 들어, findWinners()를 테스트하는 것으로 충분하다고 생각할 수 있습니다.
  • 자바나 코틀린으로 설계한다고 가정하면 현재 cars.js는 아래의 Cars와 같은 자동차 배열을 포함하는 일급 컬렉션 역할을 하게 될 거예요.
    classDiagram
        Racing --> Cars
        Racing --> AttemptCount
        Cars --> Car
    
    Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 개인적으로는 현재 기준에서는 findWinners()를 테스트하는 것으로 충분하다고 생각하였습니다! 개인마다 의견이 다를 수 있는 부분일 것 같아요.
    • findWinners() 외의 로직에서는 사용되지 않으니, 공개 메서드인 findWinners()를 테스트하는 것으로 충분히 검증할 수 있다고 생각했습니다.
  • 일급 컬렉션 개념이 JS에서 잘 적용되지는 않지만, 필요하다면 첨부해주신 것처럼 설계하는 것도 문제되지는 않는다고 생각했어요. 다만 JS는 멀티 패러다임 언어이고, 이미 잘 구현해주신 것처럼 꼭 클래스를 만들지 않아도 함수로 구현할 수 있는 부분들은 함수를 잘 활용하는 것이 더 언어 철학에 맞게 사용하는 방향일 것 같습니다.

요약: 현재 구현에 문제가 있어서 코멘트를 단 것은 아니고, 의견이 궁금했습니다. + 하지만 옮긴다면 findWinners()만 테스트되어도 충분하다고는 생각합니다ㅋㅋ!

Copy link
Collaborator

@woowahan-cron woowahan-cron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자바스크립트 레벨1 강의를 대신하셔도 될 정도로 코드가 정갈해요!
고생 많으셨습니다🙂
공원이 많은 부분 피드백을 해주셔서 제가 추가적으로 드릴 부분은 없는 것으로 보여요.

이후 Promise API, async-await를 적용하는 것은 다음 단계에서 진행하시나요?
파이팅입니다!

src/Car.js Outdated
@@ -0,0 +1,36 @@
const MAXIMUM_NAME_LENGTH = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호.. 아마 이 부분에서 스타일이 갈릴 것 같아요.
저는 상수의 경우 외부에서 참조하지 않는 한, class 내부의 static을 사용하지 않고 외부로 빼는 편을 더 선호합니다.
지금은 문제되지 않는 수준이지만, 종종 인스턴스 변수와 함께 있는 것보다는 어느 정도 물리적인 거리가 있는 편이 가독성에 더 좋기 때문이라고 생각해서요! 😄

}

function main() {
askCarNames();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그러고 보니

  • 5기: 프리코스와 자동차경주 모두 콜백 방식으로 진행 but 자동차 경주를 콜백 방식으로 진행하라는 별도의 제한을 두지 않았었던 것으로 기억해요.
  • 6기: 프리코스부터 async-await 방식으로 진행했으니 이번 자동차 경주에서도 콜백이 아니라 async-await 방식으로 진행하는 편이 좋을지, 어떻게 Promise API와 함께 비동기 강의를 구성해 보면 좋을지도 고민이네요!

src/Car.js Outdated
Comment on lines 20 to 28
if (
typeof condition === "number" &&
condition >= MINIMUM_MOVEMENT_CONDITION
) {
this.#position++;
}
if (typeof condition === "function" && condition()) {
this.#position++;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else를 이용한 것이 명시적일 수 있는 경우도 있다고 생각해서 else를 굳이 사용하지 말라는 언급은 빼도 될 것 같습니다.
그런데 이전에 else를 사용하지 않는다 또는 지양하라는 의미는 가독성을 해치지 말라는 의도로 이해했었어요.

  1. Early Return 패턴을 이용한 가독성 개선
  2. 아래와 같은 복잡한 코드 구성을 지양
    function foo() {
        if (조건1) {
            if (조건2) {
                if (조건3) {
                    if (조건4) {
                    } else {
                        throw new Error();
                    }
                } else {
                    throw new Error();
                }
            } else {
                throw new Error();
            }
        } else {
            throw new Error();
        }
    }

그런데 2번의 경우 프로그래밍 요구사항에서 함수 들여쓰기 수준을 제한하고 있어서 굳이 금지할 필요가 없겠다? 싶습니다.
혹시 제가 생각하는 의도로 금지하셨던 이유가 아니었다면? else 사용금지 언급 여부와 무관하게 그 이유도 제가 알면 좋을 것 같아요 ㅎㅎ

Copy link

@woowapark woowapark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

오프라인으로 얘기한 뒤에 approve를 잊고 있었군요...😂
내가해냄 최고심과 함께 폴리글랏 제이슨의 첫 스텝을 홀가분하게 떠나보내드리옵니다 👍👍👍

@@ -1,0 +1,49 @@
const Car = require("../src/Car");

describe("자동차는", () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오프라인에서 잡담하면서 같이 공유드렸지만... 기록 차원에서 남겨둡니당 :)
맞아요 실제로 IntelliJ IDEA 및 WebStorm 무료 버전을 사용하는 크루들도 많습니당
다만 VSCode도 꼭 무료여서 커리큘럼 내 공식 도구(?)로 강의 때 사용하고 있다기 보다는

  • 실제 실무 개발팀에서 일할 때에도 IntelliJ IDEA(혹은 WebStorm), VSCode 가 모두 사용되는 편
  • TypeScript가 사실상 표준처럼 사용되면서, TypeScript의 관리 주체인 MS가 함께 관리하고 있는 VSCode에서 TS관련 지원이 빠르게 적용되는 점

도 고려 대상일 것 같습니다. 그냥 주변에서 본 케이스들을 주관적으로 나누어보자면ㅋㅋ IDE 자체에서의 리팩터링 기능 등을 조금 더 많이 활용하고 싶어하시는 분들은 JetBrain 계열을, TS관련으로 넘어왔다가 오픈소스로 제공되는 각종 플러그인들을 설치해서 세팅하기 선호하시는 분들은 VSCode를 많이 쓰시는 것 같았어요.

module.exports = {
createCars,
filterByPosition,
maxByPosition,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 개인적으로는 현재 기준에서는 findWinners()를 테스트하는 것으로 충분하다고 생각하였습니다! 개인마다 의견이 다를 수 있는 부분일 것 같아요.
    • findWinners() 외의 로직에서는 사용되지 않으니, 공개 메서드인 findWinners()를 테스트하는 것으로 충분히 검증할 수 있다고 생각했습니다.
  • 일급 컬렉션 개념이 JS에서 잘 적용되지는 않지만, 필요하다면 첨부해주신 것처럼 설계하는 것도 문제되지는 않는다고 생각했어요. 다만 JS는 멀티 패러다임 언어이고, 이미 잘 구현해주신 것처럼 꼭 클래스를 만들지 않아도 함수로 구현할 수 있는 부분들은 함수를 잘 활용하는 것이 더 언어 철학에 맞게 사용하는 방향일 것 같습니다.

요약: 현재 구현에 문제가 있어서 코멘트를 단 것은 아니고, 의견이 궁금했습니다. + 하지만 옮긴다면 findWinners()만 테스트되어도 충분하다고는 생각합니다ㅋㅋ!

Comment on lines +12 to +13
rl.close();
callback(answer);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

설명 완료!ㅋㅋ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants