Lycolog

TypeScriptで関数を書いているときに戻り値の型を書くケースがあるが、個人的にはあれは基本書かないほうがいいと思っているので、その理由を書いていく。

コード記述が冗長になる

まず型を書くと記述が冗長になる。以下のコードを見ると戻り値型が長く読みづらく、書くのも面倒だ。まずこんな長い命名をやめたほうが…というのはあれど、現実問題として長い命名は存在するので仕方ない。

export const getCampanyAndDepartmentAndEmployeeFromPrefectureCode = (
  param: VeryveryLoooongestParamTypeNaming,
): VeryveryLoooongestReturnTypeNaming => {
  return param.db.con.execQuery(
    "SELECT * FROM foo WHERE id = :?",
    param.dbParam.foo.id,
  );
};

しかし以下であれば戻り値型がない分すっきりしていて見やすいし、書く手間も掛からない。更に型推論によって正しい型が返るので理想的だ。

export const getCampanyAndDepartmentAndEmployeeFromPrefectureCode = (
  param: VeryveryLoooongestParamTypeNaming,
) => {
  return param.db.con.execQuery(
    "SELECT * FROM foo WHERE id = :?",
    param.dbParam.foo.id,
  );
};

実装と異なる戻り値型を暗黙的に記述できる

例えば以下のように書けば戻り値の型は'foo' | 'not foo'となり、常に正しい型が返る。

export const foo = (isFoo: boolean) => {
  return isFoo ? 'foo' : 'not foo';
};

しかし以下のように戻り値の型を定義すると実装上存在しない'bar'が返る。これは実装時に無用な混乱を生む。一般的にこのようなケースは仕様削除やリファクタなどで生まれることがあると思うが、そういうメンテ漏れにもなるので書かないほうが安全だといえる。

export const foo2 = (isFoo: boolean): 'foo' | 'not foo' | 'bar' => {
  return isFoo ? 'foo' : 'not foo';
};

他にも次のケースでは戻り値がstringとなり、何が返ってくるのかが実装を見ないと解らなくなる。特に理由がないなら書かないほうがよい。

// 'foo' | 'not foo'になるはずだがstring扱いになる
export const foo3 = (isFoo: boolean): string => {
  return isFoo ? 'foo' : 'not foo';
};

型記述が混乱する

ここは基本的に前項の内容と重複する内容となる。

例えば次の二つの実装は同じだが、戻り値の型だけが異なる。こういう実装が混在すると実装の一貫性が失われ無用な混乱を生むので、指定しないほうが望ましい。

const IndexPage = (): JSX.Element => {
  return (
    <Layout title={'Hello Next.js'}>
      <>
        <h1>Hello Next.js 👋</h1>
      </>
    </Layout>
  );
};

const IndexPage2 = (): ReactElement => {
  return (
    <Layout title={'Hello Next.js'}>
      <>
        <h1>Hello Next.js 👋</h1>
      </>
    </Layout>
  );
};

書いてもよいと思うケース

例えば依存関係を持たせたい時など、インターフェースとして型を共通化したい場合は書いてもよいと思う。これはどこでそれを使うのかが自明になるからだ。改修時にも型によって関連処理が見出しやすくなるので意識しやすくなる。

export const createPostMessage = (
  channel: string,
  username: string,
  message: string,
): PostMessage => {
  return {
    channel,
    username,
    message,
  };
};

export const postMessage = async (param: PostMessage) => {
  try {
    return await fetch('https://example.com/api/postMessage', param);
  } catch (err) {
    return err;
  }
};

但しこのようなケースではUnit Testを書いて、実装された戻り値型を満たす値が返ることを確認するのが望ましい。

戻り値型を書かないことによるデメリット

TypeScriptの公式リポジトリによると、型推論の速度に悪影響を及ぼすとあるので、型推論の速度が落ちるという点だ。

もし型推論の速度が非常に遅いと感じた場合は書いてみてもよいと思うが、公式でも以下のように案内があり、和訳すると「型推論は非常に便利なので、これを普遍的に行う必要はありませんが、コードの遅いセクションを特定した場合に試してみると便利です。」とあるので、余程複雑なことをしていない限り不要だとは思うし、そんな複雑な型を返すような処理は必要がなければ書かないほうがいいだろう。

Type inference is very convenient, so there's no need to do this universally - however, it can be a useful thing to try if you've identified a slow section of your code.

少なくとも私は実務上、型推論の速度に困ったことがないのと、tscでビルドすることも稀であるため、ビルドに影響することもない。よって基本書いていない。

今の職場でコードを書いていてifに対して原則elseを書かないという雰囲気があるのですが、個人的にはelseを書きたいのでその話です。

