0. 背景
職場その他でいくつかのRailsプロジェクトを見て来て、同じ組織であってもリポジトリが違えば雰囲気が全然違うなと思い、その中でもこれはダメだろうと思ったことがありましたので、自分の備忘録も兼ねて記述します。
ここ２年ぐらいで出会ったRailsプロジェクトを見て感じた例ですので、他にも挙げようと思えば挙げられると思いますが、出会った中での記述ということでご理解ください。
また、技術的、より個別的な事象についてはRails AntiPatternsを読むといいかもしれません。
1. rubocopを導入していない
rubocopはRuby Style Guideをベースにしたrubyの静的解析ツールです。Rubyを使ったことのある人で知らない人はいないでしょう。また、Ruby Style Guideについても、Rubyを勉強する初期に一読しておかなければいけないと言われる代物です。なお、日本語訳もありますので、英語が苦手な方でも大丈夫です。
Rubyによる表現は柔軟性が高く、同じ処理をしていても、書く人によって様相が全く異なることがあります。それがRubyの強みの一つでもあるのですが、あまりにみんながみんな好き勝手なオレオレスタイルで実装すると、可読性・保守運用性等の観点で問題になります。プライベートで1人で作っているのであれば、自由に書けば良いですが、仕事で使うのであれば、その所有者は自分でありませんし、自分以外の人間が読むということを意識すれば、自然とチームでスタイルを統一しようという思いにつながるでしょう。コードは書く時間よりも読む時間の方が長いと言われます。自分がいなくなっても継続的にユーザーに価値を提供するためには、可読性が高いに越したことはありません。
Ruby Style Guideは先人たちが議論して作り上げて来たものです。今でも継続的に改善されています。つまり、こう書いた方が読みやすいという先人たちの教えとも言えます。私を含めほとんどのエンジニアは矮人なのですから、巨人の肩の上に乗るのが得策と言えます。
また、人間では気づきにくいミスであったとしても、機械なら容易に気づけることもあります。例えば、RubyのHashで同じキーが２回以上存在することはバグの温床となりますが、人間でも注意深くみれば気づけますが、機械ならば容易に気づけます。開発途中で必要と思って定義した変数が結局使われなかったにもかかわらず、そのまま残っていたりした場合も、rubocopは指摘してくれます。そうした機械的に気づけることは機械に教えてもらうというのは、品質向上の一助になることは間違いありません。
簡単に導入できる静的解析ツールが導入されていないリポジトリは品質を蔑ろにしていると言って良いでしょう。
2. Controllerにロジックがある
Railsの設計においては、業務ロジックはControllerに書くのではなく、Modelに書くのがよしとされています。MVCの基本的な概念を知っていれば、Controllerに複雑な業務ロジックをゴリゴリ書くということはしないのですが、私は、とある集計処理を描画するRailsアプリケーションで、３〜4つの集計処理がすべて一つのアクション内で処理されているのを見たことがあります。しかも、HTMLのときと、CSVのときとJSONのときとで微妙に集計が異なり、それぞれ書かれていたりするものですから、非常に長いアクションとなっていました。rubocopが導入されていれば、長いメソッドは指摘されますが、悪いRailsアプリケーションの典型例よろしく、rubocopは導入されていませんでした。
MVCの基本的な概念は当然勉強しておくとして、なぜControllerに業務ロジックを書いてはいけないか、Modelに書くべきかという大きな理由の一つとしては、テストの書きにくさにあると私は思っています。複雑な業務ロジックはできる限り、単体テストで行いたいものです。まして同じアクションに３〜４つのロジックがあったりするとパターンは膨大になり、手に負えなくなります。そのために、単体テストを書ける程度にまで最低限分割するのは、テスト容易性の観点からも大事なことでしょう。1
3. テストが全くない
ここで、テストについて少し触れていこうと思います。私は、このアンチパターンの一つに「テストがない」ということは入れないでおこうと思っていました。なぜなら、初期段階で、業務要件が定まっていない状態でテストを作り込むのは決して得策ではないと思うからです。また、初期であれば、アプリケーション自体も複雑ではなく、理解しうる規模であれば、リリースを優先して、テストを書かないという判断はありうると思ったからです。
しかし、これを書いているうちにやはり入れるべきだと思うようになりました。まだリリースしていない開発段階で、本当の意味での初期ではまだテストはなくても良いでしょう。しかし、Webアプリケーションを作るのは、何らかの少し複雑なことをコンピュータにやってもらいたいからです。そこにテストが無いというのはユーザーに価値を早く提供するという観点からもあり得ないと思いました。自動テストはわざわざ自分がブラウザ上で確認しなくてはいけないことを自動で行ってくれます。デグレがないかを確認する作業の短縮のためにも、テストは必須です。
しばしば、リリース優先でテストは後回しということがあります。私は、テストには濃淡をつけていけばいいと考えています。つまり、システム的にクリティカルな箇所はきちんとテストを書くけれども、さして重要でない箇所に関してはあまり書かないといった具合です。クリティカルな箇所の例としては、決済や請求に係るところが挙げられるでしょう。例えばECサイトであれば、決済系・請求系のシステムに不具合があると売り上げが全て吹っ飛んでしまいます。会社の存続に関わる一大事です。一方、単にサイトの説明のページなどほぼ静的なページは重要性は高くないと言えるでしょうし、複雑性も低いと思いますので、テストに時間をかけるのは得策ではないでしょう。
もう一つの視点としては、不具合に気付きやすいかどうかという観点です。例えば、ユーザーがよく使う機能であれば、エラー通知などの機構を備えて入れば不具合が発生すると、すぐに気づくことができますし、サポートへの連絡を注視していれば、早い段階で気付けます。申し訳ないけれども最初の1人に実験台になってもらって不具合に気づいて速やかに修正するというのは、手段として考慮すべき一手だと思います。
一方で、一ヶ月に数回しか回らないバッチ処理などは、気づくのが遅くなりがちです。月に数回しか気づくチャンスがないわけです。しかもバッチ処理ということは、複数のレコードを一度に処理をしてしまうということですので、考慮しなければならないパターンが多くなるということです。こういう時こそ、自動テストの出番と言えるでしょう。例えば、モデルのメソッドを少し修正した時にビューでのチェックはできたけれども、バッチに使われていることに気づかなくて不具合を生み出してしまったということも、テストがあれば気づく可能性が高まります。バッチ処理こそテストを書く意義の高い箇所と言えると思います。
4. データベースが正規化されていない
Railsアプリケーションの中には正規化されていないものはたくさんあると思います。パフォーマンスの観点等から意図的に正規化を崩している場合もあるでしょう。しかし、開発初期の段階から正規化を崩すのは得策ではありません。レコード数が増えてパフォーマンス上の問題が発生してからでも遅くはないでしょう。正規化されている状態から正規化を崩すのは容易でも、正規化されていない状態から正規化することの難易度は一般的に高いです。
中には、カラムの型すら守られていない場合があります。メールアドレスのカラムなのになぜかユーザーIDが入っていたりするアプリケーションが実在することを知った時の驚きは破壊的です。
なぜこのようなことが起こるのかというと、きちんと業務分析をしないまま開発を進めてしまうためだと思います。もちろん、変化の激しい現代において要件を１００％固めるのは不可能と言っていいでしょう。しかし、大事なのは、初期の段階でどこまで見通せるかというところです。その上で、現在の要件にはないけれども、近い将来ビジネス側は必ずこれを要求してくるであろうということをエンジニアは予測し、それに対してもデータベース設計を崩すことなく対応する必要があります。
データベースというのは、業務を極度に抽象化したものと捉えることができます。業務を写す鏡と言って良いでしょう。良いデータベース設計ができていれば、データベースモデリングから業務がわかると言われます。開発の初期段階において、データベース設計、ひいては業務分析というのは、もっとも時間をかけるべき箇所であると言えます。そもそも、ビジネス側も考慮していないケースというのは多々あります。それを、ヒアリングやディスカッションをしていくなかで聞き出し、データベース設計をしていく中で整理し、不明点、考慮漏れをできるだけ無くしていく。そうすることで洗練された業務分析あるいはビジネスモデルの確立がされると考えています。
5. 高速化と称してなりふり構わずcacheにつっこむ
私は、高速化と称してとにかくキャッシュ（Redis）に突っ込んでいるシステムを運用・保守していたことがありました。そのプロダクトでは、Model.all.select { |m| m.hoge? }というようなコードがあったりして、「それ、SQLでできますよ」案件も多くあったのですが、それ以前にリポジトリにコミットしたyamlをloadし、yamlの中を総なめしないと欲しい情報が得られないなど、「それ、リレーショナルデータベースにできますよ」案件もありました。改修をしたかったのですが、システムのコアな部分であり、修正しょうと思うと多岐に影響するので、なかなか直せない状態になっていました。情報量も多くなってきて処理に時間がかかるためか、苦肉の策として、当該部分をキャッシュし、2回目以降の処理を呼び出さないようにされてありました。そうした箇所が幾重にも存在しました。
リリースによっては、yamlの情報を追加するものも含まれるため、その度ごとにキャッシュをクリアしなければなりませんでした。さらに困ったことに、キャッシュキーがバラバラで、関係箇所だけキャッシュをクリアしたくてもできず、関係ない箇所までキャッシュをクリアしなければいけないという有様でした。次第に、キャッシュクリア後の処理が重くなり、リリース後のページロードが遅くなったり、一時的に高負荷な状態になり、終いにはページが落ちるという状況にまでなってしまいました。
キャッシュは、利用するのは簡単ですが、システム内で跋扈すると手に負えないものになってしまいます。私が経験したケースでは、キャッシュができる前に、一度のアクセスで同じキャッシュを複数回呼びに行き、ネットワーク帯域に負荷が高まってしまった場合や、キャッシュのキーが順列組み合わせで保存されるので、同じ情報がその時の順序によって別のものとして登録されてしまい、Redisの容量を食いつぶして行ったことがありました。
Martin Fowlerは、コンピューターサイエンスの中の困難なことの二つに、キャッシュの無効化を挙げています。「キャッシュのご利用は計画的に」ということなんだと思います。2