📘

PR提出前にやるリファクタリング

に公開
JavaScript
TypeScript
初学者向け
リファクタリング
tech

PR出す前に最低限やって欲しいと思っているリファクタリングメモ

リファクタリング (refactoring) とは、コンピュータプログラミングにおいて、プログラムの外部から見た動作を変えずにソースコードの内部構造を整理することである。
https://ja.wikipedia.org/wiki/リファクタリング_(プログラミング)

想定

  • 期待の新人君
  • JavaScript / TypeScript
  • フロントエンド

嗅覚を研ぎ澄ませる🐶

次の匂いがしたらリファクタリングを検討してみる

  • 不可思議な名前
  • 重複したコード
  • 長い関数
  • 長いパラメータリスト
  • 複雑なループ
  • 甘えたコメント
  • 眉間に皺がよっている

関数の抽出

関数化することで処理に名前を与え意図を明確にしよう（どうやるかではなく、なにをするかという命名が大事）

例）1. APIリクエスト

before

const token = "awesometoken"
fetch("/api/users", {
  headers: { Authorization: `Bearer ${token}` },
})
  .then((response) => response.json())
  .then((data) => console.log(data));

fetch("/api/posts", {
  headers: { Authorization: `Bearer ${token}` },
})
  .then((response) => response.json())
  .then((data) => console.log(data));

after

const fetchData = (url) => {
  const token = "awesometoken"
  return fetch(url, {
    headers: { Authorization: `Bearer ${token}` },
  })
    .then((response) => response.json())
    .then((data) => console.log(data));
};
fetchData("/api/users");
fetchData("/api/posts");

メリット

  • コードの重複が排除されてメンテナンス性が向上した
  • 共通処理を一箇所で管理できるため、変更時の影響範囲を最小限に抑制できた

例）2. バリデーションロジック

before

if (!user.email || user.email.indexOf("@") === -1) {
  alert("無効なメールアドレスです");
}
if (!user.password || user.password.length < 8) {
  alert("パスワードは8文字以上である必要があります");
}

after

const isInvalidEmail = (email) => !email || email.indexOf("@") === -1;
const isInvalidPassword = (password) => !password || password.length < 8;
const { email, password } = user;

if (isInvalidEmail(email)) {
  alert("無効なメールアドレスです");
}
if (isInvalidPassword(password)) {
  alert("パスワードは8文字以上である必要があります");
}

メリット

  • バリデーション条件が関数名として表現されて意図が明確になった
  • バリデーションロジックの再利用性が向上した
  • テストし易くなった

例）3. DOM操作の共通化

before

const addUserCard = (container, user) => {
  const card = document.createElement("div");
  
  const avatar = document.createElement("img");
  avatar.src = user.avatarUrl;
  
  const name = document.createElement("h3");
  name.textContent = user.name;
  
  const email = document.createElement("p");
  email.textContent = user.email;
  
  card.appendChild(avatar);
  card.appendChild(name);
  card.appendChild(email);
  container.appendChild(card);
};

after

const createUserCard = (user) => {
  const card = document.createElement("div");
  
  const avatar = document.createElement("img");
  avatar.src = user.avatarUrl;
  
  const name = document.createElement("h3");
  name.textContent = user.name;
  
  const email = document.createElement("p");
  email.textContent = user.email;
  
  card.appendChild(avatar);
  card.appendChild(name);
  card.appendChild(email);
  
  return card;
};

const addUserCard = (container, user) => {
  const card = createUserCard(user);
  container.appendChild(card);
};

メリット

  • カード作成と追加の責務が分離された（単一責任の原則）
  • 再利用性が向上した
  • テストし易くなった

変数の抽出

値の意味を明確にしよう

例）1. 文字列の繰り返し利用

before

fetch("https://api.example.com/v1/users");
fetch("https://api.example.com/v1/posts");

after

const BASE_URL = "https://api.example.com/v1/";
fetch(`${BASE_URL}users`);
fetch(`${BASE_URL}posts`);

