Railsのワナ: モデルでbooleanメソッドをオーバーライドするな(翻訳)

概要

原著者の許諾を得て翻訳・公開いたします。

Railsのワナ: モデルでbooleanメソッドをオーバーライドするな(翻訳)

ActiveRecordで実に有用な機能のひとつは、指定のテーブルの属性(attribute)リーダーやライターをすべてのカラムで自動的に定義してくれる点です。ただし、その中でboolean型のものについては、さらに?付きのエイリアスまで定義されるのです。このメソッドをオーバーライドして、指定された条件に合わせていくばくかの要求を追加するのが有用な場合もあるかもしれませんが、あまりよい考えとは言えないのではないでしょうか。

問題の分析

アプリの開発中、管理パネルからユーザーを有効にしたり無効にしたりできるとしましょう。しかしこのアプリは有償であり、アプリにアクセスしたいユーザーはサブスクリプションを購入する必要があります。このような状況で、そのユーザーが今有効であるかをチェックするのであれば、User#active?メソッドをオーバーライドして、サブスクリプションにまつわる要求をいくつか追加することは一応可能です。

# app/models/users.rb
class User < ApplicationRecord
  def active?
    super && valid_subscription?
  end

  def valid_subscription?
    # サブスクリプションが有効かどうかをここでチェック
  end
end

ここでは、ActiveRecordがbooleanカラムのエイリアスを定義して、元のカラム名の末尾に?を追加することを利用しています。つまり、activeというbooleanカラムがあれば、active?メソッドが定義されていてactiveと同じように動作するであろうことを期待しているわけです。

ほほう、ではユーザーが完全に有効かどうかをチェックするために、User#active?をあちこちで呼び出したとしましょう。次にユーザーをAPIで公開することを要求されたとしましょう。別に難しくありません。jsonapi-serializers gemを追加してJSONAPI互換のシリアライザを実装すればよいのですから。そして今度は、あるユーザーが有効かどうかをもAPIで公開する必要が生じたとしましょう。シリアライザは次のようになります。

# app/serializers/user.rb
class UserSerializer
  include JSONAPI::Serializer

  attribute :active
  # 他にも属性がある
end

もうこれで仕事は終わった感ありますね。しかし実は、何ともいやらしいバグがここに隠れています。シリアライザはUser#active?ではなくUser#activeの値を返すのです!

どこがまずかったのか

主にまずかったのは、命名がいい加減だったことと、ドメイン概念を正しく導入していないことです。ActiveRecordはこの問題をむしろ後押ししています。activeというカラム名に応じてactive?というメソッドが定義されるので、ここで必要なのは下の機能をオーバーライドして条件を少し加えるだけで済みました。しかし、booleanメソッドをオーバーライドしてもいいことはありません。これが行われているということは、何らかの概念を見落としているか、オーバーライドがコードに埋もれて見えなくなっているかのどちらかが起きているに決まっています。

この問題の解決方法

方法のひとつは、単にこのドメインの概念を明確に示すことでしょう。User#active?メソッドはそのユーザーが有効かどうかをチェックしているのではなく、ユーザーがアプリにアクセスできるかどうかをチェックするものです。つまり、おそらくUser#can_access_application?の方がよりふさわしい名前です。

この機能が追加されると、それに関連して別の機能も追加されるというのはままある話です。たとえば「そのユーザーは有効だがアプリにはアクセスできない」とか、単にactiveフラグ自体をチェックしたいなどです。最終的なモデルは次のようになるでしょう。

# app/models/users.rb
class User < ApplicationRecord
  def can_access_application?
    active? && valid_subscription?
  end

  def cannot_access_application?
    !can_access_application?
  end

  # 他のメソッド
end

シリアライザも更新しないといけませんね。

# app/serializers/user.rb
class UserSerializer
  include JSONAPI::Serializer

  attribute :active do
    object.active
  end

  attribute :can_access_application do
    object.can_access_application?
  end

  attribute :cannot_access_application do
    object.cannot_access_application?
  end
end

「こんな修正は不要だ」「このバグはアプリ開発者のせいだ」「モデルのメソッドがオーバーライドされていないかチェックしてシリアライザを調整しとけばよかったんだ」という反論があるかもしれません。それもある意味正しいでしょう。しかしこのコードがproductionにデプロイされたとしたら、この潜在的な問題をレビュアーが見落としたということになるのですから、この問題の特定が難しいとは思いませんか?ActiveRecordはあらゆるbooleanメソッドにエイリアスを追加するので、User#activeUser#active?の結果は同じであると考えてしまうのも無理はないでしょう。

しかし実際には、私の修正は名前衝突のリスクを最小化したうえに柔軟性も高まり、さらにUser#active?User#can_access_application?の違いが素直に表現されています。修正前の実装では、?付きメソッドでこれをシンプルに実現するのは無理です。

まとめ

命名はコンピュータサイエンスにおける2大難問のひとつであり、たとえコード量が増える羽目になろうとも、常にドメイン概念を明確に表す名前を付けるのがよいと思います。何かが暗黙であるからといって、それが存在しないということは言い切れないからです。ActiveRecordモデルではこの点をいくら注意しても注意しすぎということはありません。このようなモデルでは永続性とドメイン概念がごっちゃになり、今回のようなケースでついつい地雷を踏んでしまいがちです。ActiveRecordが生成するbooleanメソッドはオーバーライドせず、何を行うのかがひと目でわかるよう適切な名前を付けましょう。

関連記事

Ruby/Railsのプロ開発者としての5年間を振り返る(翻訳)

Railsのdefault_scopeは使うな、絶対(翻訳)

デザインも頼めるシステム開発会社をお探しならBPS株式会社までどうぞ 開発エンジニア積極採用中です! Ruby on Rails の開発なら実績豊富なBPS

この記事の著者

hachi8833

Twitter: @hachi8833、GitHub: @hachi8833 コボラー、ITコンサル、ローカライズ業界、Rails開発を経てTechRachoの編集・記事作成を担当。 これまでにRuby on Rails チュートリアル第2版の半分ほど、Railsガイドの初期翻訳ではほぼすべてを翻訳。その後も折に触れてそれぞれ一部を翻訳。 かと思うと、正規表現の粋を尽くした日本語エラーチェックサービス enno.jpを運営。 実は最近Go言語が好き。 仕事に関係ないすっとこブログ「あけてくれ」は2000年頃から多少の中断をはさんで継続、現在はnote.muに移転。

hachi8833の書いた記事

週刊Railsウォッチ

インフラ

ActiveSupport探訪シリーズ