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

[Hwirin-Kim]

과제 체크포인트

배포링크

https://hwirin-kim.github.io/front_6th_chapter2-1/

기본과제

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

심화과제

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

과제 셀프회고

이번 basic과제를 완료하고 눈으로 동작이 다 잘되는것을 확인 한 후 테스트를 마지막으로 돌려보니 딱 하나의 테스트 케이스가 동작하지 않는걸 발견했습니다.

"진짜 이제 다했다!!" 라는 생각이였다가 막혀서 그런지 실제로 한 시간 정도 막혀있었는데 체감은 두 세시간은 막혀있던 기분이였습니다.

문제는 innerHTML로 DOM이 통째로 재렌더링되면서 버튼 요소가 새로 생성되는데, 테스트 코드에서 버튼을 변수로 캐시해두면 해당 버튼을 누르지 않아 제대로 동작하지 않는것을 발견했습니다.
그래서 버튼과 수량 요소는 클릭 직전에 querySelector로 다시 가져와야 정상 작동한다는 것을 배웠는데 그것을 새벽에 블로그에 잘 정리해뒀습니다.. 링크 : https://huirin.tistory.com/250

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

1. 첫 번째로 과제를 진행하며 제일 신경쓴 부분은 내가 의도하고 변경하고자 하는 부분을 변경하고 나서 테스트코드로 검증을 하는것이였습니다.
   너무 별거 아니면서 충분히 예측 가능하다고 하더라도 일정 부분 수정을 하고 나면 반드시 테스트로 검증을 진행했습니다.

2. 두 번째로 신경 쓴 부분은 너무 한번에 다 바꾸려고 하지 않기였습니다.
   실제로 제일 처음엔 UI를 바로 분리시키고 값들을 나중에 넣어도 되지 않을까? 라고 생각했지만,,
   너무 강하게 결합되어있는 UI와 비즈니스로직들에 의해 AI도 해결책을 주지 못하였습니다.

그렇게 한 번 완전히 첫 커밋으로 돌아가 이번엔 스토어를 만들고 상태부터 분리하려고 하였습니다.
그러나 스토어를 만드는것 역시 쉽지만은 않았는데, 코드 양이 너무 많아서 어디에서 어떻게 사용되고 있는지 잘 알지 못했기 때문에 어디에서 어떤 액션함수를 만들어서 써야할지 한 눈에 보이지 않았기 때문입니다.

그렇게 시행착오를 겪다가 마지막으로 시도한 방법은 스토어와 유사한 모양이지만, 그냥 단순히 객체야! 하는 느낌으로 전역변수 부분의 형태만 객체 형태로 바꾸고, 그에 알맞게 사용 하고 있는 곳을 따라가서 이름만 바꿔줬습니다.

export const Store = {
  products: {
    list: [
      {
        id: productIds.p1,
        name: "버그 없애는 키보드",
        val: 10000,
        originalVal: 10000,
        q: 50,
        onSale: false,
        suggestSale: false,
      },
      {
        id: productIds.p2,
        name: "생산성 폭발 마우스",
        val: 20000,
        originalVal: 20000,
        q: 30,
        onSale: false,
        suggestSale: false,
      },
      {
        id: productIds.p3,
        name: "거북목 탈출 모니터암",
        val: 30000,
        originalVal: 30000,
        q: 20,
        onSale: false,
        suggestSale: false,
      },
      {
        id: productIds.p4,
        name: "에러 방지 노트북 파우치",
        val: 15000,
        originalVal: 15000,
        q: 0,
        onSale: false,
        suggestSale: false,
      },
      {
        id: productIds.p5,
        name: `코딩할 때 듣는 Lo-Fi 스피커`,
        val: 25000,
        originalVal: 25000,
        q: 10,
        onSale: false,
        suggestSale: false,
      },
    ],
  },
  ui: {
    selectedProductId: null,
    cartItems: [],
    totalAmount: 0,
    itemCount: 0,
  },
};

위처럼 최초에는 상태관리 스토어처럼 액션에 의해 변경해야하거나 그렇지 않고, 전역에 선언된 객체일 뿐이였습니다. 그치만 그렇게라도 하고나니 조금씩 분리할 수 있는 단위도 보였고, "굳이 DOM에서 가져와야 하는 부분인가??" 하는 부분들도 보이기 시작했습니다.

그 부분이 보이기 시작할 때 부터 스토어의 필요성을 느꼈고,

export const ACTION_TYPE = {
  QUERY: "QUERY",
};

/**
 * 스토어 생성
 * @param {Object} initialState - 초기 상태
 * @param {Object} actions - 액션 함수들
 * @returns {Object} 스토어 객체
 */
export const createStore = (initialState, actions = {}) => {
  let state = initialState;
  const listeners = [];

  // 스토어 객체 생성
  const store = {
    getState: () => state,
    setState: (newState) => {
      state = newState;
      listeners.forEach((listener) => listener());
    },
    subscribe: (listener) => {
      listeners.push(listener);
      return () => {
        listeners = listeners.filter((l) => l !== listener);
      };
    },
  };

  // action 함수들을 store에 바인딩
  const boundActions = {};
  Object.keys(actions).forEach((actionName) => {
    boundActions[actionName] = (...args) => {
      const result = actions[actionName](state, ...args);

      if (!result) return;

      // 모든 액션은 상태 변경을 위한 것이므로 결과를 새로운 state로 설정
      if (typeof result === "object" && result.type === "QUERY") {
        return result.data;
      }
      store.setState(result);
      return result;
    };
  });
  return {
    ...store,
    ...boundActions,
  };
};

