バリデーションとエラー処理について

値の妥当性チェックはどこでやるべきなのかの話

Webアプリケーションに限った話ではありませんが、特にWebアプリケーションを作っているとき、
ユーザから受け取った値のバリデーションは必ずどこかで行わなければなりません。

例えばJavaでモデルを作るとき、ロジックが入ってくるメソッド等では最初に引数チェックを入れるのが普通ではないでしょうか。
次はトランプのカードを作成するコンストラクタの例です。

public enum Suit { /* 略 */ }
public Card(Suit suit, int num) {
    if (num < 1 || 13 < num) {
        throw new IllegalArgumentException("カードの数値は1から13までで指定しる");
    }
    this.num = num;
    // 略
}

上の例では、Cardモデルのnumに変な値が紛れ込むことは絶対にない、ということになります。

Cardを作成するときのコードはこんな感じだったとしましょう。

List<Card> deck = new LinkedList<>();
for (Suit suit in Suit.values()) {
    for (int num = 1; num <= 13; num++) {
            deck.add(new Card(suit, num));
    }
}

このとき、外部からの入力に頼っていないので当たり前ではありますが、
Cardコンストラクタの引数チェックにはひっかかりません。
もしCardを作成する場所がここだけだったとしたら、引数チェックはなくてもよい、無駄な処理ということになります。

だとしても、上のコンストラクタの引数チェックは不要だからないほうがよい、という人はあまり居ないと思います。それがJavaの思想だと自分は理解しています。

一方でJavaScriptでは、上のような引数チェックはむしろ省略されることの方が多いのではないでしょうか。

JavaScriptは動的型付けであることから、いちいち引数に対してちゃんとチェックしようとすると、その型から調べていかなくてはいかなかったりして非常に大変です。

function Card(suit, num) {
    // 数値である、整数である、範囲内である、をチェックしている
    if (!Number.isFinite(num) || num !== (num | 0) || num < 1 || 13 < num) {
        throw new Error('カードのnumは1から13までの整数で指定しる');
    }
    this.num = num;
}

一概には言えませんが上のようなコードは、ちょっとJavaScriptとしては筋が悪い感じがします。
自分もJavaしか書いた事がなくて、初めてJavaScriptを書き始めたときは上みたいなコードを書いてました。

で、じゃあどうするかというと、基本的なチェックはコントローラ側(呼び出し元)でするということになります。

Webアプリでユーザから送られてきた値を利用するときは、基本文字列ですから
(前回書いたように、Node.jsのExpressでreq.body.xxxやreq.query.yyyの値は文字列ではなくオブジェクトになっていることがあるため、注意が必要ですが)
文字列を数値に変換したりするときに、諸々チェックすることになります。

ここのところJavaだと、いったんString型で受け取ったのをintに変換するところまではコントローラ側の責務で、そこから先の範囲チェックとかはサービス層で処理とかになりそうです。
この辺の処理がめんどくさくて、個人レベルのWebアプリをJavaで書くのはやっぱりだるいなあという感じになりますがそれは余談として。

JavaだろうとJavaScriptだろうと共通なこととして、
そのモデルにかなり密接に紐づくエラーに関しては、事前にチェックするよりも、モデルに値を渡したあとでチェックしたほうが効率的です。

例えばオセロで、白番の人があるマス目(例えばe2)に置く、という入力が送られてくることを考えます。
このときチェックしなければならない項目は次のようなものがあるでしょう。

マス目は正しい値か。(k9のように存在しないマスだったら弾く)
手番が正しいか。
そのマスに、すでに石が置いてないかどうか。
そのマスに置くことで、1つ以上石をひっくり返せるかどうか。

この項目のうち、1はオセロというゲーム自体がもつ情報からチェックでき、
2-4は、このオセロインスタンスに紐づく情報を参照しなければチェックできません。

では、これらのチェックはそれぞれ、置くメソッドを呼ぶ前と呼ぶ後どちらで処理すべきでしょうか？

まず可能かどうかを考えたとき、1は、置くメソッドを呼ぶ前にチェックすることが可能です。オセロのプログラムを作っているのだから、オセロのゲーム自体の情報はプログラム内どこからでも参照できなければおかしいからです。

2,3に関しては、オセロモデルのインスタンスに紐づく情報が必要であるので、オセロモデルがこれらの情報を隠蔽していなければ可能ですが、
「現在の手番」「盤面の状況」という情報はユーザにとっても必須であるから隠蔽されることは考えにくいので、やはり呼ぶ前にチェックできます。

では、4はどうでしょうか。
「石をひっくり返せるかどうかのチェック」は、実際に「石をひっくり返す」という処理と密接に関係しています。
チェックに使う情報は盤面の状況だけですから、外からでもチェックできないことはありませんが、
ひっくり返すのをチェックして、終わってからひっくり返す、とすると二度手間になること間違いなしでしょう。

したがってこういうものに関しては、JavaであろうとJavaScriptであろうと、モデル側で処理をするのが自然ということになります。
ひっくり返す処理を実際に行うメソッドの中でチェックを行い、ダメだった場合はエラーを返すわけです。

