Refactor (MBTI 테스트 리팩토링)

Jeonghun·2023년 8월 27일
refactor
0

React

목록 보기
11/21

프로젝트의 문제점 발견

이전에 진행했던 '맛이슈; 팀 프로젝트에서 구현했던 MBTI 페이지에서 작은 문제가 발생했다. 바로 테스트 페이지에서 뒤로가기 버튼을 광클했을 때 아래 페이지와 같이 렌더링 이슈가 발생한다는 것이다.

문제의 에러 페이지

에러의 발생 원인

위와 같은 에러가 발생하는 원인을 찾아보기 위해 기존에 작성되어 있던 이전 문제로 돌아가기 버튼 로직인 'goBack' 함수의 로직을 살펴보았다.

📌 TestPageClient.tsx

// 기존 함수 로직

const goBack = () => {
    if (count > 1) {
      setAnswerButtonAnimation(true);
      setTimeout(() => {
        setCount((prevCount) => prevCount - 1);
        setProgressStep((prevStep) => prevStep - 1);
        // 이전 문제에서 클릭한 버튼 번호를 null로 초기화
        setLastButtonNumbers((lastButtonNumbers) => {
          const updatedLastButtonNumbers = lastButtonNumbers.map((num, index) =>
            index === count - 2 ? null : num
          );
          const lastButtonNumber = updatedLastButtonNumbers[count - 2];
          if (lastButtonNumber !== null) {
            // lastButtonNumber에 따라 MBTI 성향을 업데이트
            if (lastButtonNumber === 1) {
              if (count <= 3) {
                setEI((EI) => EI - 1);
              } else if (count >= 4 && count <= 6) {
                setSN((SN) => SN - 1);
              } else if (count >= 7 && count <= 9) {
                setTF((TF) => TF - 1);
              } else if (count >= 10 && count <= 12) {
                setJP((JP) => JP - 1);
              }
            } else {
              if (count <= 3) {
                setEI((EI) => EI + 1);
              } else if (count >= 4 && count <= 6) {
                setSN((SN) => SN + 1);
              } else if (count >= 7 && count <= 9) {
                setTF((TF) => TF + 1);
              } else if (count >= 10 && count <= 12) {
                setJP((JP) => JP + 1);
              }
            }
          }
          return updatedLastButtonNumbers;
        });
        setAnswerButtonAnimation(false);
      }, 300);
    }
  };

이전 로직의 작동 방식을 살펴보면 다음과 같은 과정을 거친다.

  1. 먼저, if문을 통해 현재 문제의 수가 1번을 넘어가는지 확인
  2. 페이지 변환 애니메이션 실행
  3. 상태 값에 저장된 현재의 문제 수와 진행도 수 상태에서 -1 처리
  4. 이전 문제에서 클릭한 정답 버튼의 번호를 null로 초기화 작업
  5. 이전에 클릭한 버튼의 번호에 따른 MBTI 성향 업데이트 작업

문제가 발생하는 원인은 위 작업들을 setTimeout 안에서 비동기적으로 처리한다는 것이다. 이로인해 버튼을 연속적으로 클릭했을 때, 이전 작업이 제대로 처리되기도 전에 또 다시 로직이 실행되면서 해당 렌더링 이슈가 발생하게된다.

에러를 해결하는 방식

문제를 해결하기 위해 어떻게 해야할까 고민하던 중 두 가지의 방식이 떠올랐다.

  1. 뒤로 가기 버튼에 Debouce 적용
    디바운스는 특정 함수를 반복적으로 호출하는 것을 제한하면서, 마지막 호출 이후 일정 시간이 경과할 때 까지 함수를 실행하지 않도록 하는 기법이다. 이를 이용하여 버튼의 광클을 방지할 수 있다.
  1. 뒤로 가기 버튼의 비활성화
    goBack 함수의 로직이 실행된 후, 일정 시간 버튼을 다시 클릭하지 못하도록 비활성화 함으로써 버튼의 광클을 방지한다.

나는 문제 해결을 위해 setTimeout을 이용해 일정 시간 버튼을 클릭할 수 없도록하여 버튼의 연속적인 클릭을 방지하고자 했다.

에러 해결 과정

