-
[Portfolly] Five Lines of Code 규칙을 (최대한) 지켜보자Studying/Proj 과정 2023. 7. 27. 02:58
개요
Portfolly 프로젝트의 '포트폴리오 작성 페이지'에서 사용되는 컴포넌트인 TitleForm.tsx 파일이 너무 복잡해지는 문제가 발생했습니다. 따라서 useTitleForm 커스텀 훅을 만들어 로직을 분리했으나, 다시 useTitleForm.ts 파일이 복잡해지는 굴레에 빠지고 말았습니다.
다행히 실시간 멘토링 코드 리뷰를 통해 코드를 정리하게 되었고, 그 과정을 기록합니다.
기존 복잡한 방식의 코드
기존 useTitleForm.ts 코드는 다음과 같습니다.
// useTitleForm.ts import { useNavigate } from 'react-router-dom'; import { useDispatch, useSelector } from 'react-redux'; import { Picture, PortfolioContent, Tag } from '@/types'; import { INITIAL_PORTFOLIO } from '@/types/initials'; import { call } from '@/utils/apiService'; import { pictures, portfolio, setPortfolio } from '@/store/portfolioSlice'; export default function useTitleForm() { const savedPortfolio = useSelector(portfolio); const savedPictures = useSelector(pictures); const navigate = useNavigate(); const dispatch = useDispatch(); const postPortfolio = (body: PortfolioContent) => call('/portfolios', 'POST', body); const modifyPortfolio = (body: PortfolioContent) => call(`/portfolios/${savedPortfolio.id}`, 'PATCH', body); const deletePicture = (body: Picture) => call(`portfolios/s3/picture`, 'DELETE', body); const alertValidation = (message: string) => { alert(message); return false; }; const checkValidation = (portfolio: PortfolioContent) => { // if (savedPictures.length < 1) return alertValidation('이미지를 1개 이상 첨부해주세요.'); if (portfolio.title.length < 1 || portfolio.title.length > 30) return alertValidation('제목은 1자 이상 30자 미만으로 작성해주세요.'); if (portfolio.tags.length < 1) return alertValidation('태그를 최소 1개 이상 선택해주세요.'); if (portfolio.explains.length < 1 || portfolio.explains.length > 300) return alertValidation('소개글은 1자 이상 300자 미만으로 작성해주세요.'); return true; }; const deleteImageUrls = (htmlContent: string) => { savedPictures.forEach((url: string) => { const isImageUrlRemoved = !htmlContent.indexOf(url); if (isImageUrlRemoved) deletePicture({ fileName: url }); }); }; const changeArraytoString = (tags: Array<Tag>) => { let arrayString = '['; tags.forEach((tag: Tag) => { arrayString = arrayString.concat('{id:' + tag.id + ", name:'" + tag.name + "'},"); }); arrayString = arrayString.slice(0, -1); arrayString = arrayString + ']'; return arrayString; }; const submitPortfolio = () => { const isModified = savedPortfolio.id ? true : false; const isValid = checkValidation(savedPortfolio); deleteImageUrls(savedPortfolio.content); if (isValid) { const copiedPortfolio = JSON.parse(JSON.stringify(savedPortfolio)); copiedPortfolio.tags = changeArraytoString(copiedPortfolio.tags); copiedPortfolio.content = copiedPortfolio.content.replace(/"/g, "'"); delete copiedPortfolio.portfolioId; delete copiedPortfolio.createdAt; const body = copiedPortfolio; if (isModified) modifyPortfolio(body).then(() => navigate(`/portfolios/${savedPortfolio.id}`)); else postPortfolio(body).then(() => navigate(`/main`)); dispatch(setPortfolio(INITIAL_PORTFOLIO)); } }; return { submitPortfolio }; }
단계 1. TitleForm 에서만 사용하는 utils 파일을 만들자.
일단 useTitleForm의 반환값은 submitPortfolio 입니다. 그 외에는 관심을 둘 필요가 없습니다.
따라서 submitPortfolio를 제외한 다른 함수들은 모두 utils.ts 파일로 옮겨보겠습니다.
가장 먼저 아래 세 개의 함수를 분리하겠습니다.
// utils.ts export const postPortfolio = (body: PortfolioContent) => call('/portfolios', 'POST', body); // export const modifyPortfolio = (body: PortfolioContent) => call(`/portfolios/${savedPortfolio.id}`, 'PATCH', body); export const modifyPortfolio = (body: PortfolioContent, id: string) => call(`/portfolios/${id}`, 'PATCH', body); export const deletePicture = (body: Picture) => call(`portfolios/s3/picture`, 'DELETE', body);
그럴 경우 2번 라인(주석)의 savedPortfolio.id 값을 모르기 때문에 외부에서 불러와야 하는 상황이 발생합니다.
이 부분을 주의해야 합니다. 개발을 할 때는 사용해야 하는 변수의 범위가 멀어져선 안됩니다. 함수형 프로그래밍을 해야한다는 것을 잊지 말고, id를 매개변수로 받습니다.
함수형 프로그래밍에서 중요한 것은 사이드 이펙트를 줄이는 것입니다.
잠깐 사이드 이펙트가 무엇인지 정리해보겠습니다.
// 다음과 같이 오직 리턴 값만 있는 걸 순수함수 라고 합니다. function add(a, b){ return a + b; } // 다음과 같이 리턴과 전혀 상관 없는 로직을 사이드 이펙트라고 합니다. function example(){ add(1, 2) // side effect } // 이런 경우는 어떨까요?(1) function add(a, b){ example(add(1, 2)); // 결과랑 상관이 없으므로 side effect 입니다. return a+b; } // 이런 경우는 어떨까요?(2) function add(a, b){ const total = a + b; // 결과에 반영되므로 side effect가 아닙니다. return total; }
이렇게 함수가 리턴하는 것과 관계 없는 건 전부 사이드 이펙트입니다.
꼭 악영향을 끼치는 것만 사이드 이펙트가 아니라, 그냥 함수의 본 목적(결과값)과 관계없는 부가적인 기능이면 전부 사이드 이펙트인 것입니다.
이제 위 규칙에 따라 관련 없는 나머지 함수들도 모조리 빼면 다음과 같습니다.
// utils.ts import { Picture, PortfolioContent, Tag } from '@/types'; import { call } from '@/utils/apiService'; export const postPortfolio = (body: PortfolioContent) => call('/portfolios', 'POST', body); export const modifyPortfolio = (body: PortfolioContent, id: string) => call(`/portfolios/${id}`, 'PATCH', body); export const deletePicture = (body: Picture) => call(`portfolios/s3/picture`, 'DELETE', body); export const alertValidation = (message: string) => { alert(message); return false; }; export const checkValidation = (portfolio: PortfolioContent) => { // if (savedPictures.length < 1) return alertValidation('이미지를 1개 이상 첨부해주세요.'); if (portfolio.title.length < 1 || portfolio.title.length > 30) return alertValidation('제목은 1자 이상 30자 미만으로 작성해주세요.'); if (portfolio.tags.length < 1) return alertValidation('태그를 최소 1개 이상 선택해주세요.'); if (portfolio.explains.length < 1 || portfolio.explains.length > 300) return alertValidation('소개글은 1자 이상 300자 미만으로 작성해주세요.'); return true; }; export const deleteImageUrls = (htmlContent: string, savedPictures: string[]) => { savedPictures.forEach((url: string) => { const isImageUrlRemoved = !htmlContent.indexOf(url); if (isImageUrlRemoved) deletePicture({ fileName: url }); }); }; export const changeArraytoString = (tags: Array<Tag>) => { let arrayString = '['; tags.forEach((tag: Tag) => { arrayString = arrayString.concat('{id:' + tag.id + ", name:'" + tag.name + "'},"); }); arrayString = arrayString.slice(0, -1); arrayString = arrayString + ']'; return arrayString; };
단계 2. 5 Lines of Code 규칙을 최대한 지키자.
그런데도 아직까지 코드가 복잡해 보이는 건 여전합니다. 그 이유는 바로 함수의 형태에 있습니다.
다음 함수의 문제점들을 찾아보겠습니다.
// useTitleForm.ts const submitPortfolio = () => { const isModified = savedPortfolio.id ? true : false; const isValid = checkValidation(savedPortfolio); deleteImageUrls(savedPortfolio.content); if (isValid) { const copiedPortfolio = JSON.parse(JSON.stringify(savedPortfolio)); copiedPortfolio.tags = changeArraytoString(copiedPortfolio.tags); copiedPortfolio.content = copiedPortfolio.content.replace(/"/g, "'"); delete copiedPortfolio.portfolioId; delete copiedPortfolio.createdAt; const body = copiedPortfolio; if (isModified) modifyPortfolio(body).then(() => navigate(`/portfolios/${savedPortfolio.id}`)); else postPortfolio(body).then(() => navigate(`/main`)); dispatch(setPortfolio(INITIAL_PORTFOLIO)); } };
- if(isValid) 조건문 아래가 너무 뚱뚱하다.
- 코드가 뭉쳐있으니 가독성이 나쁘다.
if(isValid) 는 !isValid인 경우 아무런 동작도 발생하지 않습니다. 따라서 eary return pattern을 적용해 !isValid일 때 빨리 반환값을 보내 함수에서 탈출하는 게 좋습니다.
수정하면 다음과 같습니다.
const submitPortfolio = async () => { const isModified = savedPortfolio.id ? true : false; const isValid = checkValidation(savedPortfolio); deleteImageUrls(savedPortfolio.content, savedPictures); // side effect if (!isValid) return; const copiedPortfolio = JSON.parse(JSON.stringify(savedPortfolio)); copiedPortfolio.tags = changeArraytoString(copiedPortfolio.tags); copiedPortfolio.content = copiedPortfolio.content.replace(/"/g, "'"); delete copiedPortfolio.portfolioId; delete copiedPortfolio.createdAt; const body = copiedPortfolio; if (isModified) modifyPortfolio(body).then(() => navigate(`/portfolios/${savedPortfolio.id}`)); else postPortfolio(body).then(() => navigate(`/main`)); dispatch(setPortfolio(INITIAL_PORTFOLIO)); };
이제 isValid 인 경우 발생하는 동작을 보겠습니다.
객체를 깊은 복사한 다음, key에 대한 값을 변경하고 삭제합니다. 하지만 이 코드는 위와 같이 길게 작성할 필요가 없이 한 줄로 가능합니다.
const copiedPortfolio = JSON.parse(JSON.stringify(portfolio)); const updatedPortfolio = { ...copiedPortfolio, tags: changeArraytoString(copiedPortfolio.tags), content: copiedPortfolio.content.replace(/"/g, "'"), } const {portfolioId, createdAt, ...restUpdatedPortfolio} = updatedPortfolio;
위와 같이 구조 분해 할당을 이용해 restUpdatedPortfolio 라는 원하는 객체를 뽑아내는 겁니다.
그런데 이 과정 또한 하나의 함수로 빼서 utils 파일에 분리하면 더욱 코드를 이해하기 좋겠습니다. Portfolio 객체에서 몇 가지 key를 제거한 객체를 반환받는 함수이니 그 이름을 createdPortfolioWithoutSome 이라고 작성하겠습니다.
다음과 같이 utils 파일에 메서드를 분리했습니다.
// utils.ts export const createPortfolioWithoutSome = (portfolio: PortfolioContent) => { const copiedPortfolio = JSON.parse(JSON.stringify(portfolio)); const updatedPortfolio = { ...copiedPortfolio, tags: changeArraytoString(copiedPortfolio.tags), content: copiedPortfolio.content.replace(/"/g, "'"), } const {portfolioId, createdAt, ...restUpdatedPortfolio} = updatedPortfolio; return restUpdatedPortfolio; }
const submitPortfolio = async () => { const isModified = savedPortfolio.id ? true : false; const isValid = checkValidation(savedPortfolio); const 원했던객체 = createPortfolioWithoutSome(savedPortfolio); deleteImageUrls(savedPortfolio.content, savedPictures); // side effect if (!isValid) return; if (isModified) modifyPortfolio(body).then(() => navigate(`/portfolios/${savedPortfolio.id}`)); else postPortfolio(body).then(() => navigate(`/main`)); dispatch(setPortfolio(INITIAL_PORTFOLIO)); };
그런데 아직도 고쳐야 할 부분이 있습니다. if(isModified) / else 문입니다.
if/else는 여집합 관계입니다. 이는 단번에 구조를 파악하는데 어려움을 주기 때문에 최대한 else 문은 사용하지 않는 게 좋습니다. 두 번 해석해야 하는 과정을 제거해버리는 겁니다.
그리고 async await을 배운 만큼 .then 보다는 await를 활용하는 게 코드 가독성에 더 효율적입니다.
추가로, 필수는 아니지만 void 함수 또한 return null; 이라고 항상 명시하는 습관을 가지면 좋습니다.
최종적으로 수정하면 다음과 같이 정리됩니다.
if (isModified) { await modifyPortfolio(원했던객체, savedPortfolio.id as string) // side effect await navigate(`/portfolios/${savedPortfolio.id}`); // side effect dispatch(setPortfolio(INITIAL_PORTFOLIO)); // side effect return; } await postPortfolio(원했던객체) // side effect await navigate(`/main`) // side effect dispatch(setPortfolio(INITIAL_PORTFOLIO)); // side effect return;
그 결과 바뀐 코드
// useTitleForm.ts import { useNavigate } from 'react-router-dom'; import { useDispatch, useSelector } from 'react-redux'; import { INITIAL_PORTFOLIO } from '@/types/initials'; import { pictures, portfolio, setPortfolio } from '@/store/portfolioSlice'; import { checkValidation, createPortfolioWithoutSome, deleteImageUrls, modifyPortfolio, postPortfolio } from './utils'; export default function useTitleForm() { const savedPortfolio = useSelector(portfolio); const savedPictures = useSelector(pictures); const navigate = useNavigate(); const dispatch = useDispatch(); const submitPortfolio = async () => { const isModified = savedPortfolio.id ? true : false; const isValid = checkValidation(savedPortfolio); const 원했던객체 = createPortfolioWithoutSome(savedPortfolio); deleteImageUrls(savedPortfolio.content, savedPictures); // side effect if (!isValid) return; if (isModified) { await modifyPortfolio(원했던객체, savedPortfolio.id as string) // side effect await navigate(`/portfolios/${savedPortfolio.id}`); // side effect dispatch(setPortfolio(INITIAL_PORTFOLIO)); // side effect return; } await postPortfolio(원했던객체) // side effect await navigate(`/main`) // side effect dispatch(setPortfolio(INITIAL_PORTFOLIO)); // side effect return; }; return { submitPortfolio }; }
엄청 단순해졌습니다!
처음에 복잡한 코드를 다시 보면 왜 저렇게 작성했었는지 도무지 알 수 없을 정도로 간단하게 코드가 정리되었습니다.
비록 피할 수 없는 사이드 이펙트는 존재하지만, submitPortfolio 메서드 자체가 특별한 반환값이 없는 void 함수인데다가, Portfolio를 Submit하는 데 필요한 과정이라 내버려두었습니다.
새롭게 배운 점으로는 유틸 함수를 utils.ts 파일로 분리해서 관리한다는 것, if/else 문에서 else는 최대한 작성하지 않는 것, 그리고 early return pattern에 익숙해지는 것이었습니다.
이제 남은 코드들도 위 패턴을 기억해 두었다가 천천히 바꿔나가야겠습니다...
'Studying > Proj 과정' 카테고리의 다른 글
[Portfolly] 코드 Depth를 최대한 줄여보자 (0) 2023.07.27 [Portfolly] KISS 원칙으로 코드를 바꿔보자 (0) 2023.07.27 [Portfolly] 스프린트 3주차 회고 (0) 2023.07.23 [Portfolly] 스프린트 2주차 회고 (0) 2023.07.16 [Portfolly] 카테고리 Selector 만들기 (0) 2023.07.12