[4팀 김수민] Chapter 2-1. 클린코드와 리팩토링

[nimusmix]

과제 배포링크

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

과제 체크포인트

기본과제

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

심화과제

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

과제 셀프회고

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

1. 팀 코드 컨벤션 정하기
월요일 코어 타임에 모여서 페어팀 코드 컨벤션을 정했어요.
내가 안 쓰는 컨벤션의 경우에는 거침없이 NAGA를 외치며.. 정해보았는데요!
정해진 컨벤션은 다음과 같았습니다.

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

• "tabWidth": 2
  찬성 의견: 2가 근본이다.
  반대 의견: (수줍어서 말을 하지 않으셨다.)
  정말 딱 한 분 제외하고 모든 사람이 2였지만 용훈님이 수줍게 4를 외쳤다. 따가운 시선들이 한 곳으로 모였다.

• "semi": true,
  찬성 의견: 코드 블럭과 코드 블럭 사이 경계가 잘 보인다, 근본이다.
  반대 의견: 없는 게 더 깔끔하다.
  여기서 반대한 게 저였는데, 깔끔해서..도 있지만 자바스크립트가 알아서 잘 해주기 때문에!
  발생하지 않을 문제를 혹시 발생할 수도 있다는 이유만으로 성가시게 할 필요 없다고 생각했습니다~
  → 세미콜론은 사용하는 것으로 결론이 났다.

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

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

저는 원래는 축약어파였어요! 그게 깔끔하고 치기도 쉬우니까요~
그런데 저는 영어에 좀 친숙한 사람이었기에,, 제가 쓰는 축약어를 다른 사람들이 모를 때도 많았어요
모두가 아는 축약어는 어디까지인가? 어떤 축약어는 써도 되고 어떤 축약어는 안 되는가?
이런 걸 고민하기 보다 어떤 동료든 알 수 있도록, 적어도 몰랐을 때 검색해서 쉽게 찾을 수 있도록 하는 것이 중요하다 생각이 들었고
NO 축약어파로 바꼈습니다!

• if 문에는 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_]*$'],
        },
      ],

      // 팀 컨벤션 - 구조분해할당 권장
      '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,
];

2. 중간 코드 리뷰
: 화요일 테오 Q&A 세션 이후 팀원들은 혼돈의 도가니에 빠졌었습니다 (ㅋㅋ)
그래서 다들 어떻게 진행하고 있는지 확인하고 모르는 것은 공유하기 위해서 중간 코드 리뷰를 진행했어요.
한 사람씩 돌아가면서 화면 공유를 키고, 어디까지 진행했는지, 앞으로는 어떻게 할 예정인지 이야기를 나눴습니다.
다들 이 시간을 통해 혼란했던 마음을 다잡고 과제를 할 수 있었어요 !!

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

1. DOM API를 그대로 사용한 부분이 아쉬워요!
   : 제가 과제를 진행한 순서는 변수 선언자 및 변수명 정리 -> 위에서부터 아래로 읽으며 코드 정리 였는데,
   좀 더 리액트스럽게 하는 아키텍처에 신경썼다면 좋았을텐데 하는 아쉬움이 있습니다!

2. AI를 많이 사용했던 점이 아쉬워요!
   : 클린코드는 항해 플러스 시작하기 전부터 가장 기대했던 주차인데,
   제 손으로 하나하나 뜯어고치지 못했단 점이 너무나 아쉽습니다ㅠㅠ

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

테오가 doubleQuote를 선호하는 이유는 무엇인가요? 궁금해요 !!!

[eveneul]

배낄래요.
수민 공주님 파이팅~

[ckdwns9121]

오 수민님 코드 왜케 깔끔해요

[nimusmix]

페어4팀 코드 리뷰

상태에 따라 UI를 업데이트하는 부분을 어떻게 처리할지 고민이 많았는데, 다른 분들은 어떻게 하셨는지 궁금해요!
제가 한 건 리액트스럽지 않은 방법인 것 같아서(..) ㅎㅎ