- 버튼의 활성화 상태 정의

버튼의 활성화 상태를 관리하기 위해 useState를 이용해 다음과 같이 상태를 선언해주었다.

const [isButtonDisabled, setIsButtonDisabled] = useState<boolean>(false);

- 기존 로직의 수정

기존 goBack 함수 로직에서 초기 if문을 제거하고 onClick 함수를 따로 만들어 이를 관리하였다. 또한, 버튼의 비활성화 시간을 관리하는 함수를 작성하여 그 안에서 goBack 함수의 실행 후 일정 시간 버튼의 클릭을 제어하도록 했다.

 // 이전 버튼 상태 업데이트 로직 (기존의 goBack 로직)
  const updateBackButtonState = () => {
    setCount((prevCount) => prevCount - 1);
    setProgressStep((prevStep) => prevStep - 1);
    // 이전 문제에서 클릭한 버튼 번호를 null로 초기화
    setLastButtonNumbers((lastButtonNumbers) => {
      const updatedLastButtonNumbers = lastButtonNumbers.map((num, index) =>
        index === count - 2 ? null : num
      );
      const lastButtonNumber = updatedLastButtonNumbers[count - 2];
      if (lastButtonNumber !== null) {
        // lastButtonNumber에 따라 MBTI 성향을 업데이트
        if (lastButtonNumber === 1) {
          if (count <= 3) {
            setEI((EI) => EI - 1);
          } else if (count >= 4 && count <= 6) {
            setSN((SN) => SN - 1);
          } else if (count >= 7 && count <= 9) {
            setTF((TF) => TF - 1);
          } else if (count >= 10 && count <= 12) {
            setJP((JP) => JP - 1);
          }
        } else {
          if (count <= 3) {
            setEI((EI) => EI + 1);
          } else if (count >= 4 && count <= 6) {
            setSN((SN) => SN + 1);
          } else if (count >= 7 && count <= 9) {
            setTF((TF) => TF + 1);
          } else if (count >= 10 && count <= 12) {
            setJP((JP) => JP + 1);
          }
        }
      }
      return updatedLastButtonNumbers;
    });
    setButtonAnimation(false);
  };


// 버튼 클릭 처리 함수
  const handleButtonClick = async (updateState: Function) => {
    if (!isButtonDisabled) {
      setIsButtonDisabled(true); // 버튼 비활성화 상태로 변경

      setButtonAnimation(true);

      // 기다릴 시간을 정의하는 Promise
      const wait = (ms: number) =>
        new Promise((resolve) => setTimeout(resolve, ms));

      // 로직 실행 전에 300ms 기다리기 (화면 전환 애니메이션 실행)
      await wait(300);

      // 상태 업데이트 로직
      updateState();

      // 버튼 비활성화를 700ms 동안 유지 (광클 방지)
      await wait(700);

      setIsButtonDisabled(false); // 버튼 활성화 상태로 변경
    }
  };

// 이전 버튼 클릭 이벤트 함수
  const clickBackButton = async () => {
    if (count > 1) {
      handleButtonClick(updateBackButtonState);
    }
  };

위 처럼 buttonOnClick 함수에서 버튼을 클릭한 후, 700ms (0.7초)간 버튼의 비활성화 상태를 true로 전환하여 사용자가 버튼을 연속적으로 클릭하는 것을 방지하였다. 이를 통해 해결하고자 했던 렌더링 이슈는 해결되었다.

여전히 지저분한 나의 코드..

에러는 정상적으로 해결되었지만, 나의 코드를 살펴보니 너무나 지저분하다. 특히 updateBackButtonClick 함수 내의 MBTI 계산 로직에 연속된 if와 else가 나의 신경을 건드렸다. 이를 어떻게 더 깔끔하게 작성할 수 있을지 고민한 끝에 다음 방법을 선택하기로 했다.

  1. MBTI 계산 로직 분리
    MBTI의 값을 업데이트 하는 로직을 별도의 함수로 분리하여 'updateMBTI' 라는 함수를 작성한다.
  1. getLastButtonNumber 함수의 분리 작성
    지난 버튼의 번호를 가져오는 로직을 별도의 함수로 작성한다.

위 과정을 통해 로직의 관심사를 분리하여 리팩토링을 해보았다.

