0→1でスピード重視で作られたアプリのTypeScriptコードを改善するために気にしていること

はじめに

ecforce ma というサービスの開発に参画してから半年程度が経ちました。

当初、スピード重視であまりコードの品質にこだわらずにローンチを目指したサービスのようで、途中から参加した私が読めない（まったく何をしているのかわからない、あるいは理解するまでに時間がかかる）コードが多く、機能追加や不具合修正のたびに時間がかかっていました。

そのたびに、関連するコードの改善をしながら、今ではある程度読み易いコードになったと思いますので、この半年程度で遭遇し、そのたびに改善してきたことをまとめようと思います。

この記事のターゲット

• TypeScriptの経験が多くないが、業務でTypeScriptを書かないといけない人
• どんな書き方のコードをアンチパターンを捉えたらよいのか判断する材料がほしい人

フロントエンド、特にJavaScriptやTypeScriptにある程度精通している人にとっては、特に新しい情報は無いかもしれません。

具体例と解決策

1. 新しい配列の作成をforEach/ifで実装している

const newArray = [];
oldArray.forEach((a) => {
  if (a === xxx) {
    newArray.push(a);
  }
});

new Array = [] で空配列を定義して、必要な中身を後から入れるという手法だと、コードを行ごとに取り出したときに「空配列の瞬間」と「一部の中身が入った瞬間」と「すべての中身が入った状態」に分かれてしまいます。このようなコードの場合、以下のように書くことで行単位での状態がひとつに定まり、シンプルにもなります。

const newArray = oldArray.filter((a) => a === xxx);

2. スコープの中で let による再宣言をしている

let isMatched = false;
numbers.forEach((n) => {
  if (n === xxx) {
    isMatched = true;
  }
});

これも #1 と同様です。加えて、この場合、後続の処理でisMatchedがさらに書き換えられてしまう危険性を排除できません。以下のように書き換えられます。

const isMatched = numbers.some((n) => n === xxx);

同じパターンで、以下のように「分岐した結果をもとに代入しているケース」もあります。

let url = '';
if (xxx === yyy) {
  url = 'yyy';
} else {
  url = 'zzz';
}

このような場合も、後からurlを書き換えられることや、どのような条件でurlが書き換えられるのかをコード全体を追わないと正しく把握できなくなってしまいます。即時関数を用いてconstで定義しましょう。

const url = (() => {
  if (xxx === yyy) {
    return 'yyy';
  } else {
    return 'zzz';
  } 
})();

3. 過度に繰り返されているif文のネスト

if (xxx === aaa) {
  if (yyy === bbb) {
    if (zzz === ccc) {
      // ...do something
    }
  }
}

ロジックを設計せずに書きながらコードを考えているとこのような書き方になりがちです。このような場合、複数の方法で解決できます。

条件分岐をまとめられる場合

const shouldDoSomething = xxx === aaa && yyy === bbb && zzz === ccc;
if (shouldDoSomething) {
  // ...do something
}

条件に当てはまらない場合に途中終了できる場合

if (xxx !== aaa) {
  return;
}
if (yyy !== bbb) {
  return;
}
if (zzz !== ccc) {
  return;
}
// ...do something

また、以下のようなパターンもあります。これは冗長に見えますが、 文芸的プログラミング の観点から、ひとつひとつの処理の意味を伝わりやすくしたい場合に有用です。

条件分岐内の処理を抽象化できる場合

function doSomething() {
  // ...do something
}

function doSomethingWhenCcc() {
  if (zzz === ccc) {
    doSomething();
  }
}

function doSomethingWhenBbb() {
  if (yyy === bbb) {
    doSomethingWhenCcc();
  }
}

if (xxx === aaa) {
  doSomethingWhenBbb();
}

4. undefinedなオブジェクトへのプロパティアクセス防止の記述

if (obj && obj.key) {
  const value = obj.key;
  doSomething(value);
}

