기존코드의 문제점 발견하고 리팩토링하기

👿 이슈사항

 

위와 같이 신체부위 일러스트를 통해 신체부위를 클릭하면 해당 신체부위에 대해 기록한 내용을 볼수 있는 페이지로 이동하도록 네비게이터 역할을 하는 SVG가 필요했다.

 

일러스트를 통해 신체부위를 선택할 수 있는 컴포넌트를 BodyNavigator라고 칭하기로 하고, 이 BodyNavigator는 아래와 같이 구현되어야 한다.

 

-  이 BodyNavigator 컴포넌트는 기록하기 페이지, 기록확인-대시보드 페이지, 기록확인-차트 페이지 총 3가지 페이지에서 사용된다.

-  기록하기 페이지에서는 BodyNavigator 뒤에 네모 배경이 들어가야한다.

-  svg를 클릭하면 해당 신체부위의 한글이름을 매칭할 수 있어야 한다.

-  양쪽이 대칭인 신체부위(ex 윗팔, 골반, 허벅지 등)는 한쪽만 선택해도 왼쪽, 오른쪽 모두 선택되도록 한다.

-  하단의 앞, 뒤 버튼으로 신체 앞뒤 포지션을 바꿀 수 있다.

-  몸 전체 일러스트에서 머리를 클릭하면 얼굴의 자세한 부위를 클릭할 수 있도록 얼굴 부위 컴포넌트가 나타난다.

-  얼굴 부위 컴포넌트에서 몸 전체 컴포넌트로 돌아갈 수 있도록 하단 버튼을 앞,뒤 버튼 대신 몸 버튼으로 변경해야 한다.

-  마우스오버 했을때는 보라색, 클릭했을때는 민트색으로 신체부위 색이 변경되어야한다.

-  기록확인-대시보드 페이지에서의 신체의 포지션(앞,뒤,얼굴)이 그대로 기록확인-차트 페이지에서 보여져야 한다.

 

위 조건을 참고해서 다른 팀원 이 작성한 BodyNavigator.tsx 코드는 다음과 같다. (깃헙에서 실제코드 보기)

 

 

 

 

위의 BodyNavigator.tsx 파일에서 내가 느꼈던 문제점은

 

1) svg들이 전부 한 파일에 나열되어 있어서 코드가 길고 보기 힘들다.

⇒ 반복문을 사용하면 코드를 줄이고 좀 더 보기 편하게 만들 수 있을 것 같다.

 

2) 전역state를 props drilling으로 관리하게 되어 있어 기록하기 페이지에서만 사용하기에는 큰 불편이 없지만

기록확인-대시보드 페이지, 기록확인-차트 페이지에서도 사용할 것을 고려하면 state를 넘겨주기 매우 번거롭다는 것이다.

즉, 이 BodyNavigator컴포넌트의 재사용성이 고려되지 않았다.

⇒ 내려줄 필요가 없어보이는 props는 없애고 recoil 사용을 고려하는 등 props drilling의 불편함을 최대한 수정해보자.

 

 

 

 

 

💡 해결과정

 

문제점 1) svg들이 전부 나열되어 있어서 코드가 길고 보기 힘들다.

 

기존코드는 모든 svg들이 BodyNavigator.tsx에 작성되어있었다.

 

작성된 코드를 보면 신체의 앞, 뒤, 얼굴 포지션에 대해 해당 포지션에 들어가는 신체부위 svg의 path태그가

HoverPath라는 컴포넌트로 구성되어있고, 이 HoverPath는 900줄 분량이 비슷한 형태로 계속 반복된다.

 

코드 형태를 보니 각 신체 포지션(앞, 뒤, 얼굴)별로 HoverPath는 map을 사용하면 더 효율적으로 코드를 작성할 수 있을 것 같았다.

 

우선, 각 신체부위의 path의 경로(d)를 변수로 관리하기 위해 별도의 파일에 작성하였다.

// PartAreaBack.ts 파일

export const HEAD = "M203.763 8.37695C186.164 8.37695 169.412 18.0108 168.233 41.8684C167.152 63.6764 186.593 93.0688 204.193 93.0688C221.792 93.0688 240.558 63.701 241.074 41.8684C241.516 22.662 221.374 8.37695 203.763 8.37695V8.37695Z";

export const NECK = "M236.701 144.871C235.424 140.465 226.778 130.549 224.346 127.825C221.915 125.1 225.218 115.884 216.572 115.884H191.825C183.179 115.884 186.483 125.1 184.051 127.825C181.619 130.549 172.973 140.465 171.696 144.871C170.419 149.277 176.044 149.338 176.044 149.338H232.354C232.354 149.338 237.979 149.277 236.701 144.871Z";