- 리팩토링 1단계

// 이전에 클릭한 버튼의 번호를 가져오는 로직 분리
const getLastButtonNumber = (lastButtonNumbers: number[], count: number): number | null => {
  const updatedLastButtonNumbers = lastButtonNumbers.map((num, index) => 
    index === count - 2 ? null : num
  );
  return updatedLastButtonNumbers[count - 2];
}

// MBTI 업데이트 로직 분리
const updateMBTI = (lastButtonNumber: number, count: number) => {
  // MBTI 성향을 해당 업데이트 함수에 매핑
  const updateFunctionMapping = {
    1: setEI,
    2: setSN,
    3: setTF,
    4: setJP
  };

  // 문제 범위를 MBTI 성향 유형에 매핑
  const questionRangeMapping = [
    { start: 1, end: 3, type: 1 },
    { start: 4, end: 6, type: 2 },
    { start: 7, end: 9, type: 3 },
    { start: 10, end: 12, type: 4 }
  ];

  // 현재 문제 번호를 기반으로 업데이트 할 MBTI 성향 결정
  const questionType = questionRangeMapping.find(range => 
    count >= range.start && count <= range.end
  )?.type;

  // 마지막 버튼 번호에 따라 증가 값 결정
  const incrementValue = lastButtonNumber === 1 ? -1 : 1;

  // 결정된 MBTI 성향과 증가 값으로 해당 성향의 상태 업데이트
  const updateFunction = updateFunctionMapping[questionType!];
  updateFunction((currentValue: number) => currentValue + incrementValue);
}

// 이전 버튼 상태 업데이트 로직
const updateBackButtonState = () => {
  setCount((prevCount) => prevCount - 1);
  setProgressStep((prevStep) => prevStep - 1);

  const lastButtonNumber = getLastButtonNumber(lastButtonNumbers, count);
  if (lastButtonNumber !== null) {
    updateMBTI(lastButtonNumber, count);
  }

  setLastButtonNumbers((lastButtonNumbers) => {
    return lastButtonNumbers.map((num, index) => 
      index === count - 2 ? null : num
    );
  });

  setButtonAnimation(false);
};

위 코드와 같이 첫 번째 리팩토링을 진행했다. 우선 이전 버튼의 번호를 가져오는 로직을 기존의 로직에서 분리하여 따로 작성하고, MBTI의 값을 업데이트 하는 함수 또한 분리하여 새로 작성했다. 새로 작성된 로직을 살펴보면,

  1. MBTI 성향을 해당 업데이트 함수와 매핑하는 updateFunctionMapping을 설정한다.
  2. 몇번 문제의 범위가 어떤 MBTI 성향 유형에 해당하는지 정의하는 questionRangeMapping을 설정한다.
  3. 현재 count를 기반으로 업데이트 할 MBTI 성향 (questionType)을 결정한다.
  4. lastButtonNumber에 따라 증가 값이 얼마인지 계산한다.
  5. 계산된 questionType와 증가 값을 사용하여 해당 MBTI 성향의 상태를 업데이트한다.

이처럼 리팩토링을 하고보니, '매핑을 위한 객체가 굳이 필요한가?' 에 대한 의문이 들었다. 코드를 깔끔하게 작성하려고 했지만, 내가 아닌 다른 사람이 보았을 때 이 로직을 보고 한 번에 코드를 이해할 수 있을까? 라는 질문을 스스로에게 했을 때 돌아오는 답변은 "아니다." 였다. 그로인해 두 번째 리팩토링을 진행했다.

- 리팩토링 2단계

위에서 스스로에게 제안한 문제점으로 인해 시작하게된 두 번째 리팩토링 코드는 다음과 같다.

const updateMBTI = (lastButtonNumber: number, count: number) => {
  const incrementValue = lastButtonNumber === 1 ? -1 : 1;

  if (count <= 3) {
    setEI(prev => prev + incrementValue);
  } else if (count <= 6) {
    setSN(prev => prev + incrementValue);
  } else if (count <= 9) {
    setTF(prev => prev + incrementValue);
  } else {
    setJP(prev => prev + incrementValue);
  }
}

