Front-end

Good Refactoring vs Bad Refactoring

ghDev 2024. 9. 9. 10:40



이번글은리팩토링을 주제로한 글로 해외 게시글을 토대로 좋은 리팩토링과 나쁜 리팩토링의 예를 구분하여 정리합니다.

이 글의 작성자는 수년간 많은 개발자를 고용해 왔고 그중 상당수는 우리의 코드는 대대적인 리팩토링이 필요하다는 확고한 신념을 가지고있었다 말합니다. 하지만 많은 경우에서 리팩토링된 코드를 이전보다 이해하고 유지보수하기 더 어렵다고 느꼈고 더 느리고 버그가 많기도 했다고 합니다.

리팩토링 자체는 코드 베이스를 견고하게 유지하는데 중요한 과정이지만 더 나은 것을 만들려다 오히려 상황을 더 악화시키는 함정에 빠지기 쉽게 한다고 합니다.

그렇다면 좋은 리팩토링과 나쁜 리팩토링의 차이를 알아보겠습니다.


1. 코드 스타일을 크게 바꾸는것


 

before

// 🫤 this code could be cleaner
function processUsers(users: User[]) {
  const result = [];
  for (let i = 0; i < users.length; i++) {
    if (users[i].age >= 18) {
      const formattedUser = {
        name: users[i].name.toUpperCase(),
        age: users[i].age,
        isAdult: true,
      };
      result.push(formattedUser);
    }
  }
  return result;
}

 

bad refactor

import * as R from 'ramda';

