Noise-Effector 개발기 -리팩터링-

thumb_hyeok·2023년 4월 24일
0

⚡ Noise-Effector

목록 보기
3/3

🤔 개요

최근 어떤 회사의 과제테스트를 진행하고 피드백을 받았는데, 컴포넌트의 추상화에 대한 피드백을 중점적으로 받았었다. 피드백을 듣고 보니 아쉬운 점들이 꽤 많이 보였고, 개인적으로 과제 코드를 리팩터링 해보다가 사이드 프로젝트에서 리팩터링을 진행하고 포스팅을 해보면 좋을 것 같다는 생각이 들었다.

그래서 최근에 진행한 사이드 프로젝트인 Noise Effector 의 코드를 한 번 보면서 진행해볼 예정이다!


✅ 체크리스트 살펴보기

컴포넌트 추상화에 대한 여러가지 글과 컨퍼런스 영상들을 보며 그냥 적당히 분리할 게 아니라, 명확한 기준을 가지고 컴포넌트를 분리하고 추상화하는 것이 필요하다는 생각이 들었다. 아래는 내가 정리해본 좋은 컴포넌트와 코드의 기준이고, Noise Effector 의 코드는 이를 얼마나 잘 지켰는지 살펴볼 예정이다.

  • 사용처에서 편리하게 사용할 수 있는가? (인터페이스 고려)
  • 분리해서 복잡도가 낮아진 컴포넌트가 맞는가?
  • 재사용 가능한 컴포넌트인가?
  • 변경에 유연하게 대처할 수 있는 컴포넌트인가?
  • 한 가지 역할만 하는가?

가장 먼저 보고 싶었던 건, Carousel 컴포넌트이다. 분명히 도메인과 철저히 분리되어서 만들어져야하는 컴포넌트라는 생각이 드는데 내가 그렇지 않게 만들었던 것 같기 때문이다.

export const Carousel = ({ thumbnailSource, carouselContainerRotateY }: CarouselProps) => {
  const noiseStrengths = [1, 2, 3, 4, 5, 6];

  return (
    <S.Container rotateY={carouselContainerRotateY}>
      {noiseStrengths.map((el, index) => (
        <CarouselItem
          key={el}
          rotateY={(360 / noiseStrengths.length) * index}
          thumbnailSource={thumbnailSource}
          noiseStrength={noiseStrengths[index] - 1}
        />
      ))}
    </S.Container>
  );
};

역시나다. 애초에 이름부터 잘못됐다. 이 컴포넌트의 이름은 3DCarousel 이 되는게 맞을 것이다. 그리고 지금 사용하는 용도와 아주 끈끈하게 결합되어있어 절대 다른 용도로 재사용은 불가능해보인다.

그리고 아마 코드를 처음보는 사람은 CarouselProps 에 있는 것들이 왜 필요한지 어떻게 사용되는건지 짐작을 못할 거라고 생각한다. 설계단계부터 다시 해보자.

export const ThreeDimensionCarousel = ({ children }: ThreeDimensionCarouselProps) => {
  const { rotation, containerRef } = useCarouselSwipe(children.length);
  const [itemWidth, setItemWidth] = useState<number>(0);
  const itemRef = useRef<HTMLDivElement | null>(null);

  useEffect(() => {
    if (itemRef.current) {
      setItemWidth(itemRef.current.offsetWidth);
    }

    window.addEventListener("resize", () => {
      if (itemRef.current) {
        setItemWidth(itemRef.current.offsetWidth);
      }
    });
  }, []);

  return (
    <S.Container ref={containerRef}>
      <S.Carousel rotateY={rotation}>
        {children.map((item, i) => (
          <S.Item
            key={i}
            itemsGap={itemWidth / 1.1}
            rotateY={(360 / children.length) * i}
            ref={i === 0 ? itemRef : null}
          >
            {item}
          </S.Item>
        ))}
      </S.Carousel>
      {itemWidth === 0 && <Spinner />}
    </S.Container>
  );
};

interface ThreeDimensionCarouselProps {
  children: JSX.Element[];
}

위와 같이 코드를 고쳐봤다.

이제 확실하게 3D 캐러셀임을 알 수 있는 이름을 가지고 있다. props를 확인해보면, children 만을 받아서 map 을 통해 요소들을 그려주고 있다. 외부에서 반응형으로 item의 사이즈를 조절하면 거기에 맞춰 translateZ 를 조절해야하기 때문에 렌더링 이후 width 정보를 가져와야한다. 그래서 캐러셀이 그려지는 시간이 조금 늦춰지긴하지만 문제 없는 정보인 것 같다.

추가로 모바일과 데스크탑 환경을 모두 고려해 MouseEvent , TouchEvent 를 활용하여 스와이프를 통해 캐러셀을 회전할 수 있도록 컴포넌트 내부에서 커스텀훅을 불러오기 때문에, 외부에서 rotateY 같은 값을 주입해 직접 회전시키는 로직을 만들 필요도 없어졌다.

