이번 회차를 통해서 좋은 코드에 대해서 새로운 시선을 가지게 되었다. 클린 코드에 대한 내용을 모두 익힐수는 없었지만, 최소한의 가이드라인을 세우고, 좋은 코드를 짜는 나만의 기준을 만들고 리팩토링을 할 수 있었다.
회차를 통해 배우고 느낀점.
내가 세션을 듣고, 추천 영상등을 챙겨보면서 새로 배운점을 먼저 이야기 해보려고 한다.
- 선언적 프로그래밍. HOW가 아닌 WHAT으로 코드를 작성해야 가독성이 더 좋다.
- 관심사 분리. business 로직과 view로직은 분리하자.
- 적절한 추상화. 지나친 추상화는 오히려 안좋다. 그리고 추상화는 동일한 수준으로 맞춰주자.
- 비동기 처리를 더 가독성 좋게 작성하기.
머리속에 넣었으니 응용을 해보자.
1. 네이밍 수정.
const loginHandler = ({ email, password }: LoginParams) => {
const onSuccess = ({ details, token }: LoginResponse) => {
if (token) {
window.alert('로그인이 완료되었습니다.');
localStorage.setItem('TOKEN', token);
window.location.reload();
}
};
const loginAndRefresh = ({ email, password }: LoginParams) => {
const onSuccess = ({ details, token }: LoginResponse) => {
if (token) {
window.alert('로그인이 완료되었습니다.');
localStorage.setItem('TOKEN', token);
window.location.reload();
}
};
};
조금 부끄러운 예시이지만, 함수의 이름을 loginHandler에서 loginAndRefresh로 바꿨다. 작은 수정이지만, 함수가 단순히 로그인만 하는것이 아니라, 로그인을 하고 새로고침까지 한다는 내용을 네이밍에 포함해 봤다.
<TodoDetailWrapper> -> <TodoDetailContainer>
// wrapper는 단일 element를 감쌀때 사용.
// container는 여러개의 element를 감쌀때 사용.
https://stackoverflow.com/questions/4059163/css-language-speak-container-vs-wrapper
기존에는 가장 바깥쪽의 엘리먼트를 모두 Wrapper라는 네이밍을 사용했다. 네이밍에 고민을 많이 안한채로 사용을 했던것 같아서, 이번 기회에 검색을 해봤더니 Container라는 용어가 더 적절한 네임이이었다.
눈에 보이던 두가지를 우선 수정 했다. 이전에는 변수나 함수의 이름을 지을 때, 고민을 많이 안하고 작성한것 같다는 생각이 들었다. 앞으로는 더 네이밍에 신경을 써야겠다.
2. 관심사 분리
// 변경 전
const { data: todosData } = useQuery<ApiGetTodosResponse>(['getTodos'], apiGetTodos);
// 변경 후
const { data: todoData } = useGetTodoById(todoId);
기존에는 tsx파일에 모든 api요청을 보내는 코드를 작성했다. 과거에는 컴포넌트에 관련된 view logic과 buisness로직을 모두 모아서 작성하는게 응집도가 높고 유지보수가 편한게 아닌가? 라는 생각을 했다. 세션을 듣고 관련 영상을 보고나니 적절하게 추상화를 하여 관심사가 분리된 코드를 작성하게되면 더 유지보수가 편해진다는점을 느끼게 되었다. tsx파일에는 최대한 view와 관련된 코드만 작성하도록 했고, 관련 로직들은 바깥에서 처리하기로 했다. 변경 전의 코드는 How의 방식으로 작성된 코드이다. useQuery를 사용하고, 비동기 코드를 인자로 넣어 getTodos를 하여 데이터를 불러오는 함수로써 작성되었다. 어떻게 함수가 동작하는지를 설명하는 코드가 되어버린것이다. 변경후의 코드는 What의 방식으로 작성이 되었고, hook의 이름만 봐도 어떤동작을 할 지 예측이 된다. 위의 변경점을 통해서 관심사 분리를 통해서 view 영역에서는 view에 관련된 코드만 모아지게 되므로, 유지보수도 더 편해지게 되었다.
3. 적절한 추상화.
// 기타 다른 코드들은 정리를 하고 view에 해당하는 코드만 옮겨왔다.
const Wrapper = styled.div`
width: 1000px;
padding-top: 50px;
.wrapper-section {
// 생략
}
form {
// 생략
}
`;
function HomeScreen() {
const { data: todosData, refetch: refetchTodos } = useQuery<ApiGetTodosResponse>(['getTodos'], apiGetTodos);
const { mutate } = useMutation(apiCreateTodo);
const { id: todoId } = useParams();
const { register, handleSubmit, setValue } = useForm<TodoParams>();
const saveTodoHandler = ({ title, content }: TodoParams) => {};
return (
<Wrapper>
<form onSubmit={handleSubmit(saveTodoHandler)}>
<InputBasic register={register('title')} placeholder="주제." data-cy="input-todo-title" />
<InputBasic register={register('content')} placeholder="할 일을 입력 해 주세요." data-cy="input-todo-content" />
<ButtonBasic title="저장" type="submit" data-cy="button-save-todo" />
</form>
<div className="wrapper-section">
<section>
<ul data-cy="wrapper-todo-list">
{!!todosData?.length &&
todosData?.map((todo) => (
<TodoListCard key={todo.id} {...todo} refetchTodos={refetchTodos} selected={todoId === todo.id} />
))}
</ul>
</section>
<TodoDetail refetchTodos={refetchTodos} />
</div>
</Wrapper>
);
}
export default HomeScreen;
위의 코드를 보자. 실제로 내가 이번 챌린지를 위해 작성한 코드이다. 이렇게 작성된 코드의 특징은 작성할때는 알아볼 수 있어도 몇일이 지난 후 보면 이해할 수 없게 된다. 이 당시 내 머리속에서는 Todo List이기 때문에 코드량이 많지 않을 것이라고 생각을 했기 때문에, 적절하게 분리를 하지 않았다. 컴포넌트로 분리하는게 때로는 복잡도를 더 증가 시키기도 하지만, 당시에는 간단하다고 생각한 코드가 지금은 매우 복잡하게 느껴진다. 코드가 복잡한 이유를 한번 생각해보자.
- 추상화의 단계가 서로 다르다.
- return 단의 jsx코드를 보자. 일반 element와 추상화된 컴포넌트들이 섞여있다. 서로 비슷한 수준의 중요도를 가지고 있는 컴포넌트 일 지라도, 추상화 되지 않은 컴포넌트들이 장황하게 작성되어있어서, 시선의 대부분을 뺏어간다. 정확한 맥락을 파악하기 힘들어진다.
- 결과물이 무엇인지 예측이 어렵다.
- 어떤 로직을 통해서 컴포넌트가 렌더링되는지를 한번 쭉 따라서 읽어봐야 이해할 수 있다. 그리고 한번 이해를 했더라도 그 기억은 휘발되어 날아간다. 다음에 다시 코드를 보게 된다면 처음부터 다시 이해해야 하므로, 코드를 읽는데 시간이 낭비된다. 코드가 How로 작성된 예시이다.
- 관심사 분리.
- Screen이라는 단위의 컴포넌트에서 대부분의 로직이 전부 작성되어있다. 코드를 이해해보면 크게 3부분으로 나누어져있다. 만약 컴포넌트 한 부분을 수정하려 해도, 모든 내용이 작성된 코드를 전부 훑어봐야 원하는 부분을 찾아 수정 할 수 있다. 유지보수가 어렵다.
- Styled-component의 복잡성.
- 여러 단계에 걸쳐서 컴포넌트가 공존하다보니 styled-component의 코드가 너무 복잡해졌다. 컴포넌트의 css를 확인하려면 마찬가지로 sytled-component의 훑어보고 원하는 부분을 수정해야한다. 이 역시 시간을 낭비하게 한다.
위 코드의 예시는 정말 날것의 코드라고 생각한다. 코드를 작성한 당사자만 이해할 수 있고, 당사자도 시간이 지나면 다시 보고 이해해야 할 것이다. 그럼 컴포넌트 분리와 추상화를 해보자.
우선 내가 가지고 있는 컴포넌트 구조의 기준에 대해서 다시 한번 되돌아보자.
컴포넌트를 분리하기 위해 처음에는 Atomic pattern을 적용했었다. 하지만 결과적으로는 나에게 그렇게 만족스러운 경험을 주지는 못했다. 그래서 Atomic pattern을 참고하여 적용중인 나만의 pattern을 사용하고있다. 기본적으로 모든 컴포넌트는 Screen을 기준으로 관리한다. 예외적으로, Input, Button같은 프로젝트 전체에 걸쳐서 사용되는 컴포넌트는 Common폴더에 저장하여 여러곳에서 사용 할 수 있게 한다. Atomic pattern을 적용했을 당시 가장 불만족스러웠던 부분은 다음과 같다.
- Button, Input등을 제외하고는 컴포넌트의 재사용이 그렇게 많이 일어나지 않는다.
- 특정 페이지에서 특정 기능을 위해 조립된 컴포넌트가 다른곳에서 쓰이는 경우가 많이 없다. 컴포넌트는 props가 아닌 데이터 model에 의존하기 때문이다.
- 하나의 컴포넌트를 여러곳에서 쓰기위해 조건문을 여러개 걸어서 타입별로 렌더링 되면 의도치 않는 버그가 발생하거나, 한 페이지에서의 수정이 다른 페이지 까지 영향을 줄 수 있다.
- 컴포넌트의 단위(atom, molecules, organisms)를 정의하기 위해 시간이 낭비된다. 이 부분은 사람마다 정하는 기준도 조금씩 다르고, 컴포넌트의 기능을 추가하거나 분리할때 단위를 바꿔야 하는 고민도 추가적으로 생성한다.
이런 경험 때문에, 재사용성 보다는 컴포넌트의 신뢰성을 더 중요하게 생각하게 되어 기본적인 요소를 제외하고는 모두 screen디렉토리 안에서 screen별로 관리하게 되었다.
이 패턴을 현재의 Todo List 프로젝트에 한번 적용해보자.
Screen
Todo List 과제의 요구 조건을 레이아웃의 관점으로 보면 다음과 같았다.
- 로그아웃 기능.
- Todo 입력 폼.
- Todo 목록 보여주기
- Todo 상세내용 보여주기
- Todo 수정기능
요구조건에 맞춰서 레이아웃을 크게 3부분으로 나눴다. 가장 위에 입력 폼을 배치하고, 왼쪽에는 목록, 오른쪽에는 상세내용 및 수정하는 부분을 배치했다. 이 구조에 맞춰서 한번 추상화 및 관심사 분리를 진행해 보았다.
const Container = styled.div`
// 생략
`;
function HomeScreen() {
return (
<Container>
<TitleHelmet title="To Do List" />
<ButtonLogOut />
<TodoWriteTemplate />
<div className="container-todo">
<TodoListTemplate />
<TodoDetailTemplate />
</div>
</Container>
);
}
export default HomeScreen;
우선 원래 나의 구조에 맞게 Screen은 Template을 렌더링하게 만들어 주었다. 이로써 Screen은 각각의 Template을 렌더링하는 기능에만 집중하게 되었고, 어떤 레이아웃으로 Template들을 렌더링하는지 볼 수 있게되었다. Screen의 목적이 분명 해지는 것이다. 만약 레이아웃간의 배치를 조정하고싶다면, Screen에서 CSS를 수정하거나, 컴포넌트들의 위치를 바꿔주면 된다.
이로써 Screen은 Template을 레이아웃에 맞춰 렌더링한다는 목적에만 집중 할 수 있게 되었고, 각 Template들은 추상화 되어 세부내용은 감춰지게 되어 유지보수가 편해졌다.
Template
import { useParams } from 'react-router-dom';
import styled from 'styled-components';
import TodoListCard from '../component/TodoListCard';
import useGetTodos from '../hooks/useGetTodos';
export const TodoListContainer = styled.section`
// 생략
`;
const ListContainer = styled.ul`
// 생략
`;
function TodoListTemplate() {
const { data: todosData, refetch: refetchTodos } = useGetTodos({ suspense: true });
const { id: todoId } = useParams();
return (
<TodoListContainer data-cy="container-todo-list">
{todosData?.length ? (
<ListContainer>
{todosData.map((todo) => (
<TodoListCard key={todo.id} {...todo} refetchTodos={refetchTodos} selected={todoId === todo.id} />
))}
</ListContainer>
) : (
<span>할 일이 아직 없어요..</span>
)}
</TodoListContainer>
);
}
export default TodoListTemplate;
Screen에서 Template을 분리해 냈으니, Template은 Template의 기능을 잘 수행하면 된다. TodoListTemplate은 독립적으로 비동기 요청을 서버로 보내고, 받은 데이터로 리스트를 렌더링 시킨다. 상위컴포넌트에 의존하지 않으므로, Template은 어느곳에서 렌더링 시켜도 동일한 기능을 수행할 것이다. Template에서는 component들을 조합하여 기능을 구현한다. Template에서 직접 작성한 TodoListContainer, ListContainer도 있다. (지금 보니 네이밍이 좀 어색한것같다.) 그리고 리스트를 보여주기 위해 추상화 한 TodoListCard라는 컴포넌트가 있다. 여러가지의 component들의 조합으로 template이 완성되었다.
Component
import React from 'react';
import { Link, useNavigate, useParams } from 'react-router-dom';
import dayjs from 'dayjs';
import styled from 'styled-components';
import { Todo } from '../../../api/Todos/types';
import { routes } from '../../routes';
import ButtonBasic from '../../../common/components/button/ButtonBasic';
import useDeleteTodo from '../hooks/useDeleteTodo';
const Container = styled.li`
// 생략
`;
interface TodoListCardProps extends Todo {
refetchTodos: () => void;
}
function TodoListCard({ id, title, createdAt, refetchTodos }: TodoListCardProps) {
const { mutate } = useDeleteTodo();
const { id: currentTodoId } = useParams();
const navigate = useNavigate();
const isCardSelected = () => {
return id === currentTodoId
}
const deleteTodo = (e: React.MouseEvent<HTMLElement>, willDeleteTodoId: string) => {
e.stopPropagation();
mutate(willDeleteTodoId, {
onSuccess: () => {
if (isCardSelected()) {
navigate('/', { replace: true });
}
refetchTodos();
},
});
};
return (
<Container data-cy="container-todo-card" className={`${isCardSelected() ? 'selected' : ''}`}>
<Link to={routes.home + id} data-cy="link-todo-detail">
<span data-cy="text-todo-list-title">{title}</span>
<span data-cy="text-todo-list-createdAt">{`${dayjs(createdAt).format('YYYY/MM/DD')}`}</span>
</Link>
<ButtonBasic title="X" type="button" data-cy="button-delete-todo" onClick={(e) => deleteTodo(e, id)} />
</Container>
);
}
export default TodoListCard;
TodoListCard 컴포넌트이다. TodoListCard가 해야 할 일은 다음과 같다.
- 컴포넌트가 클릭되었을때, Todo의 상세내용을 띄우게 하기
- Todo삭제.
- 현재 선택된 Todo라면 Highlight하기.
- 상세내용을 띄워주는 기능은 Route를 옮기는 방식으로 Link컴포넌트를 이용하여 구현했다. 두번째로 삭제기능은 TodoListCard컴포넌트에 custom hook형태로 api함수를 넣어주었다. 과거에는 컴포넌트 단위에 hooks를 사용하지 않고 부모 컴포넌트에서 관련 함수를 내려받게 작성했다. 어쩌면 과거의 방식이 view로직과 business로직이 분리되는 측면에서 더 좋다고 생각이 된다. 이부분은 더 고민을 해봐야겠지만, 나는 컴포넌트가 개별적으로 동작할 수 있게 만들어야 한다고 생각했다. 그리고 부모와의 의존성을 낮추고, 코드를 나눠서 작성함으로써 복잡도를 낮출 수 있다고 생각한다. 세번째로 Todo가 선택되었을때 Highlight하는 기능이다. 이 기능도 부모 컴포넌트와 의존하지 않기위해서 현재의 Route를 기준으로 선택되었는지 확인하는 isCardSelected 함수를 작성했다.
여기서 더 리팩토링을 할 수 있겠지만 컴포넌트에 관해서는 우선 이 정도로 마무리를 지어보려고 한다. 1회차 세션만으로도 많은 부분을 다시 고민하고 수정할 수 있었다. 디테일한 부분은 더 공부하면서 고쳐보려고 한다.
4. 비동기 처리를 더 가독성 좋게 작성하기.
세션중에 나온 내용이고, 관련 영상 링크를 첨부해주신 부분을 보고 리팩토링을 진행했다. 미리 이야기하자면 나는 비동기에 대한 처리를 하나도 하지 않았다. 그래서 새로운 마음으로 코드를 수정해보았다.
ErrorBoundary와 Suspense를 이용한 선언적 비동기 컴포넌트
function UserList() {
return (
<AsyncBoundary pendingFallback={<Loading />} rejectedFallback={<Error />}>
<UserDropDown />
</AsyncBoundary>
)
}
// Suspended Component
function UserDropDown() {
const { data: user } = useUser() // async call
return <div>{user!.name}</div>
}
https://jbee.io/react/error-declarative-handling-1/#usage
토스 세션에도 소개가 되었고, 블로그에도 작성된 내용이다. 내용의 핵심은 선언적으로 비동기 상태를 처리했다는 점이다. 과거의 경우, 한 컴포넌트 내에서 분기를 작성하여 Loading상태, Error상태를 처리해주었다. 이런 방법은 선언적이 아닌, 명령적으로 처리한 방법이다. 위 코드를 작성한 필자는 선언적으로 이 상황을 해결하기 위한 방법으로 ErrorBounday와 Suspense를 사용했다. 두가지의 기술로 상태의 격리를 실현할 수 있다. ErrorBounday는 Error가 발생했을때, Suspense는 하위컴포넌트가 로딩 중일 때 대체할 수 있는 ui를 대신 렌더링 시킬 수 있다. 작동의 원리는 try/catch와 비슷하다. 여려겹의 try/catch가 있다면, 에러가 발생했을때 해당 부분을 감싸고 있는 try/catch에서 에러를 처리하게 된다. 에러는 try/catch내에서 격리가 되는 것이다.
내가 적용한 코드를 살펴보자.
function HomeScreen() {
return (
<Container>
<TitleHelmet title="To Do List" />
<ButtonLogOut />
<TodoWriteTemplate />
<div className="container-todo">
<LoadingAndError loadingFallback={<TodoListLoading />} errorFallback={<TodoListError />}>
<TodoListTemplate />
</LoadingAndError>
<LoadingAndError loadingFallback={<TodoDetailLoading />} errorFallback={<TodoDetailError />}>
<TodoDetailTemplate />
</LoadingAndError>
</div>
</Container>
);
}
// 이부분이 핵심
<LoadingAndError loadingFallback={<Loading />} errorFallback={<Error />}>
이렇게 LoadingAndError라는 컴포넌트를 작성했고, loadingFallback, errorFallback 이라는 props에 FallbackUI를 전달하게 했다. 네이밍에 대해서는 고민이 좀 많았다. 위의 방법을 설명한 AsyncBoundary를 그대로 쓸까 고민도 했지만, LoadingAndError을 사용하면 조금 더 직관적이지 않을까? 라고 생각을 해서 작명을 해보았다.(이 부분은 커뮤니티에 물어보기로 함.)
function LoadingAndError({ errorFallback, loadingFallback, children }: LoadingAndErrorProps) {
return (
<ErrorBoundary onError={errorFallback}>
<Suspense fallback={loadingFallback}>{children}</Suspense>
</ErrorBoundary>
);
}
해당 컴포넌트는 이렇게 간단하게 구현되어있다.
마치며.
세션을 듣고, 하루종일 코드를 수정하면서 이것저것 고민을 해보았다. 그동안 나는 기능구현에 집중하고, 철저히 내 위주로 편하게 코드를 짰다. 그런 결과물이 세션을 듣고 난 뒤에는 너무 초라해보였다. 다행히도 리팩토링을 할 수록 코드의 품질이 높아지는 것 같은 기분이 들었다. 아직도 많이 부족하지만 전보다는 더 나아졌다고 확신할 수 있다. 이번에 배운 내용을 통해 고민을 하다보면 어느 순간 또 깨닳음을 얻고 다시 한번 성장할 수 있을 거라고 확신이 들었다.
프로젝트를 시작하면서부터 Cypress적용을 생각했다. 리팩토링이 빈번하게 일어나게 되면, 테스트하는데 시간을 많이 소모 할 것이라고 생각했다. 그래서 처음부터 테스트 자동화를 통해 시간 단축을 노려봤다. 결과는 만족스러웠다. 많은 로직을 수정하고 난 뒤에, 테스트를 돌리면 어느 부분에서 새로 버그가 생겼는지도 보였기 때문이다.
아직 선언전 프로그래밍 방법에 대해 이해가 많이 부족한 것 같다. 어떻게 동작하는지 코드를 이해해야 알수 있는것이 아닌, 코드 자체로 어떤 동작을 수행하는지 명확하게 이해할 수 있어야 한다는 점이 중요 한 것 같다. 이 부분은 더 공부를 해나가면서 보완을 해야겠다.
'원티드 프리온보딩 챌린지' 카테고리의 다른 글
| 원티드 프리온보딩 챌린지 2-1회차 (0) | 2022.08.19 |
|---|---|
| 원티드 프리온보딩 챌린지 1-2회차 (0) | 2022.08.16 |
