과제 체크포인트

https://eveneul.github.io/front_6th_chapter2-1/

기본과제

• 코드가 Prettier를 통해 일관된 포맷팅이 적용되어 있는가?
• 적절한 줄바꿈과 주석을 사용하여 코드의 논리적 단위를 명확히 구분했는가?
• 변수명과 함수명이 그 역할을 명확히 나타내며, 일관된 네이밍 규칙을 따르는가?
• 매직 넘버와 문자열을 의미 있는 상수로 추출했는가?
• 중복 코드를 제거하고 재사용 가능한 형태로 리팩토링했는가?
• 함수가 단일 책임 원칙을 따르며, 한 가지 작업만 수행하는가?
• 조건문과 반복문이 간결하고 명확한가? 복잡한 조건을 함수로 추출했는가?
• 코드의 배치가 의존성과 실행 흐름에 따라 논리적으로 구성되어 있는가?
• 연관된 코드를 의미 있는 함수나 모듈로 그룹화했는가?
• ES6+ 문법을 활용하여 코드를 더 간결하고 명확하게 작성했는가?
• 전역 상태와 부수 효과(side effects)를 최소화했는가?
• 에러 처리와 예외 상황을 명확히 고려하고 처리했는가?
• 코드 자체가 자기 문서화되어 있어, 주석 없이도 의도를 파악할 수 있는가?
• 비즈니스 로직과 UI 로직이 적절히 분리되어 있는가?
• 코드의 각 부분이 테스트 가능하도록 구조화되어 있는가?
• 성능 개선을 위해 불필요한 연산이나 렌더링을 제거했는가?
• 새로운 기능 추가나 변경이 기존 코드에 미치는 영향을 최소화했는가?
• 코드 리뷰를 통해 다른 개발자들의 피드백을 반영하고 개선했는가?
• (핵심!) 리팩토링 시 기존 기능을 그대로 유지하면서 점진적으로 개선했는가?

심화과제

• 변경한 구조와 코드가 기존의 코드보다 가독성이 높고 이해하기 쉬운가?
• 변경한 구조와 코드가 기존의 코드보다 기능을 수정하거나 확장하기에 용이한가?
• 변경한 구조와 코드가 기존의 코드보다 테스트를 하기에 더 용이한가?
• 변경한 구조와 코드가 기존의 모든 기능은 그대로 유지했는가?
• (핵심!) 변경한 구조와 코드를 새로운 한번에 새로만들지 않고 점진적으로 개선했는가?

과제 셀프회고

과제를 하면서 내가 제일 신경 쓴 부분은 무엇인가요?

다른 학습메이트분들이 클린 코드 해야 할 양이 방대하니, 마음을 준비(?)하라고 했는데, 정말, 생각보다, 대박, 양이 너무 많아서.. 어디에서부터 어떻게 손을 대야 할지 감이 안 잡혔다. 일단 월요일 페어 팀(4팀 + 7팀) 코어 타임 때 모여서 페어 팀만의 코드 컨벤션을 정했다. 내용은 아래와 같다.

참고로 코드 컨벤션을 정할 때 의견은 대다수가 이기는 걸로 컨벤션을 정했다.

• singleQuote 찬성 의견: 쉬프트 키까지 쓰는 게 손가락이 아프다, 쌍따옴표는 다른 데에서 사용하기 위함이다 반대 의견: 처음부터 쌍따옴표로 개발을 시작해서 그런지 너무 익숙해졌다. (심지어 나(오하늘)만 반대함)

→ 결국 singleQuote은 true로 결정났다.

• tabWidth 찬성 의견: 2가 근본이다. 반대 의견: (수줍어서 말을 하지 않으셨다.)

정말 딱 한 분 제외하고 모든 사람이 2였지만 7팀 용훈 님이 수줍게 4를 외쳤다. 따가운 시선들이 한 곳으로 모였다. → tabWidth는 2로 결정났다.

