[Javascript] 클린코드와 리팩토링

항해 4주차에 들어서며 클린코드와 리팩토링 이라는 주제가 시작되었다.

 

 

내 과제가 우수 과제라구요..?

일단 한 가지 자랑이 있는데..

항해에선 한 주마다 과제를 평가하는 시스템이 있는데, 코치님이 과제 수행을 직접 평가하며 과제의 진행 과정과 수강생이 겪은 시행착오와 느낀점들을 PR에 잘 적어두면 그것을 같이 보며 평가하는 방식이다.

항해99 플러스 BP

1~3주차까지는 그저 PASS에 불과했는데 이번에는 무려 BP(Best Practice)를 받았다!!

이번 이번 클린코드 과제에서 내가 느낀바와 겪어나간 과정들, 그리고 진행했던 방향이 코치님의 출제 의도에 많이 부합했던것 같다.

 

 

 

이번 주차 과제는..?

그렇다면 이번 과제는 어떤 형식이었느냐..?"

클린코드 과제 원본의 일부

위 처럼 막 정의된 전역변수와 통일되지 않은 네이밍, 그냥 쓰인 매직넘버, 그리고 위 이미지에는 보이지 않지만 비즈니스로직과 DOM이 혼재 되어있고, 무려 한 파일에 정의된 사이트를 본인이 생각하는 클린코드에 알맞게 리팩토링 하는 과제였다.

또한 리팩토링이 완료 되면 그 코드를 가지고 다시 리액트로 작성하는 것이 심화 과제였다.

 

일단 이 과제를 진행하며 내가 생각한 해결해야하는 문제점은 다음과 같았다.

1. 한 페이지에서 var를 이용한 전역변수로 선언된  변수들
2. 네이밍의 불일치 (아이디가 PRODUCT_ONE이였다가 p2였다가 product_3.. 이런식으로 일치하질 않음)
3. 의도를 알 수 없는 매직넘버..
4. DOM과 분리되지 않은 데이터들 (실제 돔에서 추가된 카트 상품을 보고 역으로 데이터를 가져오는 코드였음)
5. 한 페이지에 모든 기능이 써있는 문제

이 정도 였다.

 

사실 누구나 위와 같은 문제점은 찾아낼 수 있다. (오히려 내가 덜 찾아낸걸수도..?)

하지만 어떻게 시작하느냐는 정말 다른 문제이다.

 

 

수없이 실패한 도전

일단 이번 과제를 '혹시나' 하는 마음에  커서로 "해줘" 해봤고, 리액트로 변경하는 심화과제또한 "해줘" 해봤다.

결과는 역시나 제대로 해주지 않았다.

일단 'AI가 왜 해내지 못했는가?' 에 대해선 AI개발자가 아니라서 모르겠다.

하지만 AI장기간 사용자로써 생각해보면.. AI는 이렇게 큰 덩치의 작업을 해내지 못하고, 또한 여러가지 일을 한 번에 시켰을 경우 꼭 한 두개씩 빼먹는 경우가 생기고, 또한 명확하지 않은 질문에는 의도와 다른 대답이 돌아오는 경우가 많다.

 

특히나 이렇게 하나만 잘못 바꿔도 고장나버릴듯이 강결합된 악성 코드는 당연히 될리가 없었다.

 

그래서 첫 번째로 시도한 방법은 일단 전역 변수들을 추려서 다른곳에 빼내보자! 하는 것이였다.

그렇게 하나씩 빼가기 시작하는데... 생각보다 변수 하나하나들이 너무 방대하게 여러 코드에 섞여있어서 변수 하나만 빼내려고 해도 한 번에 여러가지 기능을 수정해야하는 문제가 생겼다.

그렇게 여러가지를 수정하다가 도저히 헷갈리고 진행이 더뎌서 첫 커밋으로 다시 되돌렸다.

 

그렇게 두 번째로 시도한 방법은 DOM과 상태로직을 일단 분리해는 것이였습니다.

이벤트 핸들러에서 DOM을 참조하여 장바구니아이템을 가져오는 부분들도 존재하는 등, 이러면 안될것같은 강한 느낌의 코드들을 분리하기 시작했다.

하지만 이걸 진행하면서 결국 또 한번의 전역 변수 변경이 일어나야만 했고, 순서가 전역변수 변경이 맞다는 생각이 들었다.

 

그렇게 다시 첫 번째 커밋으로 되돌리고 아래와 같은 방법으로 다시 시도하였다.

 

작은 부분부터 시작하자

전역변수를 한번에 뽑아내려면 어렵다.