export const SHOULDER_LEFT = "M152.672 148.172C152.672 148.172 130.271 148.172 123.086 157.56C115.89 166.949 94.9989 183.885 113.925 183.651C132.838 183.418 131.094 184.805 139.924 174.84C139.924 174.84 159.685 156.836 160.201 154.75C160.717 152.664 159.353 148.638 152.685 148.172H152.672Z"

export const UPPERARM_LEFT = "M99.0273 250.094C97.7254 249.714 96.8657 248.486 96.964 247.136C97.4552 240.632 99.5431 227.685 102.073 217.744C104.971 206.38 100.219 191.42 109.147 188.757C118.076 186.094 132.469 186.548 131.548 200.587C130.627 214.627 129.694 224.592 125.862 233.747C122.03 242.903 117.155 256.819 108.914 253.579C104.21 251.726 100.943 250.671 99.015 250.094H99.0273Z";

...

export const FOOT_LEFT = "M127.36 741.309C124.044 741.051 120.999 743.175 120.102 746.378C118.727 751.262 117.793 757.828 122.03 759.362C128.662 761.767 141.251 763.768 144.21 755.422C145.475 751.863 145.5 749.016 144.996 746.807C144.247 743.506 141.14 741.297 137.75 741.542C135.687 741.69 132.445 741.702 127.348 741.309H127.36Z";

export const FOOT_RIGHT = "M281.565 741.309C284.881 741.051 287.927 743.175 288.823 746.378C290.199 751.262 291.132 757.828 286.895 759.362C280.263 761.767 267.675 763.768 264.715 755.422C263.45 751.863 263.426 749.016 263.929 746.807C264.678 743.506 267.786 741.297 271.175 741.542C273.238 741.69 276.481 741.702 281.577 741.309H281.565Z";

 

 

이렇게 각 포지션(앞, 뒤, 얼굴) 별로 신체부위 path의 경로를 각각의 파일에 변수로 선언하였다.

 

front, back, face로 파일 자체를 나눈 이유는 같은 부위여도 front에서의 path경로와 back에서의 path경로가 달랐기 때문이다.

 

 

 

path경로를 모두 변수로 분리한 후, 각 신체부위의 명칭과 path경로를 key-value로 매핑해주었다.

// svgMapping.ts 파일

import * as SvgPathFront from "./PartAreaFront";
import * as SvgPathBack from "./PartAreaBack";
import * as SvgPathFace from "./PartAreaFace";

export const FrontPaths:any = {
  "head": [SvgPathFront.HEAD],
  "neck": [SvgPathFront.NECK],
  "shoulder": [SvgPathFront.SHOULDER_LEFT, SvgPathFront.SHOULDER_RIGHT],
  "upperArm": [SvgPathFront.UPPERARM_LEFT, SvgPathFront.UPPERARM_RIGHT],
  "albow": [SvgPathFront.ALBOW_LEFT, SvgPathFront.ALBOW_RIGHT],
  ...
  "sexOrgan": [SvgPathFront.SEXORGAN], 
  "pelvis": [SvgPathFront.PELVIS_LEFT, SvgPathFront.PELVIS_RIGHT]
}

export const BackPaths:any = {
  "head": [SvgPathBack.HEAD],
  "neck": [SvgPathBack.NECK],
  "shoulder": [SvgPathBack.SHOULDER_LEFT, SvgPathBack.SHOULDER_RIGHT],
  "upperArm": [SvgPathBack.UPPERARM_LEFT, SvgPathBack.UPPERARM_RIGHT],
  "albow": [SvgPathBack.ALBOW_LEFT, SvgPathBack.ALBOW_RIGHT],
  ...
  "waist": [SvgPathBack.WAIST],
  "hip": [SvgPathBack.HIP]
}

export const FacePaths:any = {
  "head": [SvgPathFace.HEAD],
  "forehead": [SvgPathFace.FOREHEAD],
  "eyes": [SvgPathFace.EYE_LEFT, SvgPathFace.EYE_RIGHT],
  "cheek": [SvgPathFace.CHEEK_LEFT, SvgPathFace.CHEEK_RIGHT],
  "nose": [SvgPathFace.NOSE],
  "mouth": [SvgPathFace.MOUTH],
  "chin": [SvgPathFace.CHIN],
  "ears": [SvgPathFace.EAR_LEFT, SvgPathFace.EAR_RIGHT],
}

 

왼쪽, 오른쪽 구분이 있는 부위의 경우, 한쪽만 마우스오버하거나 클릭해도 양쪽 모두 반응해야하기 때문에 value는 리스트로 작성하였다.

 

 

신체부위와 path의 매핑을 완료한 후, 각 신체부위와 신체부위가 포함된 신체 포지션(앞, 뒤, 얼굴)의 관계를 정의해주기 위해

