1. 적절히 추상화 되지 않은 함수

(1) API 호출 로직 모듈화 하기

- 문제점

Todo의 CRUD와 Auth의 Login/SignUp API를 호출하는 비동기 로직을 함수로 따로 뺐는데, try ~ catch 구문이 불필요하게 계속 반복되고 있었다. 하나의 수정 사항이 생기면 같은 로직을 사용하고 있는 모든 코드를 일일히 수정해야 하는 번거로움이 있다.

/*---------- authService.ts -------------*/
//로그인
export const callLoginApi = async(params: User) => {
    try {
        const res = await axios.post(`${BASE_URL}/login`, params)
        if (res.status === 200) {
            return res
        }
    } catch (error: any) {
        alert(error.response.data.details)
        throw Error(error.response.data.details)
    }
}

//회원가입
export const callSignUpApi = async(params: User) => {
    try {
        const res = await axios.post(`${BASE_URL}/create`, params)
        if (res.status === 200) {
            return res
        }
    } catch(error: any) {
        alert(error.response.data.details)
        throw Error(error.response.data.details)
    }
}

- doAxios로 모듈화

공통 로직을 줄이고 타이핑하는 코드의 양을 줄이기 위해 doAxios 모듈을 만들어서 추상화했다. token을 받는 방식이나 header의 변경이 있어도 관련 코드를 모두 수정하지 않고도 변경 사항을 적용할 수 있다.

/*---------- doAxios.ts -------------*/
import { User } from '../types/auth'
import { Todo, TodoInput } from './../../../server/types/todos';
import axios from 'axios'
const token = localStorage.getItem("token") as string;

interface Request{
    method: string,
    url: string
    data?: Todo | TodoInput | User,
}

const instance = axios.create({
    baseURL: 'http://localhost:8080',
    headers : { Authorization: token }
})

export const doAxios = async(request: Request) =>{
    try{
        const response = await instance(request)
        if(response.status===200) return response
    } catch (error: any){
        alert(error.response.data.details)
        throw Error(error.response.data.details)
    }
}

- 추상화된 코드

/*---------- authService.ts -------------*/
//로그인
export const callLoginApi = async (data: User) => {
  const response = await doAxios({
    method: "post",
    url: `${BASE_URL}/login`,
    data,
  });
  return response;
};

//회원가입
export const callSignUpApi = async (data: User) => {
  const response = await doAxios({
    method: "post",
    url: `${BASE_URL}/create`,
    data,
  });
  return response;
};

---

2. 관심사의 분리

(1) 컴포넌트의 분리

- 문제점

아래 사진은 초반의 폴더 구조다. views에 HTML 뼈대와 컴포넌트를 조합해서 사용자에게 뷰를 보여주는 컨테이너가 들어있고, 조합될 컴포넌트는 compoents 폴더에 있다. router 설정은 App.ts에서 설정했다.

문제는 비즈니스 로직과 뷰가 분리되어 있지 않고 뷰와 컴포넌트에 혼재되어 있어 관련 기능을 수정하려면 관련된 뷰와 컴포넌트를 일일히 찾아가 수정해야 된다는 점이었다.
특히 Login과 SignUp은 같은 Auth 컴포넌트를 사용하고, 유사한 기능을 하는 함수와 동일한 형태의 user state를 쓰고 있어 코드가 반복되고 있었다. (Auth 컴포넌트로 수많은 props를 내려보내는 것은 덤이다.)

Login.tsx	SignUp.tsx

- 중복 코드의 최소화와 뷰의 분리

중복되는 코드와 로직들을 Auth 컴포넌트로 모두 넘겨버려 통일시키고 url을 받아 로그인 화면인지 회원가입 화면인지를 구분해서 부분적으로만 화면이 바뀔 수 있도록 구현했다. Login과 SignUp 컨테이너에는 정말 View의 역할만 할 수 있게 Auth 컴포넌트를 렌더링 하는 부분만 남겼다. 또 Login, SignUp, TodoList는 동일한 형태의 헤더를 사용하고 있는데, 헤더 또한 컴포넌트로 따로 빼주었다.