型定義を正しくしている場合、undefined or null になりうるオブジェクトへのプロパティアクセスがエラーとなるため、先にオブジェクトの存在判定を行うのが定石となります。この場合、ES2020で追加されたオプショナルチェーンを使うことで、エラーを防ぐことができます。

const value = obj?.key;
if (value) {
  doSomething(value);
}

5. 条件によって返り値が異なる場合の冗長な記述（.tsxの例）

if (xxx === aaa) {
  return (
    <div>
      <h1>見出しaaa</h1>
      <p>本文aaa</p>
    </div>
  );
} else {
  return (
    <div>
      <h1>見出しbbb</h1>
      <p>本文bbb</p>
    </div>
  );
}

xxx === aaa の場合と yyy === bbb の場合の返り値が似ています。共通項を変数を使って返すようにすると、パターンが増えた場合などの応用もすぐにできるようになります。

const headline = xxx === aaa ? '見出しaaa' : '見出しbbb';
const paragraph = xxx === aaa ? '本文aaa' : '本文bbb';
return (
  <div>
    <h1>{headline}</h1>
    <p>{paragpraph}</p>
  </div>
);

これでパターンがさらに増えた場合は、以下のように headline と paragraph を即時関数として切り出すことで、いくつのパターンでも対応できるようになります。

const headline = (() => {
  if (xxx === aaa) {  
    return '見出しaaa';
  } else if (yyy === bbb) {
    return '見出しbbb';
  } else {
    return '見出しccc';
  }
})();
const paragraph = (() => {
  if (xxx === aaa) {  
    return '本文aaa';
  } else if (yyy === bbb) {
    return '本文bbb';
  } else {
    return '本文ccc';
  }
})();
return (
  <div>
    <h1>{headline}</h1>
    <p>{paragpraph}</p>
  </div>
);

さらに要素が増えてくるとこれでも冗長なので、パターンをオブジェクトの配列として持ち、簡易的なDBのように処理するとさらに柔軟なコードになります。

const contents = [
  {
    type: 'aaa',
    headline: '見出しaaa',
    paragraph: '本文aaa',
  },
  {
    type: 'bbb',
    headline: '見出しbbb',
    paragraph: '本文bbb',
  },
  {
    type: 'ccc',
    headline: '見出しccc',
    paragraph: '本文ccc',
  }
];
const type = (() => {
  if (xxx === aaa) {  
    return 'aaa';
  } else if (yyy === bbb) {
    return 'bbb';
  } else {
    return 'ccc';
  }
})();
const content = contents.find((c) => c.type === type);
return (
  <div>
    <h1>{content?.headline}</h1>
    <p>{content?.paragpraph}</p>
  </div>
);

6. 参照代入を使わないことによる冗長な記述

const userInfo = {
  id: '1',
  name: 'name',
};
const newUserInfo = {
  email: '[e-mail]',
  phone: '09011112222',
};
const newUser = {
  id: userInfo.id,
  name: userInfo.name,
  email: newUserInfo.email,
  phone: newUserInfo.phone,
};

上記のように「既存のユーザー情報」と「新しく追加したいユーザー情報」をまとめたひとつのオブジェクトを作成したい場合に、 userInfo. と newUserInfo. を何度も記載するのは冗長です。このような場合、参照代入を使ってオブジェクトの情報をクローンした変数インスタンスを用意することができます。

const userInfo = {
  id: '1',
  name: 'name',
};
const newUserInfo = {
  email: '[e-mail]',
  phone: '09011112222',
};
const { id, name } = userInfo;
const { email, phone } = newUserInfo;
const newUser = {
  id: id,
  name: name,
  email: email,
  phone: phone,
};

また、ES2015以降では、JavaScriptのキーの名前とプロパティの変数名が同じであれば、省略して記述できるので、さらに以下のように書くことができます。

const userInfo = {
  id: '1',
  name: 'name',
};
const newUserInfo = {
  email: '[e-mail]',
  phone: '09011112222',
};
const { id, name } = userInfo;
const { email, phone } = newUserInfo;
const newUser = {
  id,
  name,
  email,
  phone,
};