メリット

  • 変更時の影響範囲が最小限になった
  • タイポの発生が抑制された

例）2. 意味のわかりにくいリテラル

before

setTimeout(doSomething, 86400000);

after

const ONE_DAY = 1000 * 60 * 60 * 24;
setTimeout(sendReminder, ONE_DAY);

メリット

  • マジックナンバーを意味のある定数名に置き換えたことで意図が明確になった

条件分岐の分解

複雑な条件文に意味を与えよう

例）1. 複雑なif条件

before

if (customer.age > 65 && customer.isMember && !customer.hasUnpaidFees) {
  applySeniorDiscount();
}

after

const isTargetForSeniorDiscount = (customer) =>
  customer.age > 65 && customer.isMember && !customer.hasUnpaidFees;

if (isTargetForSeniorDiscount(customer)) {
  applySeniorDiscount();
}

メリット

  • 複雑な条件を意味のある関数名に置き換えたことで意図が明確になった
  • テストし易くなった

条件分岐の結合

コードをシンプルに保とう

例）1. 複数の条件で同じ関数を呼び出す

before

if (user.role === 'admin') {
  grantAccess();
} else if (user.role === 'manager') {
  grantAccess();
}

after

const isAdmin = (role) => ADMIN_LIST.includes(role)
if (isAdmin(user.role)) grantAccess();

メリット

  • 条件分岐が減少してコードの簡潔性が向上した
  • テストし易くなった

ガード節による入れ子の条件記述の置き換え

深いネストはやめよう

例）1. ガード節で早期リターン

before

const process = (user) => {
  if (user) {
    if (user.isActive) {
      if (user.hasPermission) {
        doSomething();
      } else {
        console.log("権限がありません");
      }
    } else {
      console.log("ユーザーは無効です");
    }
  } else {
    console.log("ユーザーが存在しません");
  }
};

after

const process = (user) => {
  if (!user) {
    console.log("ユーザーが存在しません");
    return;
  }

  if (!user.isActive) {
    console.log("ユーザーは無効です");
    return;
  }

  if (!user.hasPermission) {
    console.log("権限がありません");
    return;
  }

  doSomething();
};

メリット

  • ネストが浅くなりコードの可読性が大幅に向上した
  • 早期リターンによりロジックの流れが明確になった

パラメータオブジェクトの導入

データ構造を明確にしよう

例）1. パラメータオブジェクトでまとめる

before

const calculateWorkingDays = (startYear, startMonth, endYear, endMonth) => {
  // 勤務期間の日数計算の処理
  const startDate = new Date(startYear, startMonth - 1);
  const endDate = new Date(endYear, endMonth - 1);
  const diffTime = endDate - startDate;
  const diffDays = diffTime / (1000 * 60 * 60 * 24);
  return diffDays;
};
const days = calculateWorkingDays(2022, 4, 2023, 3);

after

const calculateWorkingDays = (period) => {
  const { startYear, startMonth, endYear, endMonth } = period;
  const startDate = new Date(startYear, startMonth - 1);
  const endDate = new Date(endYear, endMonth - 1);
  const diffTime = endDate - startDate;
  const diffDays = diffTime / (1000 * 60 * 60 * 24);
  return diffDays;
};

const workingPeriod = {
  startYear: 2022,
  startMonth: 4,
  endYear: 2023,
  endMonth: 3,
};
const days = calculateWorkingDays(workingPeriod);

メリット

  • 関連するパラメータをグループ化し、データ構造が明確になった
  • 構造化されたことで、パラメータの追加・削除が容易になり拡張性が向上した
  • 関数の呼び出しが読みやすくなり可読性が向上した

例）2. Propsをパラメータオブジェクトで渡す

before

