現役エンジニアがTypeScriptのクソコードをリファクタリング

公開日 2023年7月28日(金)

環境

macOS Monterey 12.6
Windows 11
VSCode
node.js v18.6.0
npm 8.13.2
Typescript 4.6.4

今回のクソコードとリファクタリング方針

動画でもご覧いただけます

関数、関数の引数名がわかりにくいのでわかりやすい名称にする
関数の引数の型がstring型で少し広すぎるので文字列リテラルのユニオンを使って限定する
if文の条件分岐が激しいのでMapを使ってもう少し可読性を向上させる
1. 動物の種類とそれに対応する鳴き声をマッピングさせる関数
letを無くす
関数の責務を分割する
1. 鳴き声に長音が含まれるかどうかを返す関数
2. 動物の種類に応じて鳴き声を表す文字列を返す関数

console.clear();

const result = longSound("dog");
console.log(result);

// 指定した動物の鳴き声に長音が含まれるかどうかを返します
function longSound(hoge: string): boolean {
let longSound = false;

if (hoge === "cat") {
  longSound = "にゃーにゃー".includes("ー");
} else if (hoge === "dog") {
  longSound = "わんわん".includes("ー");
} else if (hoge === "elephant") {
  longSound = "ぱおーん".includes("ー");
} else if (hoge === "cow") {
  longSound = "もーもー".includes("ー");
} else if (hoge === "duck") {
  longSound = "がーがー".includes("ー");
} else if (hoge === "lion") {
  longSound = "がおーがおー".includes("ー");
} else if (hoge === "horse") {
  longSound = "ひひーん".includes("ー");
} else if (hoge === "pig") {
  longSound = "ぶーぶー".includes("ー");
} else if (hoge === "sheep") {
  longSound = "めーめー".includes("ー");
} else if (hoge === "chicken") {
  longSound = "こけこっこー".includes("ー");
} else if (hoge === "frog") {
  longSound = "けろけろ".includes("ー");
} else {
  return longSound;
}
  return longSound;
}

リファクタリング工程

関数、関数の引数名がわかりにくいのでわかりやすい名称にする

真偽値を返す関数なので、一目見てそれがわかりやすい名称に変更。引数名も動物の種類を受け取るのでanimalに変更

console.clear()

const result = isContainLongSound("dog")
console.log(result)

// 指定した動物の鳴き声に長音が含まれるかどうかを返します
function isContainLongSound(animal: string): boolean {
  let containLongSound = false

  if (animal === "cat") {
    containLongSound = "にゃーにゃー".includes("ー");
  } else if (animal === "dog") {
    containLongSound = "わんわん".includes("ー");
  } else if (animal === "elephant") {
    containLongSound = "ぱおーん".includes("ー");
  } else if (animal === "cow") {
    containLongSound = "もーもー".includes("ー");
  } else if (animal === "duck") {
    containLongSound = "がーがー".includes("ー");
  } else if (animal === "lion") {
    containLongSound = "がおーがおー".includes("ー");
  } else if (animal === "horse") {
    containLongSound = "ひひーん".includes("ー");
  } else if (animal === "pig") {
    containLongSound = "ぶーぶー".includes("ー");
  } else if (animal === "sheep") {
    containLongSound = "めーめー".includes("ー");
  } else if (animal === "chicken") {
    containLongSound = "こけこっこー".includes("ー");
  } else if (animal === "frog") {
    containLongSound = "けろけろ".includes("ー");
  } else {
    return containLongSound
  }

  return containLongSound

}

関数の引数の型がstring型で少し広すぎるので文字列リテラルのユニオンを使って限定する

文字列リテラルのユニオンで動物の種類を定義。string型よりも許容する型が限定され、間違った引数を指定するとランタイムではなくビルド時にエラーとなるため実装ミスに気づきやすくなる

console.clear()


const result = isContainLongSound("dog")
console.log(result)


// 指定した動物の鳴き声に長音が含まれるかどうかを返します
function isContainLongSound(animal: Animal): boolean {
  let containLongSound = false

  if (animal === "cat") {
    containLongSound = "にゃーにゃー".includes("ー");
  } else if (animal === "dog") {
    containLongSound = "わんわん".includes("ー");
  } else if (animal === "elephant") {
    containLongSound = "ぱおーん".includes("ー");
  } else if (animal === "cow") {
    containLongSound = "もーもー".includes("ー");
  } else if (animal === "duck") {
    containLongSound = "がーがー".includes("ー");
  } else if (animal === "lion") {
    containLongSound = "がおーがおー".includes("ー");
  } else if (animal === "horse") {
    containLongSound = "ひひーん".includes("ー");
  } else if (animal === "pig") {
    containLongSound = "ぶーぶー".includes("ー");
  } else if (animal === "sheep") {
    containLongSound = "めーめー".includes("ー");
  } else if (animal === "chicken") {
    containLongSound = "こけこっこー".includes("ー");
  } else if (animal === "frog") {
    containLongSound = "けろけろ".includes("ー");
  } else {
    return containLongSound
  }
  return containLongSound
}

type Animal =
| "cat"
| "dog"
| "elephant"
| "cow"
| "duck"
| "lion"
| "horse"
| "pig"
| "sheep"
| "chicken"
| "frog"
| "owl"
| "whale"
| "snake"
| "dolphin"
| "tiger"
| "penguin"
| "parrot"
| "koala";

