해당 포스트는..
프로젝트를 하면서 작성한 코드 중 리팩터링하면 좋을 코드를 수정해 나가며 학습하기 위한 목적으로 작성된 포스트 입니다.
데이터를 읽어온 뒤 바로 저장하는 로직
아래 코드는 출발지점의 경도/위도 상태와 도착지점의 경도/위도 상태의 변경에 따라 출발지에서 도착지 까지 길찾기를 수행하는 로직이다. 보면 알 수 있듯이 onClickGetDirections 라는 함수명을 가지고 있음에도 읽어오는(Get) 동작뿐만 아니라 setpath 와 같이 설정(Set) 하는 기능까지 수행하고 있다.
/**
* onClick | KAKAO API | 현재 위치에서 목적지 까지 길찾기 api 요청
* reference: https://developers.kakaomobility.com/docs/navi-api/directions/
*/
async function onClickGetDirections() {
const { lat: startLat, lng: startLng } = coords.start;
const { name: endName, lat: endLat, lng: endLng } = coords.end;
const url = `https://apis-navi.kakaomobility.com/v1/directions?origin=${startLng},${startLat}&destination=${endLng},${endLat},name=${endName}&waypoints=&priority=TIME&car_fuel=GASOLINE&car_hipass=false&alternatives=false&road_details=false`;
const { roads, distance } = (await getFetch(url, config)).routes[0]
.sections[0];
const newResult = extractLatAndLngArray(roads);
setPath(newResult);
setTotalDistance(distance);
}
[문제] 함수가 두 가지 이상의 기능을 하고 있다.
앞서 언급했듯이 해당 함수는 Get 이라고 명시된 동작 외에도 Set 의 기능도 동시에 수행함에 따라 여러 책임을 가진다.
이 부분이 문제가 되는 것중 하나는 해당 함수의 이름만 보고는 도대체 어떤 동작을 수행하는지 감이 잡히지 않으므로 결국 로직 자체를 뜯어볼 수 밖에 없다는 점이다. 이는 유지보수를 어렵게 만들고, 각 기능이 혼재되어 있기 때문에 테스트하기에도 곤란함을 야기할 수 있다.
따라서 해당 함수에서 onClick 를 두 기능의 래퍼 함수로 사용하고 나머지 Get과 Set 의 역할을 하는 함수를 각각 분리(모듈화)하는 것이 향후 확장성이나 유지보수성, 테스트하기에도 좋아 보이기에 이를 반영하여 로직을 수정하였다.
개선 코드
/**
* CLICK EVENT | 길찾기
* reference: https://developers.kakaomobility.com/docs/navi-api/directions/
*/
async function onClickDirections() {
const { roads, distance } = await getDirections(coords)
const newRoads = extractLatAndLngArray(roads);
setDirections(newRoads, distance)
}
/** GET | 길찾기 좌표 정보 요청 */
async function getDirections(coords: CoordsType) {
const { lat: startLat, lng: startLng } = coords.start
const { lat: endLat, lng: endLng, name: endName } = coords.end
const url = `https://apis-navi.kakaomobility.com/v1/directions?origin=${startLng},${startLat}&destination=${endLng},${endLat},name=${endName}&waypoints=&priority=TIME&car_fuel=GASOLINE&car_hipass=false&alternatives=false&road_details=false`;
const { roads, distance } = (await getFetch(url, config)).routes[0]
.sections[0];
return { roads, distance }
}
/** SET | 출발지 - 도착지 좌표 및 총 거리 설정 */
function setDirections(newRoads: { lng: number, lat: number }[], distance: number) {
setPath(newRoads);
setTotalDistance(distance);
}
우선 onClick 이벤트를 호출하는 함수를 래퍼 함수로 두며, 그 안에서 각 기능을 모듈화하여 독립적인 단위로 관리할 수 있도록 로직을 수정했다.
getDirections 함수의 경우 출발지와 도착지의 위도경도 정보를 받아서 카카오 모빌리티의 길찾기 요청을 Fetch 라고 그 결과를 반환한다.
그 다음 앞서 반환 받은 데이터 중 거리 정보가 담겨 있는 roads 데이터를 카카오 Map SVG 상에 실제 맵핑할 수 있도록 재가공하기 위해 extractLatAndLngArray() 함수의 인자로 전달하도록 했다.
그 후 가공된 데이터를 반환받고, 이를 실제 화면에 표시할 수 있도록 상태를 재설정하는 setDirections 함수에 전달하도록 하였다.
계속 고민해봐야 하는 것
현재 setDirections 의 경우 상태를 두 가지 설정하는 메소드를 가지고 있고, 연관된 기능이기 때문에 같이 두었는데, 이렇게 되면, 해당 함수는 각각의 set 메소드에 의존성을 가지기 때문에 재사용하기가 어렵다. 따라서 이를 재사용할 수 있도록 하는 것이 맞을지, 그 이상 재사용할 이유가 없다고 보고 이대로 두는 것이 맞을지가 앞으로의 과제라고 생각한다.
리사이징에 따른 슬라이드 전환 애니메이션
아래 코드는 이번에 슬라이드 전환 애니메이션 GSAP Flip 을 사용하여 구현한 코드이다. 윈도우 크기가 리사이징 되면, 새로 측정한 innerWidth 를 1024px 를 기준으로 가로 방향으로 플립 애니메이션이 동작하는 것을 1024px 밑으로는 세로 방향으로 애니메이션이 동작하도록 설계 되어 있다. ( 참고로 resizeWidth 는 커스텀 훅을 사용하여 내부적으로 디바운싱 기술이 적용되어 있다)
/** 슬라이드 전환 및 윈도우 리사이즈 시 GSAP FliP 애니메이션 적용 */
useEffect(() => {
if (!wrapperRef.current) return;
const element = wrapperRef.current;
const elements = gsap.utils.toArray([
'#first_article',
'#second_article',
]) as HTMLElement[];
const state = Flip.getState(elements);
page === 0 &&
(element.style.cssText =
resizeWidth > 1024
? `flex-direction: row !important`
: `flex-direction: column !important`);
page === 1 &&
(element.style.cssText =
resizeWidth > 1024
? `flex-direction: row-reverse !important`
: `flex-direction: column-reverse !important`);
Flip.from(state, {
duration: 1,
ease: 'power2.inOut',
absolute: false,
});
}, [page, resizeWidth]);
useEffect 훅의 경우에는 의존성 배열에 있는 page 와 resizeWidth 가 변동될 때 마다 매번 리렌더링을 촉발시킨다. 따라서 해당 콜백 내에 있는 모든 로직들이 새롭게 생성되는 것과 같다.
[문제] css 분기처리 로직의 불필요한 재생성
그 중에서 아래 로직의 경우에는 매번 조건이 바뀔 때 마다 flex-direction 에 적용하는 값이 리렌더링 시 마다 새롭게 추가되기 때문에 성능상 좋지 못하다.
개선 코드
따라서 해당 로직을 외부로 분리하여 매번 생성되는 비효율적인 처리를 제거해준다.
const flexDirection = resizeWidth > 1024 ? 'row' : 'column';
const flexDirectionReverse = resizeWidth > 1024 ? 'row-reverse' : 'column-reverse';
/** 슬라이드 전환 및 윈도우 리사이즈 시 GSAP FliP 애니메이션 적용 */
useEffect(() => {
if (!wrapperRef.current) return;
const element = wrapperRef.current;
const elements = gsap.utils.toArray([
'#first_article',
'#second_article',
]) as HTMLElement[];
const state = Flip.getState(elements);
element.style.cssText=`flex-direction: ${page ===0 ? flexDirection : flexDirectionReverse} !important`
Flip.from(state, {
duration: 1,
ease: 'power2.inOut',
absolute: false,
});
}, [page, resizeWidth, flexDirection, flexDirectionReverse]);
[문제] gsap.utils.toArray 메소드의 불필요한 연산
앞서 개선된 코드에서 gsap 의 유틸 함수 중 toArray 메소드를 사용하여 배열의 요소로 전달된 id 를 가진 HTML 태그를 가져와서 자바스크립트 배열로 만들고 이를 elements 라는 변수에 담아주고 있는데, 이 경우도 굳이 useEffect 내부에서 사용하지 않아도 동작에 문제를 주지 않는다. 애초에 배열에 비어 있다면 빈배열을 반환하고, 애니메이션을 실행하는 경우에는 해당 요소들은 DOM 트리에서 읽어올 수 있기 때문에, 굳이 내부에 둘 필요가 없는 것이다.
개선 코드
따라서 해당 elements 변수의 경우에도 외부에서 처리하여 사용할 수 있도록 로직을 분리한다. 아까 보다 외부에 의존하는 요소들이 증가하여 의존성 배열이 커진 것은 흠이지만, 많이 가벼워 진 것을 확인할 수 있다.
/** 슬라이드 전환 및 윈도우 리사이즈 시 GSAP FliP 애니메이션 적용 */
useEffect(() => {
if (!wrapperRef.current) return;
const element = wrapperRef.current;
const state = Flip.getState(elements);
element.style.cssText=`flex-direction: ${page ===0 ? flexDirection : flexDirectionReverse} !important`
Flip.from(state, {
duration: 1,
ease: 'power2.inOut',
absolute: false,
});
}, [page, resizeWidth, flexDirection, flexDirectionReverse, elements]);
[문제] 불필요한 변수 선언
코드 가독성을 높이고자 element 변수를 선언하고, 그 후 div 요소의 참조를 담고 있는 wrapper.current 를 할당하여 사용하고 있었는데, element 라는 변수를 재사용하는 경우가 없기 때문에 굳이 이렇게 변수를 초기화하여 사용하는 것은 불필요한 메모리 공간을 차지하는 것 일 수 있다.
개선 코드
따라서 해당 변수를 제거하여 warpper.current 에 직접 접근할 수 있도록 로직을 수정한다.
/** 슬라이드 전환 및 윈도우 리사이즈 시 GSAP FliP 애니메이션 적용 */
useEffect(() => {
if (!wrapperRef.current) return;
const state = Flip.getState(elements);
wrapperRef.current.style.cssText = `
flex-direction: ${page === 0
? flexDirection
: flexDirectionReverse} !important`
Flip.from(state, {
duration: 1,
ease: 'power2.inOut',
absolute: false,
});
}, [page, resizeWidth, flexDirection, flexDirectionReverse, elements]);
이게 맞을까?
일단은 위와 같이 로직을 수정해 보았으나, 큰 문제점이 존재하는 것을 확인했다. 애초에 flexDirection 과 같은 조건에 따라 할당되는 값이 다른 경우에는 강제적으로 의존성을 가질 수 밖에 없다. 이러한 로직을 바깥으로 빼낸다고 하더라도, 그 의존성이 사라지는 것은 아니므로 결국 useEffect 의 의존성 배열에 추가함으로써 로직을 수정하기 전과 후에 동작상에는 실질적으로 아무런 변화가 없다고 보는게 타당하다.
즉, 해당 로직이 간소화되고 가벼워져보이는 것과는 별개로 해당 리팩터링은 옳은 방향이라고 보기는 어렵고, 이를 어떻게 하면 의존성을 줄이고, 필요한 상황에서만 리렌더링을 촉발시킬 수 있을지 고민해봐야 겠다.