7. スプレッド構文を使わないことによる冗長な記述

上記の #6 のケースでは、実はそれ以前に スプレッド構文 を使うことにより、オブジェクトを簡単に展開することができます。

const userInfo = {
  id: '1',
  name: 'name',
};
const newUserInfo = {
  email: '[e-mail]',
  phone: '09011112222',
};
const newUser = {
  ...userInfo,
  ...newUserInfo
};

配列の場合も同様です。

const userIds = ['1', '2', '3'];
const newUserIds = ['4', '5', '6'];
const allUserIds = [
  ...userIds,
  ...newUserIds
];

8. 単純な条件一致に Array.prototype.some を使っている

const userId = '1';
const userIds = ['1', '2', '3'];
if (userIds.some((i) => i === userId)) {
  // ...do something
}

Array.prototype.some は、条件に当てはまるかどうか比較を行う（計算をするなど）に対し、ES2016で追加された Array.prototype.includes() は、シンプルな「一致」を判定します。

const userId = '1';
const userIds = ['1', '2', '3'];
if (userIds.includes(userId)) {
  // ...do something
}

Array.prototype.some は Array.prototype.includes よりも格段に処理に時間がかかることが明らかになっています。たとえば JavaScript の Array.some と Array.includes と Array.everyの実行速度比較 などの記事でその結果が紹介されています。

9. 意味のないコメント

// AをBにセットする
function setAtoB() {
  // ...do something
}

言語を問わず、ソースコードではこのような「意味のないコメント」が見られることが少なからずあります。 setAtoB というメソッドに対して AをBにセットする というコメントをする意図は何でしょうか？ コメントに魂を込めよ（意訳）ということが、 リーダブルコード でも述べられています。このようなコードでは、「読み手に何を伝えたいのか」という気持ちが一切感じられません。（英語が読めない人がいるかもしれないというなら、思い切って関数名や変数名を日本語で定義すればよいでしょう。）

私のおすすめは、「コードから読み取れないこと（そのコードを実装するに至った背景や、修正・削除するタイミングなど）」をコメントすることです。

// この処理のロジックはチケット https://... に記載されています
// XXX機能でしか使わないので、XXX機能が不要になったらこのコードも不要になります
function setAtoB() {
  // ...do something
}

10. メソッドの処理内容や目的をコメントで記載している

// Aが...に存在していたら...を実行する
function checkA() {
  // ...do something
}

上で記載した #9 と同様、 リーダブルコード には「メソッドの処理内容をコメントで説明しなければならないなら、それはメソッド名がイケてないからだ（意訳）」ということが記載されています。上記の例では、 checkA というあまりにも抽象的なメソッド名のせいで、「Aの何をどうチェックするのか」がコード（またはコメント）を読まないと伝わらなくなっています。

冗長でも、下記のように「メソッド名を見たら処理内容（と、さらに言えばその目的）がわかる」ような記載を心がけましょう。

function do...WhenAexistsIn...() {
  // ...do something
}

この例の場合、たとえばこのようなメソッド名を書いたときに「そもそも WhenAexistsIn... をさらに分割できないか？」ということに気が利くようになり、以下のようにさらに改善できるかもしれません。

function isAEsistsIn...() {
  // ...check A existance in...
}
function do...() {
  // ...do something
}
if (isAEsistsIn...()) {
  do...();
}

いったん、思いついたことを羅列してみただけですので、今後追加するかもしれません。

本文中であげた リーダブルコード 以外にも、最近では以下のような書籍も出版されており、参考になるでしょう。

• リファクタリング(第2版): 既存のコードを安全に改善する(2019/12/1)
• 良いコードを書く技術 ── 読みやすく保守しやすいプログラミング作法(2021/4/28)
• 良いコード/悪いコードで学ぶ設計入門 ―保守しやすい 成長し続けるコードの書き方(2022/4/30)