첫 번째 로직보다 훨씬 깔끔해졌다. 지난 버튼의 번호에 따른 로직 계산을 미리 incrementValue 라는 상수로 선언하고, if문을 통해 현재 문제 번호에 따른 성향 값을 재계산한다. 이렇게 코드를 수정하고보니, 문득 최근 유데미의 JS 클린코드 강의에서 보았던 'else를 지양하라'는 내용이 생각났다. 분명 내 코드는 이미 이전보다 충분히 간결하고 깔끔해보인다. 하지만 배운 내용을 직접 내 코드에 적용시켜보고 싶었고, 이를 실천하기 위해 세 번째 리팩토링을 진행했다.

- 리팩토링 3단계

위에서 제시한 else와 else if를 지양하는 방법을 고민하던 중, 중첩 if문에 대한 처리를 switch문으로 재구성하면 어떨까 생각해보았다. 하지만 각 성향별 set 함수를 배열 안에 담아서 관리하고, 현재의 문제 번호인 count에 따라 배열의 index를 불러와 해당하는 set 함수를 실행하는 방식을 사용하여, if문과 switch문을 모두 사용하지 않고 로직을 작성할 수 있었다.

const updateMBTI = (lastButtonNumber: number, count: number) => {
  const incrementValue = lastButtonNumber === 1 ? -1 : 1;

  // MBTI 성향 업데이트 함수들을 배열로 정리
  const updateFunctions = [setEI, setSN, setTF, setJP];

  // count 값에 따른 배열 인덱스를 가져옴
  const index = Math.floor((count - 1) / 3);

  // 해당 성향 업데이트 함수 실행
  updateFunctions[index](prev => prev + incrementValue);
}

위 처럼 MBTI를 업데이트 하는 로직은 최종적으로 완성되었다. 기존의 로직과 비교했을 때 훨씬 간결하고, 중첩된 조건문을 제거하여 불필요한 연산을 줄여 효율적인 계산이 가능해졌다. 이제 마지막으로, 이전 버튼의 번호를 불러오는 getLastButtonNumber의 로직을 수정함으로써 리팩토링의 마무리 단계에 접어들었다.

- 리팩토링 4단계 (최종)

이전에 작성한 로직을 살펴보면 함수는 두 가지의 목적을 가진다.

  1. 현재 문제의 번호 값인 'count'에 해당하는 이전 문제에서 클릭한 버튼 번호를 가져오는 것.
  1. 가져온 번호의 값을 null로 초기화 하는 것.
// 이전 작성된 로직
const getLastButtonNumber = (lastButtonNumbers: number[], count: number): number | null => {
  const updatedLastButtonNumbers = lastButtonNumbers.map((num, index) => 
    index === count - 2 ? null : num
  );
  return updatedLastButtonNumbers[count - 2];
}

이렇게 두 가지 기능을 가지게 되는데, 이들을 서로 분리된 함수로 작성하여 각 함수는 하나의 책임만 가지도록하는 '단일 책임의 원칙'을 지키도록 했다.

// 이전 버튼의 번호를 가져오는 함수
const getLastButtonNumber = (lastButtonNumbers: number[], count: number): number | null => {
  return lastButtonNumbers[count - 2];
}

// 가져온 버튼의 값을 null로 초기화 하는 함수
const setLastButtonNumberToNull = (lastButtonNumbers: (number | null)[], count: number): (number | null)[] => {
  const updated = [...lastButtonNumbers]; // 배열 복사
  updated[count - 2] = null;
  return updated;
}

이런식으로 기존 함수의 기능을 분리하여 기존 로직의 map 함수를 제거함으로써 연산을 최소화했다.

최종 완성된 로직

// [ 이전 문제에서 클릭한 버튼의 번호를 가져오는 함수 ]
const getLastButtonNumber = (lastButtonNumbers: (number | null)[], count: number): number | null => {
  return lastButtonNumbers[count - 2];
}

// [ 가져온 버튼의 번호를 null로 초기화 하는 함수 ]
const setLastButtonNumberToNull = (lastButtonNumbers: (number | null)[], count: number): (number | null)[] => {
  const updated = [...lastButtonNumbers]; // 배열 복사
  updated[count - 2] = null;
  return updated;
}

