🙄

初学者向けリファクタリング術

に公開

はじめに

最近、過去に自分が書いたコードを読む機会がありました。
見返してみると「読みにくい」「拡張性がない」と感じる箇所が多く、改善の余地が大きいコードでした。
裏を返せば、そう感じられるのは以前より視点が変わってきた証拠かもしれません。

振り返ってみると、その差を生んでいる大きな要因はリファクタリングの差だと思いました。
そこで今回は、エンジニアとして約1年半の経験を通して培ったすぐに実践できそうなリファクタリング術を共有します。

リファクタリングとは?

機能は変えずに、コードをより良くする作業を指します
新しく機能を追加する、バグを修正するのではなく既存のコードを「読みやすく・保守しやすく・変更しやすく」する作業です。

よくあるリファクタリングが必要になる瞬間

実務上では、以下が発生したときにリファクタリング作業に取り組む事が多いです。

  1. 条件分岐が複雑になりすぎたとき
    • ネストが深すぎて、どの条件でどの処理が走るのか追うのが大変になる
  2. 関数やクラスが肥大化したとき
    • 1つの関数がバリデーション・DB処理など複数の責務を持ってしまう
  3. 名前が曖昧で意図が伝わらないとき
    • fn, array, dataのような抽象的すぎる名前が使われていて、後から読むと何をしているのか分からない
  4. 同じ処理を使い回しているとき
    • 仕様変更が入ったとき、全箇所を修正しなければならず、バグが発生しやすくなる

リファクタリング例

ネストが深い条件分岐を“早期return&ガード節”に

Before

function canDownload(user?: User | null) {
  if (user) {
    if (user.isActive) {
      if (user.role === "admin" || user.role === "manager") {
        return true;
      } else {
        return false;
      }
    }
  }
  return false;
}

After

function canDownload(user?: User | null): boolean {
  if (!user?.isActive) return false;
  const allowedRoles = new Set<Role>(["admin", "manager"]);
  return allowedRoles.has(user.role);
}

ポイント

  • ネスト解消で読みやすさUP
  • Setで“許可ロール”を宣言的に表現

巨大関数の分割

Before

type CreateUserInput = { email: string; name: string };
export async function createUser(input: CreateUserInput) {
  await syncUser();

  // バリデーション
  if (!input || typeof input.email !== "string" || typeof input.name !== "string") {
    throw new Error("invalid");
  }
  const email = input.email.trim();
  const name  = input.name.trim();
  if (!email.includes("@")) throw new Error("invalid email");
  if (!name) throw new Error("name required");

  // DB処理(重複チェック&保存)
  const exists = await User.findOne({ where: { email } });
  if (exists) throw new Error("already exists");

  const user = await User.create({ email, name });
  return user.toJSON();
}

After

import { User } from './db/models/User'; // Sequelizeで定義したUserモデル
type CreateUserInput = { email: string; name: string };

// バリデーションだけを担当
function validateCreateUserInput(input: CreateUserInput): CreateUserInput {
  if (
    !input ||
    typeof input.email !== 'string' ||
    typeof input.name !== 'string'
  ) {
    throw new Error('invalid');
  }
  const email = input.email.trim();
  const name = input.name.trim();
  if (!email.includes('@')) throw new Error('invalid email');
  if (!name) throw new Error('name required');
  return { email, name };
}

// 重複チェック
function findUserByEmail(email: string) {
  return User.findOne({ where: { email } });
}

// DB処理だけを担当
async function insertUser(data: CreateUserInput) {
  try {
    const user = await User.create(data);
    return user.toJSON(); // { id, email, name }
  } catch (e) {
    if (e?.name === 'SequelizeUniqueConstraintError') {
      throw new Error('already exists');
    }
    throw e;
  }
}

// ユースケース本体
async function createUser(input: CreateUserInput) {
  // デモ用の同期処理
  await syncUser();
  // バリデーション
  const data = validateCreateUserInput(input);
  // 重複チェック
  if (await findUserByEmail(data.email)) {
    throw new Error('already exists');
  }
  return insertUser(data);
}

ポイント

  • 関数名で処理意図が明確になり、テスト単位が切りやすい。

さいごに

今回は、自分が実務の中で学んだリファクタリングについてまとめてみました。
リファクタリングは難しいものではなく、日々の小さな改善の積み重ねだと感じています。
「昨日の自分よりも今日の自分が読みやすい」と思えるコードを目指して、少しずつ取り入れていければと思います!

株式会社ソニックムーブ

Discussion