여러 포지션에서 공통되는 신체부위는 common이라는 변수에 리스트로 선언하고

각 포지션 별로 다른 신체부위는 front, back, face변수로 선언하여 bodyFront는 신체 포지션 앞부분의 모든 신체부위를 포함하도록

bodyBack은 신체 포지션 뒷부분의 모든 신체부의를 포함하도록 정의해주었다.

const common: bodyPartType[] = [
  "head",
  "neck",
  "shoulder",
  "upperArm",
  "albow",
  "forearm",
  "wrist",
  "hand",
  "thigh",
  "knee",
  "calf",
  "ankle",
  "foot",
];
const front: bodyPartType[] = ["chest", "stomach", "sexOrgan", "pelvis"];
const back: bodyPartType[] = ["back", "waist", "hip"];
const face: bodyPartType[] = ["head", "forehead", "eyes", "nose", "mouth", "cheek", "chin", "ears"];

const bodyFront = [...common, ...front];
const bodyBack = [...common, ...back];

 

 

마지막으로, BodyNavigator.tsx에서 각 신체포지션(앞, 뒤, 얼굴)별로 길게 나열되어있던 HoverPath컴포넌트들을 map() 함수를 사용하여 작성하였다.

 

기존 코드와 리팩토링한 코드 비교해보기

 

 

결과적으로 리팩토링을 통해,

불필요하게 1000줄 넘게 나열되어 보기 힘들던 svg코드를 반이상 줄여서 효율적으로 작성하였고,

path가 변경되더라도 BodyNavigator.tsx에서 일일이 위치를 찾아가며 수정하기보단

별도로 정리된 파일에서 쉽게 찾아 관리할 수 있도록 하였다고 생각한다.

 

 

 

아직 조금 더 수정해야할 아쉬운 점은,

map()을 사용해서 HoverPath컴포넌트 반환 시 왼쪽, 오른쪽 구분이 있는 신체부위와 없는 부위를 if-else문으로 나눠서 작성한 점이다.

if (FrontPaths[part!].length > 1) {
  return (
    <React.Fragment key={`${part} + front`}>
      <HoverPath
        isChecked={isWritePage ? selectedBodyPart === part : position === part}
        onClick={() => {
          if (isWritePage) {
            setSelectedBodyPart(part);
          } else {
            setCurrentPosition("front");
            router.push(
              isHospital
                ? {
                    pathname: `/hospital/chart`,
                    query: { position: part },
                  }
                : `/users/records/chart/${part}`,
            );
          }
        }}
        isHover={hoveredPart === part}
        onMouseEnter={() => setHoveredPart(part!)}
        onMouseLeave={() => setHoveredPart("")}
        d={FrontPaths[part!][0]}
      />
      <HoverPath
        isChecked={isWritePage ? selectedBodyPart === part : position === part}
        onClick={() => {
          if (isWritePage) {
            setSelectedBodyPart(part);
          } else {
            setCurrentPosition("front");
            router.push(
              isHospital
                ? {
                    pathname: `/hospital/chart`,
                    query: { position: part },
                  }
                : `/users/records/chart/${part}`,
            );
          }
        }}
        isHover={hoveredPart === part}
        onMouseEnter={() => setHoveredPart(part!)}
        onMouseLeave={() => setHoveredPart("")}
        d={FrontPaths[part!][1]}
      />
    </React.Fragment>
  );
} else {
  return (
    <HoverPath
      key={index}
      isChecked={isWritePage ? selectedBodyPart === part : position === part}
      onClick={() => {
        if (part === "head") {
          setCurrentPosition("face");
        } else {
          isWritePage
            ? setSelectedBodyPart(part)
            : router.push(
                isHospital
                  ? {
                      pathname: `/hospital/chart`,
                      query: { position: part },
                    }
                  : `/users/records/chart/${part}`,
              );
        }
      }}
      isHover={hoveredPart === part}
      onMouseEnter={() => setHoveredPart(part!)}
      onMouseLeave={() => setHoveredPart("")}
      d={FrontPaths[part!]}
    />
  );
}

 

이 부분을 if문을 사용하지 않고 map() 안에서 한번 더 map을 통해 작성하고 싶었는데 return을 사용하다보니

첫번째 반복에서 return을 해버려서 원하는대로 작동이 되지 않았고,

map안에 map을 한번 더 작성하는게 더 좋을지 코드가 길지 않으니 그냥 if-else문으로 작성하는게 더 좋을지 확신이 서지 않았다.

우선은 다른 작업들도 할게 많아서 이 정도에서 리팩토링을 끝냈지만 프로젝트가 끝난 뒤

한번 더 차근차근 리팩토링을 해보아야겠다.

 

 

 

 