if文の条件分岐が激しいのでMapを使ってもう少し可読性を向上させる

Mapを使うとsetメソッドでキーバリューをチェーンして追加できるので記述しやすくなります。また、 if文をたくさん書くよりも可読性が上がります

console.clear()

const result = isContainLongSound("cow")
console.log(result)

// 指定した動物の鳴き声に長音が含まれるかどうかを返します
function isContainLongSound(animal: Animal): boolean {
  let containLongSound = false

  const animalSoundMap = createAnimalSoundMap()
  const sound = animalSoundMap.get(animal)
  
  if (sound === undefined) throw new Error(`Invalid animal type=${animal}`)
    containLongSound = sound.includes("ー");
    return containLongSound
}

type Animal =
| "cat"
| "dog"
| "elephant"
| "cow"
| "duck"
| "lion"
| "horse"
| "pig"
| "sheep"
| "chicken"
| "frog"
| "owl"
| "whale"
| "snake"
| "dolphin"
| "tiger"
| "penguin"
| "parrot"
| "koala";


function createAnimalSoundMap(): Map<Animal, string> {

  return new Map<Animal, string>()
    .set("cat", "にゃーにゃー")
    .set("dog", "わんわん")
    .set("elephant", "ぱおーん")
    .set("cow", "もーもー")
    .set("duck", "がーがー")
    .set("lion", "がおーがおー")
    .set("horse", "ひひーん")
    .set("pig", "ぶーぶー")
    .set("sheep", "めーめー")
    .set("chicken", "こけこっこー")
    .set("frog", "けろけろ");
}

letを無くす

letは再代入可能な変数のキーワードで不用意に使うべきではないのでincludesメソッドの実行結果をそのままreturnするように変更

console.clear()

const result = isContainLongSound("dog")
console.log(result)

// 指定した動物の鳴き声に長音が含まれるかどうかを返します
function isContainLongSound(animal: Animal): boolean {

  const animalSoundMap = createAnimalSoundMap()
  const sound = animalSoundMap.get(animal)
  
  if (sound === undefined) throw new Error(`Invalid animal type=${animal}`)
  return sound.includes("ー")
}

type Animal =
| "cat"
| "dog"
| "elephant"
| "cow"
| "duck"
| "lion"
| "horse"
| "pig"
| "sheep"
| "chicken"
| "frog"
| "owl"
| "whale"
| "snake"
| "dolphin"
| "tiger"
| "penguin"
| "parrot"
| "koala";


function createAnimalSoundMap(): Map<Animal, string> {
  return new Map<Animal, string>()
    .set("cat", "にゃーにゃー")
    .set("dog", "わんわん")
    .set("elephant", "ぱおーん")
    .set("cow", "もーもー")
    .set("duck", "がーがー")
    .set("lion", "がおーがおー")
    .set("horse", "ひひーん")
    .set("pig", "ぶーぶー")
    .set("sheep", "めーめー")
    .set("chicken", "こけこっこー")
    .set("frog", "けろけろ");
}

関数の責務を分割する

鳴き声に長音が含まれるかどうかを返す関数

function isContainLongSound(animal: Animal): boolean {
  return soundOf(animal).includes("ー")

}

動物の種類に応じて鳴き声を表す文字列を返す関数

function soundOf(animal: Animal) {
  const sound = animalSoundMap.get(animal)
  if (sound === undefined) throw new Error(`Invalid animal type=${animal}`)
  return sound
}

リファクタリング後のコード

冒頭にお見せしたクソコードよりもかなりみやすくなったのではないのかなと思います。また、関数の責務を細かく分割できたので単体テストも書きやすくなりました。

console.clear()

const animalSoundMap = createAnimalSoundMap()

const result = isContainLongSound("dog")
console.log(result)


// 指定した動物の鳴き声に長音が含まれるかどうかを返します
function isContainLongSound(animal: Animal): boolean {
　　return soundOf(animal).includes("ー")
}


function soundOf(animal: Animal) {
　　const sound = animalSoundMap.get(animal)
　　if (sound === undefined) throw new Error(`Invalid animal type=${animal}`)

　　return sound
}


type Animal =
| "cat"
| "dog"
| "elephant"
| "cow"
| "duck"
| "lion"
| "horse"
| "pig"
| "sheep"
| "chicken"
| "frog"
| "owl"
| "whale"
| "snake"
| "dolphin"
| "tiger"
| "penguin"
| "parrot"
| "koala";


function createAnimalSoundMap(): Map<Animal, string> {


return new Map<Animal, string>()
  .set("cat", "にゃーにゃー")
  .set("dog", "わんわん")
  .set("elephant", "ぱおーん")
  .set("cow", "もーもー")
  .set("duck", "がーがー")
  .set("lion", "がおーがおー")
  .set("horse", "ひひーん")
  .set("pig", "ぶーぶー")
  .set("sheep", "めーめー")
  .set("chicken", "こけこっこー")
  .set("frog", "けろけろ");
}

まとめ

本記事では、TypeScriptのクソコードをリファクタリングしています。主にif文の条件分岐や変数名や関数名の命名、型の絞り込みなど駆け出しエンジニアがコードレビューで指摘を受けそうな箇所と重なる内容が含まれてますので、ぜひ参考にしてみてください。