日々の破片

_ ファットコントローラよりも怖いファットビュー

今更ながらangular.jsをいじくってベストプラクティスを探しているのだが、少なくともワーストプラクティスは見つけた。

それはファットビューだ。

例

<div ng-show="model.state=='initializing' || model.state=='waitResponse' || model.state=='processing'"> <!-- もっとたくさん条件があったりする -->

(ところで、model.stateがStringを取るとわかりきっていて、かつ比較対象がどう転んでもinitializingだのwaitResponseだのといったStringの場合に===を使うのは僕には過剰だと思えるが、JavaScriptプログラマはこういう場合でも===を使うものなんだろうか？ 何らかのバグでmodel.stateがnullやundefinedや1000歩譲ってNumberになってしまう場合があるとしても真になることはあり得ないはずだ)

ファットビューの特徴はtell, don't ask原則に反していることだ。

ここではtell, don't askをオブジェクト自身が持つ判定メソッドを呼び出す代わりにプロパティ（この例ではフィールドだが）を参照してオブジェクトを使う側が自前で判定する意味で使っている。tell（判定せよ）ではなく、ask（今のプロパティ値は何？）を動詞にすると、使う側がそのオブジェクトの振る舞いを決定することになる。

つまり上の例だと、ビューであるHTMLが、model（ビューモデルだと思う）の状態を問い合わせてDIVの表示/非表示を切り替えようとしている。

問題は、表示するかどうかの判断をビューが行っていることにある。それはビューの責務から当然か？

当然ではない。ビューモデルにビューがついているのは偶然ではないからだ。

まず、ビューとビューモデルは一方通行だが相当密接な関係にある。呼び出しの方向から行けばビューはビューモデルを操作できる。一方のビューモデルはどう表示されるのかはビューに任せてはいるものの、何を表示するかは自分が知っている。当然、現在表示されるべきか隠されるべきかも知っているし、知らなければならない。

自分が知っているやつの状態を並べて表示すべきかどうかを判断するというのは越権行為だと考えた方が良い。

端的には、以下のようにすべきだ。

<div ng-show="model.isVisible()">

isVisible: function() {
  return state == 'initializing' || ...  // とは言え、もう少し賢い管理方法はあるんじゃないか？
}

isXXXメソッドもゲッタメソッドのうちなので、model.isVisible()もaskじゃないかというような屁理屈を言うことはできるが、やはりそこは違う。ここでのisVisibleは条件判断を伴うからだ（というわけで、単にプロパティ値を返すだけのgetXXX()メソッド=ゲッタはJavaBeanのようにフレームワークが要求するのでなければ禁止したほうが良い。そもそもゲッタはアノテーションが無い時代にオブジェクト状態を永続化するために導入された苦肉の策が発祥であって、通常のコードではこれっぽちも必要ないものだ）。

今、ここに別のプログラマがやってきて、modelの新しい状態queryingServerとその状態を引き起こすコードを追加したとする。当然話の都合上、queryingServer状態も表示しておきたいとする。

最初のファットビューは簡単に、html側の修正を忘れる。正常系については普通はテストをするとはいってもqueryingServer状態は通常0.1秒で解消するとすると、Serverがハングアップして10秒くらい応答がないときに初めて発覚するバグとなる。異常系のテストをして見つければ良いけど、（異常系のテストのしやすさにもよるが）どうもコミットが無駄に2回に分かれるような予感がする。

もちろん、isVisibleの修正を忘れたり気付かなかったりする可能性もあり得る。しかし、ビューが状態を判断するのと、ビューモデル自身が表示状態を判断するのとどちらが、あり得るだろうか？ コミットが2回に分かれるとしても、同じソースの修正のほうがましだという感覚がある。

逆に状態を削除する場合もある。queryServerを追加したら結果としてwaitResponseの状態は不要になったので削除したとする。

同じくisVisibleの修正を忘れたとする。でもそれは誰か気の利いたやつがふとこの状態を設定するコードがないことに気付けば削除できるかも知れない。

一方、html側から削除するのは相当至難なことになる。ビューモデル自身のコードでは設定していないのはすぐわかるが、model.stateを生でビューが見ているということは、同じく生でどこかで設定しているかも知れない。面倒だから放置しておこう……こうやってコードの窓は割れていくのだ。

ちょっと待ったと声がする。それはわざわざどこかで、$rootScope.model = new Model()みたいなコードを入れているからそのオブジェクトにisVisibleとかが定義できるのであって、コントローラが直接$rootScope.state = 'initialize'とか設定することを考えると、コントローラに$rootScope.isVisible = function() { return ... }というようなコードが入ってファットコントローラになるではないか。ファットコントローラは悪いと聞いたのでビューに判定させているのだ。これもあまり良い考えではない。ファットコントローラになるなと感じたらビューモデルの導入を考えれば良いだけのことで、ビューに条件を記述して良い理由にはならない（9600bpsの回線なのでファイルが1個増えるかどうかで速度が変わるという場合は、そもそもangular.jsとか使ってはいけない）。

あるいは根本的にisVisibleな状態を判定するのが間違いで、最初のHTMLが単に

<div ng-show="!(model.state || model.state=='idle')">

だったらどうだろうか？

状態がidleかnullだかundefinedなら非表示という判定方法なら最初の長々としたHTML記述を避けられるから問題ないじゃないか。

これは一面の真実で、だめなコードは見事なまでにだめな選択（この場合は真偽判定をif側とelse側のどちらで見るか）をしているものだ。

でもだめな選択を仮にしても問題ないようにするには、やはりビューに許すのは、ビューモデルの単一のフィールドの参照か（ゲッタはほとんど意味ないと思うのでフィールド参照は否定しない（まれにフックしたいときにはフィールド参照は困るがほとんど例外的だ）。ある意味では同一パッケージなので可視性があって当然だ）、単一のメソッド呼び出しに限定させるべきだ。というのは、HTMLを修正するよりも、コードを修正するほうが楽だからだ。早い話、HTMLで対応する)を忘れたりして文法エラーを引き起こすよりも、自前のJavaScriptの中で文法エラーを引き起こすほうが、はるかに見つけるのが楽だからだ（angular.jsの生成コード内をトップにしたスタックトレースからちゃんとエラーを見つけて修正しているやつはあまり見かけないという事実があったりして）。だめな選択をするようなプログラマが文法エラーなしのコードをいきなり書くと想像するのは無理がある。

Angular.jsの本ってどれも星が少ないけどなぜだろう？