문제점 2) props drilling 사용으로 인해 해당 컴포넌트가 필요한 다른 페이지에서 재사용하기 어렵게 작성되어있다.

 

이 BodyNavigator 컴포넌트가 필요한 곳을 총 3 페이지이다.

기록하기 페이지 / 대시보드 페이지 / 차트 페이지

 

 

초기 BodyNavigator컴포넌트는 기록하기 페이지만을 기준으로 작성되어있었다.

기록하기 페이지의 BodyNavigator컴포넌트는 기록하기 페이지(next/pages/users/records/write/index.tsx)에서

바로 불러와서 사용되는 컴포넌트라 state를 props로 내려주어도 큰 불편이 없다.

 

하지만, 기록확인-대시보드 페이지, 기록확인-차트 페이지의 경우 

BodyNavigator컴포넌트까지 state를 내려주려면 4~5개의 props를 3번의 단계를 거쳐 내려줘야하기 때문에 불편하였다.

 

물론 처음부터 기록확인 페이지를 최대한 props drilling을 피할 수 있는 구조로 구성했다면 리팩토링이 필요없었을 수도 있지만,

추후 컴포넌트별로 추가할 애니메이션과 모바일 반응형을 고려하여 구성한 구조이기 때문에

컴포넌트 구성을 바꾸는 것 보다는 props drilling을 수정하는 방향이 더 좋은 방법이라고 생각했다.

 

 

초기 BodyNavigator컴포넌트에 내려지는 props는 총 5개였다.

 

selectedSite  :  클릭된 신체부위 string값

setSelectedSite  :  set함수

hoveredSite  :  마우스오버한 신체부위 string값

setHoveredStie  :  set함수

isWritePage  :  기록하기 페이지인지 기록확인 페이지인지 구분하여 일러스트 뒤에 네모배경을 줄것인지 결정하는 boolean값

 

여기에 기록확인하기 페이지에서는 현재 신체 포지션(앞, 뒤, 얼굴) 전역 state가 추가로 필요했다.

이유는 기록확인-대시보드 페이지에서 어떤 신체부위를 클릭하면 기록확인-차트 페이지로 이동하되

그 때의 신체 포지션(앞, 뒤, 얼굴)값을 기억하고 있어야하기 때문이다.

 

 

우선 마우스오버한 신체부위 값의 경우 props로 받을 필요 없이 BodyNavigator컴포넌트 안에서만

state로 관리해도 될 것 같아서 props에서 제외하고 BodyNavigator에 useState로 작성해주었다.

const [hoveredPart, setHoveredPart] = useState("");

 

 

또한, 리팩토링하면서 site, part, position 등 섞어서 사용되던 이름을 변수이름을 통일하기 위해

신체 포지션(앞, 뒤, 얼굴)에 관련된 변수는 ~~position으로,

신체부위(허리, 가슴 등)에 대한 부분은 ~~part, ~~bodyPart로 통일해주었다.

 

isWritePage의 경우, 기록하기 페이지인지 기록확인하기 페이지인지를 구분짓는 값이라

해당 페이지의 컴포넌트에서 props로 넘겨주는게 더 깔끔할 것 같아서 그대로 두기로 했다.

 

남은 selectedSite, setSelectedSite의 경우 recoil로 관리하도록 수정했었지만 이 작업을 수행할 당시

recoil이 프로젝트 전체적으로 별로 사용할 일이 없다면 아예 안쓰는 쪽으로 하자는 의견이 있었어서 다시 props로 넘겨주는 방식으로 되돌렸다.

(결과적으로 recoil이 필요한 여러 부분들이 생겨 프로젝트에서는 recoil을 사용했지만 이부분은 우선 시간 관계상 그대로 두었다.)

 

추가로 필요했던 state인 신체 포지션(앞, 뒤, 얼굴)의 경우 여러 페이지에서 값이 동일하게 공유되어야하는 부분이 있어서 recoil로 관리하도록 작성하였다.

 

 

 

결과적으로 리팩토링을 통해,

컴포넌트 간의 불필요하게 넘겨주던 props를 제거하여 코드를 깔끔하게 정리하였고, 필요한 state를 새로 추가하여 오류를 수정하였다.

 

 

아직 조금 더 수정해야할 아쉬운 점은,

작업을 하면서 서로 commit이 꼬여서 리팩토링한 파일이 덮어씌워지는 이슈도 있었고, 아직 경험이 부족하다보니 어떤 방법이 더 좋은지 확신이 안서는 부분도 많았던 것 같다. 그리고 나중에 모바일에 필요한 컴포넌트를 추가하다보니 다시 코드가 지저분해져서 꾸준한 리팩토링이 필요할 것 같다.