[BangDori]

React스럽지 않은 방법이라고 말씀해주셨는데, 수민님이 생각하신 React 스러운 방법은 어떤건가요??

[eveneul]

className으로 hidden, show 처리 하는 거 좋다고 생각해요. 지금은 바닐라니까 돔을 뺐다 꼈다 하면 렌더링이 되어야 하고, 중요하지 않은(단순 UI 같은 경우)는 css 하는 게 더 간편하다고 생각이 듭니다. hidden이라는 클래스가 어떤 css를 바라보는지는 모르겠지만..! 만약 접근성을 따진다고 하면 aria-hidden도 넣어 주면 어떨까요오?

[nimusmix]

@BangDori 상태에 따른 UI 업데이트를 각 컴포넌트에서 처리하는 거요!

[nimusmix]

@eveneul 제가 여쭤봤던 건 className에 따라 처리하는 방식이 괜찮은지는 아니었고,
상태 변경에 따른 UI 업데이트를 별도의 함수가 담당하는 구조를 말씀드린 거였어요!

[geonhwiii]

전체적인 코드를 보진 못했지만,

updateCartUI 내부에서 로직을 전체구현하는 것보단 분리되는 것이 더 좋아보여서 저는 괜찮아보입니다!

지금 생각이 드는건, 특정 행동 시 명확히 UI가 변경되는 것을 알 수 있다면, 저는 어느 방향이든 괜찮다고 생각이 드네요~!

[Elli-Lee]

저도 비슷하게 했던 것 같아요!
지금처럼 CSS로 처리하는게 DOM 조작이 적어서 효율적일 것 같아서 그렇게 했습니다.
DOM 조작을 최소화 한다는 점에서는 방식은 다르더라도 오히려 리액트가 추구하는 방향과 가까울 수 있을것 같아요!
(제가 잘 못 이해한 걸 수도 있지만요...!)

[esoby]

저는 상태를 스토어로 관리했어요! dispatch action이 돌아가고 나면 상태와 관련된 element 렌더링을 다시 시키는 흐름으로 UI 업데이트를 했숩니다! original main 코드가 여기저기서 UI를 변경하는 게 머리 아파서 싸악 모아버렸는데요. 큰 흐름이 명확해서 전체적으로는 낫밷이었지만 사용 범위가 좁은 지역 상태값도 스토어에 포함되어서 불필요하게! 불필요한 부분까지! 렌더링 될 때가 있도라구요 수민님 코드 보다보니 갑자기 반성 타임 됨 헤헤

[BangDori]

@nimusmix

개인적인 의견으로는, "React스럽게 구현한다"는 건 단순히 컴포넌트처럼 구조화하는 걸 넘어서, React 내부에 내장된 생명주기, 상태 변화 감지, 렌더링 최적화 같은 선언적 프로그래밍 흐름을 구현해야 한다는 뜻이라고 생각해요.

하지만 이번 과제는 바닐라 JS 기반이라 그런 내부 코어 로직까지 구현하는 건 현실적으로 쉽지 않았고, 그래서 저는 컴포넌트 단위로 책임을 나누고, UI와 비즈니스 로직, 이벤트 핸들러를 분리해서 최대한 구조적인 코드로 작성하려고 했어요.

오히려 이번 과제는 React 흉내내기가 아니라, MVC처럼 역할을 나누고 클린한 코드를 지향하는 방향이 더 핵심이 아니었나 싶어요!

---

저도 처음에 "각 컴포넌트에서 이벤트 핸들러를 가지고 있으면 좋지 않나? 이를 어떻게 해결할 방법이 없을까?"를 고민을 했는데, 어느순간 스스로가 React에 너무 매몰된 사고를 하고 있지는 않나?하는 생각이 들긴 햿어요..

[eveneul]

아! 이해했어요. 병준 님 말씀처럼 지금 상황에서는 수민 님이 작성해 주신 코드가 제일 적합하고 깔끔한 구조라고 생각돼요.