• 세미콜론 사용 여부 찬성 의견: 코드 블럭과 코드 블럭 사이 경계가 잘 보인다, 근본이다. 반대 의견: 없는 게 더 깔끔하다.

세미콜론 사용 여부도 딱 한 분만 세미콜론을 사용하지 않으셨다. → 세미콜론은 사용하는 것으로 결론이 났다.

• var, 동등연산자(==) 찬성 의견이 없었다. 찬성한다면 젭 나가라고 호통 쳤다.

• 축약어 찬성 의견: 변수, 함수명이 길면 읽느라 업무 시간 다 끝난다. 모두가 알 수 있는 건 줄여 쓰는 것도 나쁘지 않다. 반대 의견: 알아듣지 못한다. 길게 쓰는 게 간지(?)다.

나는 button을 btn으로, calculator를 cal로 사용한다. 이유는 사람들이 이미 많이 사용하고 익숙한 것들에 대해 길게 작성할 필요가 없다고 생각하기 때문이다. 그런데 말입니다...

최근에 회사에서 프로젝트를 진행하면서 title을 tit로 줄여 쓰시는 분을 봤다. tit의 뜻은 직접 설명하기 조금.. 부끄러우므로 링크로 대신하겠다. (링크)

이 에피소드를 생각하니 축약어를 조금씩 줄여 나가야겠다는 생각이 들었다..... 하지만 쉽지는 않을 것 같다.

• early return 찬성 의견: 그게 깔끔하다. 간지(?)다. 반대 의견: 반대 의견이 없었음.

위와 같은 코드 컨벤션을 정하고 prettier와 esLint를 수정하고 팀원분들에게 공유했다.

// prettier
{
  "semi": true,
  "trailingComma": "es5",
  "singleQuote": true,
  "printWidth": 80,
  "tabWidth": 2,
  "useTabs": false
}

// eslint.config.js
import typescript from '@typescript-eslint/eslint-plugin';
import typescriptParser from '@typescript-eslint/parser';
import prettier from 'eslint-config-prettier';
import compat from 'eslint-plugin-compat';
import cypressPlugin from 'eslint-plugin-cypress';
import importPlugin from 'eslint-plugin-import';
import eslintPluginPrettier from 'eslint-plugin-prettier';
import react from 'eslint-plugin-react';
import reactHooks from 'eslint-plugin-react-hooks';
import vitestPlugin from 'eslint-plugin-vitest';
import globals from 'globals';