단순한 전역 객체에서 시작한 스토어는, 점차 상태와 로직을 분리할 필요가 생기며 위와같이 createStore 구조로 발전했습니다.
그 후 액션 함수에서 조회 기능도 필요해지면서 ACTION_TYPE.QUERY 같은 프로토콜을 붙이게 되었고, 점차 미니 상태 관리 라이브러리처럼 자라나게 되었습니다.

이처럼 처음부터 모든걸 하려고 했다면 보이지 않았을 것들을 작게보면 잘 보인다는것을 깨달았고, 다시 한번 요약해보면 가장 중요하다고 느낀것은 *테스트이고 그 다음은 작은 단위부터 변경하기 입니다!

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

####일단 회고를 한번 해보겠습니다..
사실 이번 과제를 진행하면서 DOM에서 정보를 얻어오는 대신 스토어를 만들어서 스토어로 데이터를 보관하는 역할을 만드는데 4일, 그리고 이제 역할이 줄어든 DOM을 분리하는데는 5시간이 채 안걸린것 같습니다.
물론 4일 안에는 수많은 리셋과 그 만큼 어떤 방향이 좋은 방향인지를 찾아가는 시간이 제일 많이 들었고, DOM에서 어떤 정보를 얻어서 비즈니스로직을 만드는 코드들을 분리하는데 두 번째로 많은 시간이 들었습니다.
이 부분들은 특히 AI를 거의 쓰지 않고 진행하여 더욱 시간이 많이 들었는데, 오히려 AI를 쓰지 않을 때 부터 작은 단위로 분석하기 시작하여 집중도도 높아지고 무언가 해결이 되는 기분이 들어 몰입이 되는 시간이였습니다.

그렇게 DOM과 비즈니스로직을 어느정도 떼어낸 후 부터는 AI에게 맡기면 정말 제가 예상한대로 잘 분리가 되었습니다. 그래서 그 뒤로 몇 시간 지나지 않아 basic과제를 마칠 수 있었습니다.

다시하면 더 잘해볼 수 있겠다, 아쉽다 하는 부분

리액트로 변환하는 과정은 사실상 AI 80%정도 라고 생각합니다. 일단 UI를 그리라고 시켰더니 거의 유사한 그림이 나왔고, 그 다음 테스트 코드를 복사해오라고 시켰더니 테스트를 리액트에 알맞게 가져왔고, 할인 및 계산 로직을 가져오라고 시켰더니 가져오고.. 이렇게 순차적으로 그저 시켰는데 정말 잘 가져왔습니다. 물론 과제 제출의 욕심이 있었기에 스스로 하기보다 처음부터 AI를 쓸 생각으로 진행했지만... basic부분을 하루라도 빨리 끝냈더라면 리액트로 변환하는 과정도 손수 진행해보고 싶은 마음이 들었습니다..
그냥 마음속으로는 내가 basic을 잘 만들어놔서 AI가 쉽게 가져온걸꺼야.. 라고 생각하는 중이긴 합니다..!!

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

이번 리팩토링 과제를 진행하며 궁금했던 부분은 과연 함수를 어느정도까지 쪼개야 하는 걸까? 하는 것입니다.

예시로 어떤 막대하고도 심오한 일을 하는 함수가 있는데, 이 함수를 한 네 가지 정도의 일처리로 분류를 할 수 있었다고 가정해보겠습니다.
그런데 그렇게 분류된 네 개의 일처리 함수 중 일부는 2~3가지로 또 분류 될 만한 함수였습니다.

그런식으로 최초의 큰덩어리에서 그 밑 작은 일 단위로 보면 네 가지, 그런데 그 각각의 함수를 또 분류하려고 보면 2~3가지

이렇게 최소 단위까지 분리한걸 결국 어떤 함수들은 조립자 역할만 하게 될것이고, 그 조립자들을 또 조립한 거대한 함수를 만들어야 최초에 막대하고 심오한 일을 할 수 있는 함수가 될텐데, 그렇다면 이 함수는 한 가지 일을 하는 함수가 맞는 건가요..?

어느정도까지 함수의 일처리 단위를 쪼개야하는지 아직 잘 감이 오지 않습니다..

[Yangs1s]

휘린님 글 너무 잘읽었습니다
문제를 직면하고 해결하고 하는게 잘 보여지는 글인거 같고 인상적이네요 새벽에 블로그에 정리까지

[chan9yu]

멋찌다 일주일동안 고생했어여

[creco-hanghae]

이러면 더 쉽게 구할 수 있어요

Suggested change

	let hasKeyboard = false;
	let hasMouse = false;
	let hasMonitorArm = false;
	// 상품 조합 보너스 포인트 계산
	items.forEach((item) => {
	switch (item.id) {
	case PRODUCT_IDS.p1:
	hasKeyboard = true;
	break;
	case PRODUCT_IDS.p2:
	hasMouse = true;
	break;
	case PRODUCT_IDS.p3:
	hasMonitorArm = true;
	break;
	}
	});
	const hasKeyboard = items.some(x => x.id === PRODUCT_IDS.p1);
	const hasMouse = items.some(x => x.id === PRODUCT_IDS.p2);
	const hasMonitorArm = items.some(x => x.id === PRODUCT_IDS.p3);