🔥 리팩토링 직접 해보기

<aside> 📌 이 코드는 유데미 부트캠프 최종 테스트였던 “계산기 만들기” 때 작성했던 것입니다.

그 중 “입력된 값의 루트를 계산해서 화면에 보여주는” 함수를 리팩토링 해봤습니다.

화면에 보여줄 숫자 길이의 최대값이 14입니다.

</aside>

// 기존 코드
  const getRoot = () => {
    const result = String(Math.sqrt(numberOnDisplay));
    const dotIndex = result.indexOf(".");
    if (dotIndex > 0) {
      const temp = 14 - dotIndex;
      const rootFormatted = Number(result).toLocaleString("ko-KR", {
        maximumFractionDigits: temp,
      });
      setNumberOnDisplay(rootFormatted);
    } else {
      setNumberOnDisplay(result);
    }
  };

• 루트버튼을 클릭하면 getRoot 함수가 실행됐음

⛔️ 기존 코드에서 아쉬운 점

• 의미를 알 수 없는 변수명
  • result
  • temp
• 기능이 여러개임
  • 루트값 구하기
  • 소수점 위치 구하기
  • 루트값의 최대 소수 자릿수를 제한하여 상태값 변경
• temp를 구할 때 썼던 14를 상수로 따로 저장해 놓고 갖다 썼으면 좋았을 것 같음

// 메서드 추출을 통해 수정해본 코드

const DISPLAY_MAXIMUM_LENGTH = 14;

const getRoot = (number) => {
  return Math.sqrt(number);
};

const getDotIndex = (number) => {
  return String(number).indexOf(".");
};

const displayRoot = (rootNumber, dotIndex) => {
  if (dotIndex > 0) {
    const limitedLength = DISPLAY_MAXIMUM_LENGTH - dotIndex;
    const rootFormatted = rootNumber.toLocaleString("ko-KR", {
      maximumFractionDigits: limitedLength,
    });
    setNumberOnDisplay(rootFormatted);
  } else if (dotIndex === -1) {
    setNumberOnDisplay(String(rootNumber));
  }
};

const clickRootBtn = () => {
	//numberOnDisplay는 전역변수인데 그냥 이렇게 갖다 쓰는게 맞나? 이것도 ClickRootBtn의 매개변수로 받아야 하나?
  const rootResult = getRoot(numberOnDisplay);
  const rootDotIndex = getDotIndex(rootResult);

  displayRoot(rootResult, rootDotIndex);
};

🫠 리팩토링하면서 느낀점

• 짧은 코드라 쉽게 리팩토링 할 수 있을 줄 알았는데 생각보다 고민하는데 시간이 꽤 걸렸다.

• 전체적인 코드는 늘어났지만 함수 기능 분리와 적절한 함수명을 지음으로써 가독성이 확실히 증가한 것 같다.

• clickRootBtn함수에서 주석으로도 써놨는데,

  numberOnDisplay는 전역변수인데 전역변수 가져다 쓸 때 항상 고민되는게

  // 전역 변수를 바로 갖다 씀
  const clickRootBtn = () => {
    const rootResult = getRoot(numberOnDisplay);
    const rootDotIndex = getDotIndex(rootResult);
  
    displayRoot(rootResult, rootDotIndex);
  };
  
  // 여기 코드가 깔끔함
  <Button onClick={clickRootBtn}>Root</Button>
  

  // 전역변수를 굳이 매개변수로 넘김
  const clickRootBtn = (displayNumber) => {
    const rootResult = getRoot(displayNumber);
    const rootDotIndex = getDotIndex(rootResult);
  
    displayRoot(rootResult, rootDotIndex);
  };
  
  // 여기 코드가 좀 복잡해짐
  <Button onClick={() => clickRootBtn(numberOnDisplay)}>Root</Button>
  

  둘 중 어떻게 쓰는게 좋은걸까 🧐