물론, 외부에서 버튼방식을 사용하거나 캐러셀 회전방식을 세세하게 수정하는 건 힘들게 되었지만 선택의 자유도가 높은 것만이 정답은 아니라고 생각한다. 지금은 한가지 방식을 강요하는 것이, 선택을 강요하는 것보다 이 컴포넌트를 사용하기 편하다고 판단했다.

어디 원래 코드와 바꿔도 문제 없이 동작하는지 살펴보자.

export const ThumbnailOptions = () => {
  const [searchParams] = useSearchParams();
  const [isClick, setIsClick] = useState(false);
  const rgba = useContext(RgbaContext);
  const navigate = useNavigate();
  const thumbnailSource = String(searchParams.get("thumbnail-source"));
  const noiseStrengthArray = Array.from({ length: MAX_NOISE_LEVEL }, (_, i) => i);

  const handleClickButton = (noiseStrength: number) => {
    if (!isClick) return;

    navigate(`${ROUTE_PATH.THUMBNAIL_RESULT}?noise-strength=${noiseStrength}`, {
      state: { thumbnailSource },
    });
  };

  useEffect(() => {
    if (!rgba) {
      alert("이미지 정보가 사라졌습니다. 다시 시도해주세요.");
      navigate(ROUTE_PATH.HOME);
    }
  }, [rgba]);

  return (
    <S.Container>
      <ThreeDimensionCarousel>
        {noiseStrengthArray.map((noiseStrength) => {
          return (
            <CarouselItem
              thumbnailSource={thumbnailSource}
              noiseStrength={noiseStrength}
              onClick={() => handleClickButton(noiseStrength)}
              onMouseDown={() => setIsClick(true)}
              onMouseMove={() => setIsClick(false)}
            />
          );
        })}
      </ThreeDimensionCarousel>
    </S.Container>
  );
};

예상치 못한 문제가 발생하긴 했지만, 지금 당장 하고 있는 것과는 크게 연관이 없는 것 같아서 무시하기로 했다. 스와이프를 마우스 이벤트로 관리하다 보니, 스와이프 도중 클릭 이벤트 핸들러가 동작해버리는 문제가 있었다.

일단 onMouseDown , onMouseMove 를 통해 스와이프 도중인지, 진짜 클릭인지 구분하도록 변경해주었다.

이외에는 동작의 문제는 없는 것 같았다.


✅ 체크리스트 검토하기

이제 체크리스트를 검토하며 한 번 제대로 리팩터링이 되었는지 살펴보자.

  • 사용처에서 편리하게 사용할 수 있는가? (인터페이스 고려)
  • 분리해서 복잡도가 낮아진 컴포넌트가 맞는가?
  • 재사용 가능한 컴포넌트인가?
  • 변경에 유연하게 대처할 수 있는 컴포넌트인가?
  • 한 가지 역할만 하는가?

사용처에서 편리하게 사용할 수 있는가?

조금 아쉬운 부분이 있다. 원인은 오직 “스와이프” 때문인데, 스와이프 도중 클릭 이벤트 핸들러가 동작해버리는 문제 때문에 children 으로 전달하는 캐러셀 아이템에 onMouseDown , onMouseMove 에 이벤트 핸들러로 isClick 상태를 변경해주면서 onClick 이벤트 핸들러를 동작시켜야하기 때문이다.

스와이프를 유지하면서 이를 해결할 수 있는 방법은 당장은 모르겠다..

그래도 이 점만 제외하면 편리하게 사용할 수 있는 컴포넌트가 맞다고 생각한다.. children 만 전달해주면 요소들을 3D 캐러셀로 바로 그려줄 수 있기 때문이다!


분리해서 복잡도가 낮아진 컴포넌트가 맞는가?

사실 새로 따로 분리한 부분도 아닐 뿐더러 분리를 하지 않아야 하는 컴포넌트는 아니라고 생각한다.
짧게 다음으로 넘어가자.


재사용 가능한 컴포넌트인가?

이제 정말 재사용이 가능해졌다고 볼 수 있다. 재사용성이 떨어지는 부분을 내 눈으로는 찾아볼 수 없다.


변경에 유연하게 대처할 수 있는 컴포넌트인가?

크게 문제가 없다고 생각한다. 아래에 목차 인디게이터를 부착하거나, 좌우 버튼을 다는 정도의 변경을 예측하고 만든 컴포넌트인데, useCarouselSwipe 만 상위 컴포넌트로 옮기면 모든 상위 컴포넌트에서 캐러셀의 회전을 제어할 수 있으므로 상위컴포넌트에서 인디게이터나 버튼을 전달하거나 따로 사용할 수 있다.


한 가지 역할만 하는가?

그렇다.


📖 후기

생각보다 컴포넌트를 리팩터링해보면서 고쳐야할 부분이 많다고 느꼈다. 캐러셀이 제일 심각한 것 같아서 캐러셀을 건드리긴 했지만, 여전히 문제가 있는 부분들은 많다는 생각이 든다. 시간이 나면 천천히 고쳐보면 좋을 것 같다.

ps. 혹시 스와이프에 대해서 좋은 방법을 알고 있다면 조언 부탁드립니다.

profile
우아한테크코스 4기 웹 프론트엔드

0개의 댓글