私なら、Javaなら1-4全てモデル側でチェック、JavaScriptなら1を呼び出し元・2-4をモデル側でチェックすると思います。

また、「エラーを返す」という微妙に曖昧な書き方をしましたので、その方法について書くと、
Javaを想定すると、この関数の場合、成功時に特に返さなければいけないデータがあるわけではないので、
成功または失敗理由のenumを用意し、それを返すような実装に自分ならします。

もちろん例外を使っても良いのですが、失敗理由全ての例外クラスを用意しなければならないため、ちょっと仰々しい感じがします。

Node.jsでのエラー処理について

Node.jsにおいて非同期なメソッドをつくるとき、守るべき規約があります。

最後の引数に、コールバック関数を取る。
そのコールバック関数の第1引数には、エラーがあるときはErrorオブジェクト、ないときはnullかundefinedを渡す。

function anAsyncMethod(foo, bar, callback) {
  var baz = syncMethod(foo, bar);
  anotherAsyncMethod(baz, function(err, result) {
    if (err) {
      // エラーがあるときは第一引数にErrorオブジェクトを渡す
      return callback(err);
    }
    // 正常終了のときは第一引数nullでコールバックを呼ぶ
    callback(null, result);
  });
}

したがって、非同期なメソッドの中でエラーが発生する可能性がある場合は、この規約を守ればよいので特に問題はありません。

問題は、同期メソッドの時にどうするか。

自分は次の3通りが考えられると思います。(他にあったら教えてほしいです。)

例外機構(throw / try-catch)を使う。
Errorオブジェクトをreturnする。
同期メソッドも非同期メソッドと同じ書き方にする。

このうち、3は、場合によってはありえると思います。
具体的には、処理が重い関数などは、本来なら同期処理ですむ場合でもnextTickやsetImmediateを挟んで一旦他の人に順番を渡す、ということをすべきになる可能性があります。
nextTickやsetImmediateを挟んだ瞬間、非同期関数になりますから、
そうなった場合に、呼び出し元の書き方を変える必要がないのは一つのメリットです。

しかしながら、そういうことが考えにくい場合は、同期処理なのに非同期のように書くのは混乱を生じさせるだけと思いますので、少なくともプロジェクト全体を3で統一するというのは考えにくいです。

それでは、1と2の書き方の違いを見てみましょう。

// オセロモデル側
// 石を置く
Othello.prototype.put = function(teban, x, y) {
  if (this.teban !== teban) {
    throw new Error('手番ではありません');
  }
};

// 呼び出し側
try {
  othelloInstance.put('white', 2, 3);
} catch(e) {
  // エラー処理
}

// オセロモデル側
// 石を置く
Othello.prototype.put = function(teban, x, y) {
  if (this.teban !== teban) {
    return new Error('手番ではありません');
  }
};

// 呼び出し側
var result = othelloInstance.put('white', 2, 3);
if (result instanceof Error) {
  // エラー処理
}

見た目的にどっちが良いかは好みとしか言いようがないかと思います。
タイプ数も大してかわりません。

なので、今回は速度を計測してみました。

var error = new Error('foo');
function method1() {
    throw error;
}
function method2() {
    return error;
}

console.time('a');
for (var i = 0; i < 1000000; i++) {
    try {
        var result = method1();
    } catch(e) {
    }
}
console.timeEnd('a');

console.time('b');
for (var i = 0; i < 1000000; i++) {
    var result = method2();
    if (result instanceof Error) {
    }
}
console.timeEnd('b');

結果

a: 152ms
b: 16ms

というわけで、例外を使うほうが10倍遅いという結果になりました。

なお、今回はnew Errorせずに同じエラーオブジェクトを使い回していますが、
new Errorしたところそれだけで(なにも処理せずとも)5600msかかりましたので、
まあ極力newするなというのもそうですし、
他の処理の重さに比べたらtry-catchにかかる時間など誤差といって差し支えないかもしれません。

例外機構は、catchしなければそのまま上にthrowされるので、その仕組みを有効に使えるのなら、特に使うべきであると思います。
しかしJavaScriptの場合、Javaと違い、try-catchしないと文法エラーとかはないので、2段階以上に例外が伝播するのを正しく運用するのは結構難しい気もしてしまいます。
(Javaの場合、try-catchで囲むか、メソッド宣言にthrowsをつけないとエラーになります。ただし例外がRuntimeExceptionおよびそのサブクラスである場合は除く)

その他では、1にするべき理由としては、標準APIで例外を利用している場合もあるので(JSON.parseなど)、それを使ったりそれに近いことをする場合は、合わせた方が良いとも思います。

また、2の方ですが、
計測した感じでは、Node.jsでのinstanceofは十分速いという結論でよさそうなのですが、それでも気になる場合、あるいはinstanceofなんて長くて書きたくない場合、ErrorオブジェクトのプロトタイプにisErrorプロパティをくっつけて、それをtrueにしておけば、

// result instanceof Errorのかわりに
if (result.isError) {
}

と書く事が出来ます。しかし、resultがnullやundefinedである可能性がある場合、result && result.isErrorと書かなくてはなりませんので、これを考えると別にタイプ数が減るわけでもないので善悪は微妙かなという感じがします。