export default [
  {
    ignores: ['**/node_modules/**', 'dist/**'],
  },
  {
    files: ['**/*.{js,jsx,ts,tsx}'],
    languageOptions: {
      ecmaVersion: 'latest',
      sourceType: 'module',
      globals: {
        ...globals.browser,
        ...globals.es2021,
        Set: true,
        Map: true,
      },
      parser: typescriptParser,
      parserOptions: {
        ecmaFeatures: {
          jsx: true,
        },
        tsconfigRootDir: '.',
      },
    },
    plugins: {
      prettier: eslintPluginPrettier,
      react,
      'react-hooks': reactHooks,
      '@typescript-eslint': typescript,
      compat,
      import: importPlugin,
    },
    settings: {
      react: {
        version: 'detect',
      },
      browsers:
        '> 0.5%, last 2 versions, not op_mini all, Firefox ESR, not dead',
    },
    rules: {
      // Prettier 통합 규칙
      'comma-dangle': [
        'error',
        {
          arrays: 'always-multiline',
          objects: 'always-multiline',
          imports: 'always-multiline',
          exports: 'always-multiline',
          functions: 'never',
        },
      ],

      // React 관련 규칙
      'react/prop-types': 'off',
      'react/react-in-jsx-scope': 'off',
      'react-hooks/rules-of-hooks': 'error',

      // TypeScript 관련 규칙
      '@typescript-eslint/no-explicit-any': 'warn',

      // 팀 컨벤션 - var 사용 금지
      'no-var': 'error',
      '@typescript-eslint/no-unused-vars': 'error',

      // 팀 컨벤션 - 동등 연산자 (==, !=) 금지
      eqeqeq: ['error', 'always', { null: 'ignore' }],

      // 팀 컨벤션 - 얼리 리턴 권장
      'consistent-return': 'error',
      'no-else-return': ['error', { allowElseIf: false }],

      // 팀 컨벤션 - 템플릿 리터럴 규칙
      'prefer-template': 'error',
      quotes: [
        'error',
        'single',
        {
          avoidEscape: true,
          allowTemplateLiterals: false,
        },
      ],

      // 팀 컨벤션 - 상수는 대문자
      camelcase: [
        'error',
        {
          properties: 'never',
          ignoreDestructuring: false,
          ignoreImports: false,
          ignoreGlobals: false,
          allow: ['^[A-Z][A-Z0-9_]*#x27;],
        },
      ],

      // 팀 컨벤션 - 구조분해할당 권장
      'prefer-destructuring': [
        'error',
        {
          array: true,
          object: true,
        },
        {
          enforceForRenamedProperties: false,
        },
      ],

      // 기본 코드 품질 규칙
      'prefer-const': 'error',
      'arrow-body-style': ['error', 'as-needed'],
      'object-shorthand': 'error',
      'no-multiple-empty-lines': ['error', { max: 1, maxEOF: 0 }],
      'no-console': ['warn', { allow: ['warn', 'error'] }],
      'no-debugger': 'warn',
      'no-undef': 'off',

      // import 순서 규칙
      'import/order': [
        'error',
        {
          groups: ['builtin', 'external', ['parent', 'sibling'], 'index'],
          alphabetize: {
            order: 'asc',
            caseInsensitive: true,
          },
          'newlines-between': 'always',
        },
      ],
      'import/extensions': 'off',
    },
  },
  // 테스트 파일 설정
  {
    files: [
      '**/src/**/*.{spec,test}.[jt]s?(x)',
      '**/__mocks__/**/*.[jt]s?(x)',
      './src/setupTests.ts',
    ],
    plugins: {
      vitest: vitestPlugin,
    },
    rules: {
      'vitest/expect-expect': 'off',
    },
    languageOptions: {
      globals: {
        ...globals.browser,
        globalThis: true,
        describe: true,
        it: true,
        expect: true,
        beforeEach: true,
        afterEach: true,
        beforeAll: true,
        afterAll: true,
        vi: true,
      },
    },
  },
  // Cypress 테스트 파일 설정
  {
    files: ['cypress/e2e/**/*.cy.js'],
    plugins: {
      cypress: cypressPlugin,
    },
    languageOptions: {
      globals: {
        cy: true,
      },
    },
  },
  prettier,
];

결론은 팀원들끼리 정한 컨벤션을 맞추려고 했다. 조금 뜻대로 되지 않을 때도 있지만.. prettier와 eslint를 사용하여서 조금은 수월했던 것 같다.

과제를 다시 해보면 더 잘 할 수 있었겠다 아쉬운 점이 있다면 무엇인가요?

시간이 너무 부족했다. 물론 시간을 못 쓴 나의 책임도 있지만... 시간이 2-3일 정도 더 있었다면 심화 과제를 만족스럽게 마칠 수 있었을 것 같다. 결국 AI의 도움을 받았다. Ai 도움 없이는 이 방대한 레거시 코드들을 바꾸지 못할 것 같다.

처음에는 Class로 Store랑 Component를 만들었다. 파일 하나하나 만들어 가고 있던 중에 테오의 QNA가 있었고, 들으면서 나중에 리액트로 변경할 때 Class가 조금 어려울 것 같다는 생각이 들었다. 내가 너무 챕터 1처럼 하려고 했나? 생각이 들었다.

결국 갈아엎었다.

ui들을 다 ui.js에 한번에 몰아넣었다. 그리고 테오가 주었던 팁처럼

function Select({ id, className, options }) {
  return `
    <select id="${id}" class="${className}">
      ${options.map((option) => option).join('')}
    </select>
  `;
}

이런 식으로 ${} 안에 다른 컴포넌트들을 불러왔다.

또 문제가 생겼다. 이제 토큰 거의 다 바닥 났어요... 하는 AI를 붙잡고 이것저것 물어보면서 변경하는 중에, 이 친구가 이벤트 함수 리팩토링을 전혀 하지 못하는 것이다. 즉, 이벤트 함수만 만지기만 했다고 하면 테스트 코드 함수가 전부 fail 뜨는 것이다. (이럴 수가 있나?)

결국.. 이벤트 함수는 내가 작성했다..

1. 먼저 selItem이라는 변수를 변경하고, for문으로 된 반복문을 some으로 변경했다. (링크)

2. 그런데 some으로 쓰면 아래에 있는 for 반복문을 제거하기에는 조금 망설여졌다. 이 반복문의 존재 이유가 추가할 상품의 정보를 확인하기 위해서임을 깨달았기 때문에.. (링크)

3. 이미 추가한 상품인지, 아니면 새로운 상품인지, 그리고 재고가 없을 때 알럿이나 UI를 업데이트해 주었다. (링크)

과연 이게 보기 좋은 코드일지도 모르겠는데 죄악 같은 for문을 제거하니 게비스콘 먹은 마음이 조금씩 생겨났다.

수요일, 황준일 코치님의 멘토링에서 이런 코멘트를 달았다.

기존 레거시 코드에는 읽기도 힘든 for문이 너무 많았다. 이거를 map 같은 다른 함수들로 변경할 수 있을 텐데? 라는 생각이 들었는데, 그동안 개발 지식을 얕게 얻었다고 생각했지만 준일 코치님은 어느 정도 코드를 읽는 것에 숙련되어 있어서 for, if를 봤을 때 불편함을 느끼는 거라고 하셨다. 나.. 성장했구나..! 생각이 들었다. 아무튼 결론은, for문은 죄악이다.

리뷰 받고 싶은 내용이나 궁금한 것에 대한 질문 편하게 남겨주세요 :)