const ChildComponent = ({ 
  employeeName, department, position,
  salary, bonus, workingHours,
  location, floor, roomNumber
}) => {
  return (
    <div>
      <h2>社員情報</h2>
      <p>名前: {employeeName}</p>
      <p>部署: {department}</p>
      <p>役職: {position}</p>
      
      <h3>給与情報</h3>
      <p>基本給: {salary}円</p>
      <p>ボーナス: {bonus}円</p>
      <p>勤務時間: {workingHours}時間/日</p>
      
      <h3>勤務場所</h3>
      <p>オフィス: {location}</p>
      <p>フロア: {floor}</p>
      <p>部屋番号: {roomNumber}</p>
    </div>
  );
};

const ParentComponent = () => {
  return (
    <ChildComponent
      employeeName="山田太郎"
      department="開発部"
      position="シニアエンジニア"
      salary={500000}
      bonus={1000000}
      workingHours={8}
      location="東京本社"
      floor={5}
      roomNumber="501"
    />
  );
};

after

const ChildComponent = ({ employeeInfo }) => {
  const { 
    basicInfo,
    salaryInfo,
    locationInfo
  } = employeeInfo;

  return (
    <div>
      <h2>社員情報</h2>
      <p>名前: {basicInfo.name}</p>
      <p>部署: {basicInfo.department}</p>
      <p>役職: {basicInfo.position}</p>
      
      <h3>給与情報</h3>
      <p>基本給: {salaryInfo.base}円</p>
      <p>ボーナス: {salaryInfo.bonus}円</p>
      <p>勤務時間: {salaryInfo.workingHours}時間/日</p>
      
      <h3>勤務場所</h3>
      <p>オフィス: {locationInfo.office}</p>
      <p>フロア: {locationInfo.floor}</p>
      <p>部屋番号: {locationInfo.roomNumber}</p>
    </div>
  );
};

const ParentComponent = () => {
  const employeeInfo = {
    basicInfo: {
      name: "山田太郎",
      department: "開発部",
      position: "シニアエンジニア"
    },
    salaryInfo: {
      base: 500000,
      bonus: 1000000,
      workingHours: 8
    },
    locationInfo: {
      office: "東京本社",
      floor: 5,
      roomNumber: "501"
    }
  };

  return <ChildComponent employeeInfo={employeeInfo} />;
};

メリット

  • データをグループに分類されたことで構造が一目で理解可能になった
  • 型定義が容易になり、TypeScriptとの相性🙆

パイプラインによるループの置き換え

宣言的なコードで簡潔性を保とう

例）1. forループで配列を加工する

before

const numbers = [1, 2, 3, 4, 5];
const doubledEvens = [];

// 偶数のみを2倍にして配列に追加
for (let i = 0; i < numbers.length; i++) {
  if (numbers[i] % 2 === 0) {
    doubledEvens.push(numbers[i] * 2);
  }
}
console.log(doubledEvens); // [4, 8]

after

const numbers = [1, 2, 3, 4, 5];
const isEven = (number) => number % 2 === 0;
const double = (number) => number * 2;

const doubledEvens = numbers
  .filter(isEven)
  .map(double);
console.log(doubledEvens); // [4, 8]

メリット

  • 宣言的なコードになったことで処理の意図が明確になった
  • テストし易くなった

例）2. forループで合計値を計算する

before

const transactions = [
  { type: "income", amount: 1000 },
  { type: "expense", amount: 200 },
  { type: "income", amount: 500 },
];

// 収入の合計を計算
let incomeTotal = 0;
for (let i = 0; i < transactions.length; i++) {
  if (transactions[i].type === "income") {
    incomeTotal += transactions[i].amount;
  }
}
console.log(incomeTotal); // 1500

after

const transactions = [
  { type: 'income', amount: 1000 },
  { type: 'expense', amount: 200 },
  { type: 'income', amount: 500 },
];

const incomeTotal = transactions
  .filter(t => t.type === 'income')
  .reduce((sum, t) => sum + t.amount, 0);

console.log(incomeTotal); // 1500

メリット

  • 複雑なループ処理が簡潔な関数チェーンに変換され可読性が向上した
  • 処理の各ステップが明確になりデバッグが容易になった
  • コードがシンプルに保たれている

参考

Discussion