うまく抽象化できてないコードは読みづらい

短いコードのほうが読みやすい傾向はあります。しかしながら、 短くて誤読しやすいコードよりは、長いけど誤読しないコードのほうが可読性が高いです。 今回はその話をします。

「短ければ可読性が高い」というのは勘違い

短くても可読性が低いコードはあります。例えば以下の2つの main 関数を比べてみます。

短いけど抽象化に失敗しているコード:

const main = async () => {
  const _article = await fetch('/article');
  const article = transformItem(_article);
};

長いけど分かりやすいコード:

const main = async () => {
  const _article = await fetch('/article');
  const article = {
    ..._article,
    fetchedAt: Date.now(),
    id: `${item.title}__${item.body}`,
  }
};

main 関数は前者のほうが短いです。しかし、transformItem の中身を読まなければ動作が分かりません。

読まずに済むコードを増やす

上記の短いけど抽象化に失敗しているコードを改善します。例えばこんな感じです。

const main = async () => {
  const _article = await fetch('/article');
  const article = addUniqueId(addArticleFetchedTime(_article, Date.now()));
};

「短いけど抽象化に失敗しているコード」と同じくらいの長さになりましたが、main 関数を読むだけでコードの動作をだいたい予想できます。

このように、コードを短くするには詳細を読まずに予想でカバーできる部分を増やす必要があります。 そして、詳細を読まなかったときに誤読させてはいけません。

抽象化に失敗しているケース

抽象化に失敗しているケースは他にもあります。いくつか見ていきましょう。

• フォールバックが含まれていることが表現されていない
• 関数の動作が引数によって変わりうる

フォールバックが含まれていることが表現されていない

const App = () => {
  const cart = response.item.length > 0 ? response.item.join(', ') : 'カートは空です';
  /* ... */
  return <div>カート内のアイテム: {cart}</div>;
}

コンポーネントの実装を読むとき、return の直後から読む人がほとんどだと思います。しかし、return 以降だけ読むと このコンポーネントは「カートは空です」と表示されうることを読み取れません。 また、「カートは空ですと表示されることが cart を読めばわかる」ということも読み取れません。

この場合、フォールバックを cart に含めないように修正すると分かりやすくなります。

const App = () => {
  const cart = response.item.length > 0 ? response.item.join(', ') : null;
  /* ... */
  return <div>カート内のアイテム: {cart !== null ? cart : 'カートは空です'}</div>;
}

これなら cart の宣言部分を読まなくてもコードの意図をおよそ誤読せずに理解できます。

あるいは、ちょっと苦しいですが変数名を変えるのも手の一つです(フォールバックを含めないようにした方が大抵良いです)

const App = () => {
  const cartMessage = response.item.length > 0 ? response.item.join(', ') : 'カートは空です';
  /* ... */
  return <div>カート内のアイテム: {cartMessage}</div>;
}

この例を見てわかる通り、やはり「フォールバックが存在する」と変数名で表現するのは無理が生じやすいです。

関数の動作が引数によって変わりうる

const fetchArticle = (articleId?: string) => {
  if (typeof articleId !== 'string') {
    return;
  }
  return fetch(`/article/${articleId}`);
}

fetchArticle は動作が articleId によって変わっています(undefined の場合は何もせず、string の場合は fetch する)。一般に 動作が変わることを関数名と引数だけでは予想できません。

解決するには、articleId を string にすれば良いです。

const fetchArticle = (articleId: string) => {
  return fetch(`/article/${articleId}`);
}

こういったコードは、最初は JavaScript で書かれていたことが多いです。TypeScript にするタイミングで直しましょう。