インスタンス変数を持つモジュールは有害と考えられます。
背景
Railsはどういうわけか、モジュールやインスタンス変数をあらゆるところで使うことを奨励しています。たとえば、コントローラ、ヘルパー、ビューでインスタンス変数を使うなどです。また、ActiveSupport::Concern
の使用も奨励しています。これは、すべてを巨大な単一のオブジェクトに保存し、その巨大なオブジェクト内のすべてにアクセスできるという考えをさらに強化するものです。
問題点
もちろんこれは開発者にとっては便利なことです。しかし、選択した対象が成長した場合、これにはいくつかの欠点があります。
同じコンテキストにあるものが多すぎて、それらが緊密に結合しているのかいないのか、お互いに依存しているのかしていないのかがわからないのです。複雑さが一段と増すと、コードの追跡も非常に難しくなります。例えば、あるクラスが3つの異なるインスタンス変数を使っていて、それらすべてが3つの異なるモジュールから初期化され、操作されているとします。これらの変数がいつ問題を起こし始めるかを追跡するのは困難です。どのモジュールが突然変数の1つを変更するのかわかりません。すべてが何かに触れる可能性があります。
同様の懸念
多重継承は良くないと言われています。複数のインスタンス変数があちこちに散らばっている複数のモジュールを混在させると、同じイシューに悩まされます。ActiveSupport::Concern
も同様です:懸念事項を専用クラスとコンポジションに置き換えることを検討
似たようなアイデアもあります:デコレータとインターフェイスの分離を使用して、成長しすぎるモデルの問題を解決します。
included
はイシューのすべてを解決しているわけではないことに注意してください。依存関係は定義されていますが、各モジュールが最終的な巨大オブジェクトのインスタンス変数を通じて暗黙的に会話することを許しています。
解決方法
巨大なオブジェクトを複数のオブジェクトに分割し、API、つまり公開メソッドを使って互いに通信するようにします。要するに、継承よりコンポジション。こうすることで、それぞれの小さなオブジェクトは、それぞれの限定された状態、つまりインスタンス変数を持つことになります。インスタンス変数がおかしくなっても、それがその小さなオブジェクトのものであることは明らかです。
明確に定義されたAPIがあれば、物事の結合が少なくなり、デバッグや追跡が非常に簡単になります。また、暗黙の依存関係ではなく、明確な方法で通信するため、他のオブジェクトが使用する拡張性も高くなります。
使用可能な用途
しかし、インスタンス変数をモジュール内で使用することは、同じモジュールにコンテナされている限り、つまり、他のモジュールやオブジェクトがインスタンス変数に触れていない限り、必ずしも悪いことではありません。
特に、1つのインスタンス変数に||=
。これは次のようになります:
module M
def f
@f ||= true
end
end
残念ながら、より複雑なルールを警官にコード化するのは簡単ではないので、私たちは人々の最善の判断に頼っています。警官に簡単に追加できる他の良いパターンが見つかれば、そうすべきです。
この警官を書き換えて無効にしない方法
この警官を無効にできたとしても、それは避けるべきです。いくつかのコードは、簡単な形に書き換えることができます。この方法を考えてみましょう:
module Gitlab
module Emoji
def emoji_unicode_version(name)
@emoji_unicode_versions_by_name ||=
JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json')))
@emoji_unicode_versions_by_name[name]
end
end
end
この方法はすでに自己完結しているので、まったく問題ありません。他のメソッドは@emoji_unicode_versions_by_name
。しかし、||=
だけではないので、まだ警官の気分を害していますし、警官はこれでいいと判断できるほど賢くはありません。
一方、このメソッドを2つに分けることもできます:
module Gitlab
module Emoji
def emoji_unicode_version(name)
emoji_unicode_versions_by_name[name]
end
private
def emoji_unicode_versions_by_name
@emoji_unicode_versions_by_name ||=
JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json')))
end
end
end
これで警官は文句を言いません。
この警官を無効にする方法
同じ行のコードのすぐ後に、無効にするコメントを書いてください:
module M
def violating_method
@f + @g # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
end
複数の行がある場合は、セクションごとに有効・無効を切り替えることもできます:
module M
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def violating_method
@f = 0
@g = 1
@h = 2
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end
ある時点で有効にしないと、それ以下はチェックされないことに注意してください。
今すぐ無視すべきこと
Railsヘルパーとメーラーの仕組み上、インスタンス変数の使用を避けられないかもしれません。そのような場合は、現時点では無視してもよいでしょう。これらのモジュールは他のランダムオブジェクトと共有されていないので、まだある程度分離されています。
ビューのインスタンス変数
インスタンス変数を誰が使っているのか(コントローラから見て)、どこで設定したのか(パーシャルから見て)が簡単にわからないため、データの依存関係を追跡するのが非常に難しくなるからです。
そのため、データの依存関係を追跡するのが非常に難しくなります:
= render 'projects/commits/commit', commit: commit, ref: ref, project: project
そしてパーシャルでは
- ref = local_assigns.fetch(:ref)
- commit = local_assigns.fetch(:commit)
- project = local_assigns.fetch(:project)
こうすることで、これらの値がどこから来たのかが明確になり、インスタンス変数を使うよりもタイプミスチェックができる利点があります。将来的には、パーシャルでのインスタンス変数の使用も禁止すべきです。