// 🚩 adopted a completely different style + library
const processUsers = R.pipe(
const processUsers = R.pipe(
  R.filter(R.propSatisfies(R.gte(R.__, 18), 'age')),
  R.map(
    R.applySpec({
      name: R.pipe(R.prop('name'), R.toUpper),
      age: R.prop('age'),
      isAdult: R.always(true),
    })
  )
);

 

이 리팩토링의 문제점은 새로운 라이브러리(Ramda)를 적용하고 코딩 스타일이 완전히 달라진점 입니다. 이 방식은 익숙하지 않은 팀에게는 오히려 가독성과 유지보수성을 저하 할 수 있습니다.

good refactor

// ✅ cleaner and more conventional 
function processUsers(users: User[]): FormattedUser[] {
  return users
    .filter(user => user.age >= 18)
    .map(user => ({
      name: user.name.toUpperCase(),
      age: user.age,
      isAdult: true
    }));
}

 

이 방식은 보편적인 자바스크립트 메서드를 사용해서 기존 코드를 개선했습니다. 더 읽기 쉬워졌지만, 코드 스타일이 크게 바뀌웠다던지 외부 라이브러리를 사용하지 않았습니다.

2. 불필요한 추상화


이 글의 작성자는 기본 코드를 이해하지 못한채 새로운 추상화들을 추가한 사람을 고용한 적이 있었고 함께 묶어서는 안되는 것들을 그룹화하고 점차 코드의 일부를 서로 다른 방향으로 분리하려 했다고 합니다. 또한 서로 다른 API에는 각각 다른 설정이 필요했는데, 일부 설정들을 잘못 통합해 버렸다고 합니다.

before

// 🫤 this code could be cleaner
function processUsers(users: User[]) {
  const result = [];
  for (let i = 0; i < users.length; i++) {
    if (users[i].age >= 18) {
      const formattedUser = {
        name: users[i].name.toUpperCase(),
        age: users[i].age,
        isAdult: true
      };
      result.push(formattedUser);
    }
  }
  return result;
}

 

Bad refactor

// 🚩 there are way more layers and abstractions here than necessary
class UserProcessor {
  private users: User[];

  constructor(users: User[]) {
    this.users = users;
  }

  public process(): FormattedUser[] {
    return this.filterAdults().formatUsers();
  }

  private filterAdults(): UserProcessor {
    this.users = this.users.filter(user => user.age >= 18);
    return this;
  }

  private formatUsers(): FormattedUser[] {
    return this.users.map(user => ({
      name: this.formatName(user.name),
      age: user.age,
      isAdult: true
    }));
  }

  private formatName(name: string): string {
    return name.toUpperCase();
  }
}

const processUsers = (users: User[]): FormattedUser[] => {
  return new UserProcessor(users).process();
};

 

이 리팩토링은 여러 메서드를 가진 클래스를 도입하는데, 이는 더 OOP(객체지향 프로그래밍)처럼 보일수 있지만, 실제로는 더 복잡해지고 한눈에 이해하기 어려워졌습니다.

Good refactor:

// ✅ cleaner and more conventional 
const isAdult = (user: User): boolean => user.age >= 18;

const formatUser = (user: User): FormattedUser => ({
  name: user.name.toUpperCase(),
  age: user.age,
  isAdult: true
});

function processUsers(users: User[]): FormattedUser[] {
  return users.filter(isAdult).map(formatUser);
}


이 방식은 불필요한 메서드가 들어가지 않고, 로직을 재사용할 수 있는 작은 함수로 분리한 방법입니다.

 

3. 일관성 부족


작성자는 개발자들이 코드 베이스를 개선이라는 명목하에 바꾸려고 시도하다가 기존의 코드 베이스와는 완전히 다르게 동작하게 만든 사례를 본적이 있다고 합니다. 이는 개발자가 서로 다른 스타일 간에 컨텍스트를 전환해야 하는 다른 개발자에게 혼란과 좌절감을 줄수 있습니다.


예를 들어, 리액트 쿼리를 사용하여 데이터 페칭을 처리하는 리액트 앱이 있다고 가정하고

// Throughout the app
import { useQuery } from 'react-query';

function UserProfile({ userId }) {
  const { data: user, isLoading } = useQuery(['user', userId], fetchUser);

  if (isLoading) return <div>Loading...</div>;
  return <div>{user.name}</div>;
}


한 개발자가 특정 하나의 컴포넌트에서만 리덕스 툴킷을 사용한다고 가정을 해봅니다.

// One-off component
import { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { fetchPosts } from './postsSlice';

function PostList() {
  const dispatch = useDispatch();
  const { posts, status } = useSelector((state) => state.posts);

  useEffect(() => {
    dispatch(fetchPosts());
  }, [dispatch]);

  if (status === 'loading') return <div>Loading...</div>;
  return <div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>;
}


이런 불일치는 단 하나의 구성 요소에 대해 완전히 다른 상태 관리 패턴을 도입하기 때문에 좋지 않습니다.
더 나은 접근은 리액트 쿼리를 계속 사용하는 것 입니다.

// Consistent approach
import { useQuery } from 'react-query';

function PostList() {
  const { data: posts, isLoading } = useQuery('posts', fetchPosts);

  if (isLoading) return <div>Loading...</div>;
  return <div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>;
}

 

이 버전은 앱 전체에서 데이터 페칭에 리액트 쿼리를 사용하여 일관성을 유지할 수 있다. 더 심플하고, 다른 개발자들이 한 컴포넌트를 위해

새로운 패턴을 익힐 필요가 없습니다.
또한 코드 베이스의 일관성을 유지할 수 있다는 중요한 점이 있습니다. 새로운 패턴을 도입해야 하는 경우 일회성 불일치를 만들기보단 먼저 팀원들의 동의를 얻는 방법을 고려해야 합니다.

4. 비즈니스 맥락 이해


작성자는 끔찍한 레거시 코드가 가득한 회사에 입사한 적이 있었습니다. Angular.js를 사용해서 이커머스 회사를 더 빠르고, 현대적이면서, 더 나은 기술로 마이그레이션 하는 프로젝트를 이끌었습니다.

그런데 알고보니 이 비즈니스는 SEO에 크게 의존하고 있었고 2년동안 해당 팀이 만든 것은 기존 웹사이트를 더 느리고 버그가 많으며 유지보수하기 어려운 복제품뿐이였습니다.
왜 이런일이 벌어졌을까요? 이 프로젝트를 주도한 사람들(작성자를 포함해서)이 이 사이트에서 일해본 적이 없었기 때문입니다.

Bad refactor:

// 🚩 a single page app for an SEO-focused site is a bad idea
function App() {
  return (
    <Router>
      <Switch>
        <Route path="/product/:id" component={ProductDetails} />
      </Switch>
    </Router>
  );
}

 

이 방식은 CSR 방식으로 현대적이고 깔끔해 보일 수 있지만, SEO에 크게 의존하는 이커머스 사이트에는 재앙이 될 수 있습니다.

Good refactor:

// ✅ server render an SEO-focused site
export const getStaticProps: GetStaticProps = async () => {
  const products = await getProducts();
  return { props: { products } };
};

export default function ProductList({ products }) {
  return (
    <div>
      ...
    </div>
  );
}

 

Next.js 기반 접근 방식은 SSR을 기본적으로 제공하며, 이는 SEO에 매우 중요합니다. 또한 초기 페이지 로드를 더 빠르게 합니다.

올바른 리팩토링 방법


코드 리팩토링은 분명 필요하지만 제대로 해야 합니다. 우리의 코드는 완벽하지 않고 정리가 필요하지만, 코드 베이스와 일관성을 유지하고 코드를 잘 이해하며 추상화에 신중을 가해야 합니다.

 

성공적인 리팩토링을 위한 몇가지 팁은

  1. 점진적 작업. 대대적인 수정보다는 관리가 가능하도록 변경을 작게 만듭니다.
  2. 중요한 리팩토링이나 새로운 추상화를 도입하기 전에 깊은 코드이해가 필요합니다.
  3. 기존의 코드 스타일 유지. 일관성은 유지보수의 핵심입니다.
  4. 너무 많은 새로운 추상화 피하기. 복잡성이 정말 필요한 경우가 아니면 간단하게 유지합니다.
  5. 새로운 라이브러리는 팀의 동의 없이 추가하지 않습니다.
  6. 테스트를 작성하고 진행하면서 리팩토링하기. 이를 통해 원래 기능이 유지되는지 확인할 수 있습니다.
  7. 이러한 원칙을 동료들과 함께 준수하기

 

결론


리팩토링은 개발의 필수적인 부분이지만, 기존 코드 베이스와 팀의 역할을 중심으로 신중하게 이루어져야 합니다. 리팩토링의 목표는 외부 동작을 변경하지 않으면서 코드의 내부 구조를 개선하는것입니다.

 

리팩토링의 최종 목표는 외부 사용자에게는 눈에 띄지 않지만, 개발자의 작업을 훨씬 쉽게 만드는 것으로 가독성, 유지보수성, 효율성을 향상하면서 전체 시스템을 방해하지 않는것 입니다.

이 글을 통해 리팩토링시에 어떤 방식으로 접근해야하는지에 대해 알수 있었습니다. 짧은 코드만이 능사가 아니고 항상 일관성을 유지하면서 다른사람들이 쉽게 이해할수 있는 코드를 작성하려 더 생각할 수 있는 계기가 됐습니다.


출처: https://www.builder.io/blog/good-vs-bad-refactoring