世間的にelseを書かないというコーディングルールは一定の支持があるようで、まぁ一種の宗教だとは思っていますが、個人的にはどうなんかなと思っています。まぁ割と最近の流行りな気もしていますが…。（昔はなかったと思う…というとアレですが）

elseがあってほしい理由

else ifやelseブロックがあると処理の関連性が見やすくなると考えています。

処理の関連性がわかりやすくなる

例えば以下のコードの場合、ifが連続しているよりelseがある方が処理の関連性が明示的に見えると思います。elseで繋げないことは基本的に処理として関連性がないはずです。

// ifのみ
export const hoge = (mode: 1|2|3) => {
  if (mode === 1) {
    // なんかの処理
    return 'hoge';
  }
  if (mode === 2) {
    // なんかの処理
    return 'piyo';
  }
  if (mode === 3) {
    // なんかの処理
    return 'fuga';
  }
}

// elseあり
export const piyo = (mode: 1|2|3) => {
  if (mode === 1) {
    // なんかの処理
    return 'hoge';
  } else if (mode === 2) {
    // なんかの処理
    return 'piyo';
  } else if (mode === 3) {
    // なんかの処理
    return 'fuga';
  }
}

予期せぬ不具合が入り込みづらくなる

ifだけで構成されたコードでは以下のようにifブロックの外に処理を入れ込むことができますが、このときA1, B1, C1のifの外にある処理は後続処理に影響します。しかしこの書き方だと処理の影響範囲が見積もりづらく、不必要なバグを生む元になるため、個人的には避けたいと考えています。（他の処理の作用に引っ張られているので、副作用のある実装ということになると思っています）

またA1, B1, C1の行数が伸びるとコードの見通しが悪くなり、if同士の関係性が分かりづらくなります。

elseを使うとこういった副作用的な振る舞いをする実装が作りづらくなり、影響範囲もブロックインデントで分かれるので、そういった心配がなくなります。

// ifのみ
export const hoge = (mode: 1|2|3) => {
  // なんかの処理 A1
  if (mode === 1) {
    // なんかの処理 A2
    return 'hoge';
  }
  // なんかの処理 B1
  if (mode === 2) {
    // なんかの処理 B2
    return 'piyo';
  }
  // なんかの処理 C1
  if (mode === 3) {
    // なんかの処理 C2
    return 'fuga';
  }
}

elseが必要なのに書き忘れることが減る

elseを書かないことに捕らわれていると以下のようなコードが生まれることがあります。

さてこのコードにおいて!validHoge(hoge) || !hasPiyoのケースはどのように処理されるのでしょうか？答えは処理されませんが、もしこのコードに対してUnit testが実装されておらず、特段のドキュメントもなければ、それが正しいのかどうかをコードから読み取ることが出来ません。

const isValidUsername = (username: string, hasInputed: boolean) => {
  if (validUsername(username) && hasInputed) {
    return requestSearchUsername(username);
  }
}

せめてこう書いてあれば判断も付くでしょう。

const isValidUsername = (username: string, hasInputed: boolean) => {
  if (validUsername(username) && hasInputed) {
    return requestSearchUsername(username);
  } else {
    // 何もしない
  }
}

因みにこの処理はユーザー登録フォームでユーザー名の書式が正しければ既に存在するユーザー名かどうかをAPIに問い合わせ、既に存在すればエラーメッセージを、存在しなければtrueを返すという内容ですが、そもそも書式が不正である場合は何もしないため、ユーザーはエラーであることを知ることが出来ません。

もしelseブロックを書いていれば考慮漏れに気づけた可能性もあったのではないかと思います。

elseを使うケースを考えなくて良くなる

ifに対して原則elseを書かないというルールがあってもelseを書かないと成り立たないケースは存在します。そう言った場合に原則ifしか書いてはならないというルールがあるとelseを書いてよいかどうか考える必要が出てきますが、元からelseも書いて良いルールであればそこは考える必要がありません。個人的に判断の余地が生じるコーディングルールはチーム開発ではない方が良いと考えています。

またこのルールの結果、本来必要だったelseを書かなかったことにより不具合が発生する可能性もあります。（実装者が軽率にelseを使わなかったことによる判断ミス）

そもそも何故elseを書いてはならないのでしょうか？コードのネストが深くなるからでしょうか？少なくともelse if相当のifには、その作用はありません。あるとしたら純粋なelseブロックはインデントが減るでしょう。しかしこのインデントはあったほうが処理の関連性が掴みやすくなると思います。

elseのないifは悪か？