• 아직 폴더 구조를 어떻게 나누어야 할지 모르겠습니다. 회사에서 프로젝트를 진행하는데, 에이전시이다 보니까 유지/보수보다는 처음부터 구축할 일이 많은데요, component 폴더 안에 공통 컴포넌트는 또 common으로 나누어야 하나? 그런데 common으로 쓰다가 또 자주 쓰일 일이 없으면 어쩌지? 한 곳에서만 쓰일 줄 알았던 컴포넌트가 사실은 공통이면 어쩌지? 같은 생각과.. core 폴더보다는 ts라는 폴더로 사용한 경험이 많아서 익숙하지가 않습니다. 코치님께서는 어떤 기준으로 폴더를 나누시나요? 이건 다음 주차 때 나올 이야기가 맞지요?

• 정말 함수 하나하나 다 나누었습니다. 단일 책임의 원칙이라고는 하지만.. 너무 많은 책임을 준 건 아닐까 생각이 듭니다. 가령

function addNewItem(item) {
  cartDisplay.insertAdjacentHTML('beforeend', CartItem(item));
  item.quantity--;
}

이런 함수가 있는데요. 장바구니에 아이템을 추가하는 함수인데, 고작 두 줄밖에 되지 않습니다. 이럴 경우 따로 함수로 빼지 말고 그냥 이벤트 함수 안에서 쓰는 게 더 낫지 않을까요? 함수를 잘게잘게 찢어 놓으면 장점이 있을까요? 저는 이 파일, 저 파일 왔다갔다 보는 게 조금 힘들어서.. 코드 길이가 긴 것 아니면 한 곳에 쓰려고 하는데, 이게.. 클린한 게 맞을까 고민이 됩니다.