그래서 기능별로 뽑아내기를 시도했다. 어차피 프론트엔드는 사용자가 화면을 보고 조작하는것을 만들기에, 그 순서대로 진행해보기로했다.

일단 장바구니에 상품을 추가하는 기능이 있다면 그 클릭으로 발생하는 이벤트를 찾아 먼저 수정하기를 찾는 등, 이렇게 실제 동작을 한 기능 단위라고 생각하고 진행했다.

 

그럼에도 결국 전역변수를 뽑아내면 다른곳이 문제가 생겨 코드가 돌아가지 않는다.

그렇다면 어떤 방법이있을까?

바로 전역 스토어와 기존 전역변수를 같이 사용하며 진행하는것이다.

일단 기능 어떤 사용자 액션이 전역변수를 변경한다면, 해당 부분에 전역스토어의 액션함수도 같이 넣어서 변경하는 것이다.

이렇게 하면 다른 기능들도  살아있으면서 내가 추가하려는 스토어도 잘 동작하는지 알 수 있다.

 

그리고 변경을 진행하며 제일 중요시 여긴것은 바로 테스트이다.

실제 내가 생각하는 방향을 작은단위마다 코드에 적용했다면 무조건 곧바로 테스트를 진행했다.

이게 정말 중요한 이유가 리팩토링은 결국 원본코드가 잘 돌아가는것을 훼손하지 않아야한다.

그렇지 않으면 결국 진짜 동작도 안해버리는 쓰레기 코드가 되어버리기 때문이다.

 

이렇게 작은 부분을 변경하기 위해 나는 처음에 전역변수 prodList를 Store객체를 만들었다.

 

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,
  },
};

 

이 코드는 전역변수로 존재하던 productList를 Store로 만들었다.

그리고 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,
  };
};

 

사실 이 스토어는 완성본이고.. 원래는 ACTION_TYPE을 넣는것도 없었는데, 스토어를 생성하면서 액션함수에 단순 조회용 액션 함수가 필요해지면서 위 모양까지 변경되게 되었다.

이렇게 스토어를 도입하고 기능별로 하나씩 전역변수를 스토어변수로 변경하면서 동작이 잘 되는것을 확인한 후, 전역변수를 지워도 동작하지 않을까? 라는 생각이 들어  지우고 테스트를 해보니 아주 잘 되었다!

이처럼 단계적으로 진행했어야 하는 부분을 내가 한 번에 하려고 했으니.. 당연히 내 머리로 감당이 되질 않는 것이였다.

 

AI야 이젠 해줘야지..?

이렇게 데이터와 UI의 분리가 명확해지고 나니 Cursor에게 내가 좀 더 작은 단위로 명령을 내릴 수 있었고, 또한 명확한 분리를 요구할 수 있었다. Cursor또한 이제는 이해가 잘 되는지.. 원래 어떻게 질문해도 잘 못해주던 애가.. 내가 원하는 방향으로 잘 고쳐주기 시작했다!!

 

그렇게 처음엔 잘 되지 않던 부분이 이제는 불과 몇시간만에 과제를 끝내버릴 정도로 완벽히 잘 되었다.

심지어 리액트로 변환하는 과정또한 사실상 한번에 완료될 정도였다..

 

느낀점

이번 과제를 진행하면서 클린코드를 지향해야하는 이유를 많이 느꼈다. 코드를 봐도 덩치가 너무 큰 악성코드는 이해하기 너무 어렵고, 내가 어느 부분을 놓치는지 알기도 힘들다.

심지어 이를 리팩토링 할 때, 작은 단위를 천천히 변경해야하는데 변경하면서 무조건 테스트를 도입하고 절대 기존 기능이 무너지면 안된다는 것도 느꼈다.

기존 기능이 무너져버리면 다른 기능을 변경할 때, 현재 잘 되는건지 확인할 길이 없기 때문이다.

 

그리고 AI 사용에 대해 느낀점은..  AI를 적극 활용해야하지만, 잘 써야한다는 것이다. 실제로 Store로 전환 후 AI가 없었다면 나머지 작업들도 많은 시간이 걸렸을것이다. 하지만 AI에게 명확한 요구를 진행할 수 있게 되었을 때, AI는 분명히 나보다 훨씬 빠른 속도와 훨씬 질 좋은 코드를 만들어낸다.

 

요약하자면..

1. 리팩토링을 진행하면 코드 수정 후 반드시 테스트 통과를 기반으로 진행한다. (모두 끝난 후 통과가 아닌 단위별로!)
2. 내가 이해하지 못한 코드는 AI에게 넘겨주지 말자
3. AI가 제대로 고쳐주지 않는다면 내가 오히려 코드를 잘못 이해하고 질문한건 아닌지? 의심해보자