ここまで散々elseを書くべきと言ってきましたが、ではelseがないifは悪かと言うと、そうは思いません。例えば、以下のような早期リターンと呼ばれる記法であればそれは良いと考えています。では何を早期リターンとするかですが、個人的には例外的に処理を継続しない場合を一つの基準にしています。私もここの感覚は上手く言語化できていないのですが、恐らく何もしないときだけelseを書かないのは問題ないと考えています。

これは本質的ではない処理のせいで、いたずらにネストが増えるのを防ぐためです。関連性がある場合にelseを使うのがコードのメリハリとして目視で読んだときの認知性の向上に繋がると考えています。

export const putLog = (message: string) => {
  if (message === '') return;

  console.log(message);
}

この点については個人的に共感できる意見があったので以下の記事を紹介させて頂きます。

else句を使わないのが良いコードなの？いや、そんなはずは・・・ · DQNEO日記

記事中にある以下の部分、特殊ケースに対してガード節を使うというところで、例外ケースでのみ早期リターンをするという部分がありますが、やはりこれが一番無難だなと思いました。

条件記述の単純化　「ガード節による入れ子条件記述の置き換え」
特殊ケースに対してガード説を使う

ただ例外ケースとは何か？それは数値化でき、チーム開発で標準的に取り扱えるものなのか？と言われると、正直私もそこまでは言語化できていないので難しい部分ではあります。そもそも人間が書くコードが完全に均一になることはないと考えているので、そこまで強い思想を持って考えてはいないです。（もしChatGPTに書かせてもバラツキは出るでしょう）

私がこの記事で言いたいのは単にelseを書いてもいいのではないかということと、その場合でも早期リターンは許容しても問題ないだろうというところなので、一旦早期リターンの基準についてはここでは考えないこととします。

そうなると結局elseなしでifだけあればいいじゃないかという話に戻ってくるとは思うのですが、正直そこは常識で考えてほしいというところです。まぁ常識も人の数だけあるので難しいですし、そんな物があればこんな記事も生まれていないわけで、ソフトウェア開発は難しいですね。

undefinedの判定方法が複数あるということでundefined判定の処理速度比較をしてみたのでその結果。

端的に言うと、hoge === undefinedとtypeof hoge === 'undefined'の二方式がある。後者は原則考慮不要だが、言語仕様上存在しているので比較したが、現実的に見た場合、どちらで記述した場合でも処理速度に有意な差はないように感じた。

確認環境

比較結果

hoge === undefinedの方が早く見えるが実行するタイミングで変わるので誤差の範疇だと思う。

確認コード

const tyof = (param?: string) => {
  return typeof param === 'undefined';
};

const undef = (param?: string) => {
  return param === undefined;
};

const tyStart = +new Date();
for (let i = 0; i < 10000000000; i++) {
  tyof();
}
console.log('typeof', +new Date() - tyStart);

const unStart = +new Date();
for (let i = 0; i < 10000000000; i++) {
  undef();
}
console.log('undefined', +new Date() - unStart);

TSから生成されたJS

"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
const tyof = (param)=>{
    return typeof param === 'undefined';
};
const undef = (param)=>{
    return param === undefined;
};
const tyStart = +new Date();
for(let i = 0; i < 10000000000; i++){
    tyof();
}
console.log('typeof', +new Date() - tyStart);
const unStart = +new Date();
for(let i = 0; i < 10000000000; i++){
    undef();
}
console.log('undefined', +new Date() - unStart);

あとがき

MDNを読む限りtypeof hoge === 'undefined'は該当変数が存在しない場合に有用なようであるが、TypeScriptで書いている場合、通常このようなコードが生まれることがなく、仮に起きるとした場合、次のようなコードになるため現実的に考慮する必要はない。なおMDNにも「こんなことはしないこと」と書いてあるので、一般的なコードでないことは客観的にも伺えるだろう。

(() => {
  const undefined = 123;
  const hoge = undefined;

  if (typeof hoge === 'undefined') {
    console.log('hoge is undefined');
  } else {
    console.log('hoge is not undefined');
  }
})();

上記コードの実行結果としてはhoge is not undefinedが出力される。

このコードの主な問題点

1. const undefined = 123;というコードは予約語を変数名にしているため、混乱を招くコードであり、書かないことが好ましい
   1. MDNには予約語ではないとあるが、一般的には予約語の一つとして解釈して支障ないと考える
2. このコードはESLintのeslint:recommendedで検知されるため、通常であれば書かれることはない
   1. no-shadow-restricted-namesに引っかかる

なお、このコードは例示のために即時実行関数形式で記述しているが、必要がない限りこの形式での実装は避けたほうが問題が少なくなると思う。これは不必要なネストが生まれたり、スコープの混乱を生むためである。