// [ MBTI 업데이트 로직 ]
const updateMBTI = (lastButtonNumber: number, count: number) => {
  const incrementValue = lastButtonNumber === 1 ? -1 : 1;

  // MBTI 성향 업데이트 함수들을 배열로 정리
  const updateFunctions = [setEI, setSN, setTF, setJP];

  // count 값에 따른 배열 인덱스를 가져옴
  const index = Math.floor((count - 1) / 3);

  // 해당 성향 업데이트 함수 실행
  updateFunctions[index](prev => prev + incrementValue);
}
  
 // [ 뒤로가기 버튼 클릭시 실행 로직 (기존의 goBack) ]
const updateBackButtonState = () => {
  // 현재 count와 progress step을 감소
  setCount(prevCount => prevCount - 1);
  setProgressStep(prevStep => prevStep - 1);

  // 이전 문제에서 클릭한 버튼의 번호를 가져옴
  const lastButtonNumber = getLastButtonNumber(lastButtonNumbers, count);

  if (lastButtonNumber !== null) {
    // MBTI 업데이트 로직을 실행
    updateMBTI(lastButtonNumber, count);
  }

  // 가져온 버튼의 번호를 null로 초기화
  const updatedLastButtonNumbers = setLastButtonNumberToNull(lastButtonNumbers, count);
  setLastButtonNumbers(updatedLastButtonNumbers);

  // 버튼 애니메이션 상태 업데이트
  setButtonAnimation(false);
};

}

// [ 버튼 클릭 처리 함수 ]
  const handleButtonClick = async (updateState: Function) => {
    if (!isButtonDisabled) {
      setIsButtonDisabled(true); // 버튼 비활성화 상태로 변경

      setButtonAnimation(true);

      // 기다릴 시간을 정의하는 Promise
      const wait = (ms: number) =>
        new Promise((resolve) => setTimeout(resolve, ms));

      // 로직 실행 전에 300ms 기다리기 (애니메이션 실행)
      await wait(300);

      // 상태 업데이트 로직
      updateState();

      // 버튼 비활성화를 700ms 동안 유지 (광클 방지)
      await wait(700);

      setIsButtonDisabled(false); // 버튼 활성화 상태로 변경
    }
  };

  // [ 이전 버튼 클릭 onClick 이벤트 함수 ] 
  const clickBackButton = async () => {
    // 2번 문제부터 로직 활성화
    if (count > 1) {
      handleButtonClick(updateBackButtonState);
    }
  };

이로써 리팩토링된 최종 코드가 완성되었다. 기존의 로직보다 길이가 길어진 것 같은 느낌을 받을수도 있지만 (?) 관심사를 분리하고, 함수를 잘개 쪼갬으로써 확실히 가독성 측면에서 우수해졌고, 재사용성과 유지 보수의 효율성 까지 높일 수 있었다. 또한, map 함수를 사용하여 배열을 순회하던 기존의 로직을 필요한 index만 참조하는 방식으로 수정함으로써 불필요한 연산을 최소화하여 최적화된 로직이 완성됐다고 생각한다.

포스팅을 마치며

작은 문제점을 발견하면서 시작된 리팩토링. 코드의 리팩토링을 진행하면서, 기존 코드의 불편함을 파악하고 이를 개선함으로 어떻게 하면 더 깔끔하고 효율적인 코드를 작성할 수 있을까 많이 고민해보는 시간을 가질 수 있었다. 이를 통해 왜 개발자들이 기능 구현에서 그치는 것이 아니라 유지 보수와 리팩토링에 힘쓰는지 알 수 있었다. 물론 내가 작성한 현재의 코드가 정답은 아닐것이며, 분명히 더 좋은 방법의 코드가 존재할 것이다. 하지만, 리팩토링 과정을 통해 문제를 분석하는 시선과, 정답을 통해 한 걸음씩 나아갈 수 있는 능력을 키울 수 있었던 것 같아 너무 뿌듯한 시간이었다!

profile
Jeonghun
안녕하세요, 프론트엔드 개발자 임정훈입니다.
이전 포스트

React (useImperativeHandle & forwardRef)

다음 포스트

Octokit을 이용한 git api 호출하기

0개의 댓글