/*---------- Login.ts -------------*/
function Login() {
  return <Auth title="Login" />;
}

/*---------- SignUp.ts -------------*/
function SignUp() {
  return <Auth title="SignUp" />;
}

/*---------- Auth.ts -------------*/

function Auth({ title }: Props) {
  const navigate = useNavigate();
  const [user, setUser] = useState<User>({ email: "", password: "" });
  const { pathname } = useLocation();

  //인풋 이벤트
  const onInputChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
    setUser({ ...user, [e.target.id]: e.target.value });
  };

  //로그인 버튼 클릭
  const onLoginClick = async () => {
    const response = await callLoginApi(user);
    localStorage.setItem("token", response?.data.token);
    navigate("/");
  };

  // 회원가입 페이지로 이동
  const onSignUpClick = () => {
    navigate("/sign_up");
  };

  // 회원가입하기
  const onSignUpSubmit = async () => {
    const response = await callSignUpApi(user);
    alert(response?.data.message);
    navigate("/login");
  };

  //토큰이 있는 경우 Login 또는 SignUp 페이지 접근 시 리디렉션
  useEffect(() => {
    const USER_TOKEN: string | null = localStorage.getItem("token");
    if (USER_TOKEN) {
      navigate("/");
    }
  });
  
  {
    return(//생략)
  }

- 폴더 구조 리팩토링

컴포넌트를 잘게 쪼개고 로직과 뷰를 분리하다보니 폴더 구조도 아래와 같이 바뀌었다. 어디서 뭘 하는지 좀 더 직관적으로 알 수 있다.

---

3. 지저분한 코드와 맥락을 이해하기 힘든 변수명

(1) 맥락을 이해하기 힘든 변수명

How가 아닌 What으로 이 함수가 의도하는 게 뭔지 함수명에 드러나야 한다. 나는 '어떤'걸 할 건지는 써있는데, '대상'이 적혀있지 않은 경우가 많았다. 그래서 관련된 문제를 가지고 있는 변수명을 모두 고쳤다.

/*---------- TodoTitle.ts -------------*/
  const onClickDelete = async (e: React.MouseEvent<HTMLButtonElement>) => {
    ....
  };

/*---------- TodoTitle.ts -------------*/
  const onDeleteTodoClick = async (e: React.MouseEvent<HTMLButtonElement>) => {
    ....
  };

(2) 필요 없는 코드와 복잡한 전개

- 문제점

가독성 있는 코드를 만들려고 수정할 부분이 없나 살펴보고 있는데 이게 대체 뭔가 하는 부분이 있었다. 투두 삭제, 수정 관련된 부분이었는데 투두를 클릭하면 해당 투두의 id를 가져와서 삭제/수정 API로 전달하고 state에도 변화를 주는 코드였다. 문제는 todoId를 쓸데없는 방식으로 가져오고 있다는 점이었다. 이러한 코드가 곳곳에 존재했다.

/*---------- TodoTitle.ts -------------*/
  const onTodoDeleteClick = async (e: React.MouseEvent<HTMLButtonElement>) => {
    e.stopPropagation();
    const clickedId: string = e.currentTarget.closest("li")!.id;
    callDeleteTodoApi(clickedId);
    if (todoId?.includes(clickedId)) {
      navigate("/");
    }
    handleDeleteTodo(index);
  };

- 이해할 수 있는 흐름의 코드

투두를 감싸고 있는 li의 id값을 가져오는 것 같은데 왜 굳이 이렇게 했나 싶다. 부모 컴포넌트에서 todo의 id를 props로 내려보냈다. 코드 길이도 짧아지고 가독성도 좋아졌다. 비슷한 맥락의 문제를 가지고 있는 코드도 모두 수정했다.

/*---------- TodoTitle.ts -------------*/
  // 투두 삭제 버튼 클릭
  const onTodoDeleteClick = async() => {
    callDeleteTodoApi(todo.id);
    handleDeleteTodo(index)
    currentUrl?.includes(todo.id) && navigate("/");
  };