LoginSignup
3
0

More than 3 years have passed since last update.

定数にクラス外から直接アクセスしない方がいい

Last updated at Posted at 2020-08-28

クラスに定義されている定数にクラス外から直接アクセスするのはやめて、仲介メソッドを挟もうという話をします。
この記事はコードレビューのときにポンと渡せるように書いています。

TL;DR

# DO NOT

class Language
  AVAILABLE_LANGUAGES = [:ja, :en]
end

class PreferencesController
  def edit
    @available_languages = Language::AVAILABLE_LANGUAGES
  end
end
# DO

class Language
  AVAILABLE_LANGUAGES = [:ja, :en]

  def self.available_languages
    AVAILABLE_LANGUAGES
  end
end

class PreferencesController
  def edit
    @available_languages = Language.available_languages
  end
end

では例を元にこの理由を見ていきます。
理由は端的に言うと、コード変更に弱く、クラスが持つべき責務が流出しやすいからです。

クラスが持つ定数に直接アクセスした場合

Language というクラスが、使用可能な言語のリストを持っているとしましょう。日本語と英語の2つをサポートしています。このリストをコントローラーで参照して、設定画面に渡すとします。

この時点では何も問題はありません。

# A-1 クラスが持つ定数に直接アクセスした場合

class Language
  AVAILABLE_LANGUAGES = [:ja, :en]
end

class PreferencesController
  def edit
    @available_languages = Language::AVAILABLE_LANGUAGES
  end
end

ここで、サービスがドイツに進出して、そこではドイツ語と英語をサポートすることにしたとしましょう。
このとき、一番ナイーブな変更は以下のように変更することです。

# A-2 要件が増えたとき

class Language
  AVAILABLE_LANGUAGES = [:ja, :en]
  AVAILABLE_LANGUAGES_IN_GERMANY = [:de, :en]
end

class PreferencesController
  def edit
    if current_country == :japan
      @available_languages = Language::AVAILABLE_LANGUAGES
    elsif current_country == :germany
      @available_languages = Language::AVAILABLE_LANGUAGES_IN_GERMANY
    end
  end
end

このコードの何がいけないのでしょうか?
一番の問題は、Language が持つべきビジネスロジック(= 国と使用可能な言語のリストの対応)が利用者側に流出していることです。つまり責務の境界が曖昧になっています。
また、他でも使用可能な言語を知りたい場所が出てきたら毎回この分岐を書く必要が生じます。

メソッド下に隠蔽した場合

今度は定数への直接のアクセスを許さず、メソッドを通して取得するようにしてみます。
この時点では単に冗長なだけのような気もします。

# B-1 メソッド下に隠蔽した場合

class Language
  AVAILABLE_LANGUAGES = [:ja, :en]

  def self.available_languages
    AVAILABLE_LANGUAGES
  end
end

class PreferencesController
  def edit
    @available_languages = Language.available_languages
  end
end

しかし先ほどと同じようにサポートする国が増えたとき、今度は国と言語の対応を Language クラスの中に留めることができます。つまり責務の境界を保つことができます。
また、利用者側は引数を一つ渡すようにするだけで済みます。これなら再利用もしやすいです。

# B-2 要件が増えたとき

class Language
  AVAILABLE_LANGUAGES_IN_JAPAN = [:ja, :en]
  AVAILABLE_LANGUAGES_IN_GERMANY = [:de, :en]

  def self.available_languages(country:)
    if country == :japan
      AVAILABLE_LANGUAGES_IN_JAPAN
    elsif country == :germany
      AVAILABLE_LANGUAGES_IN_GERMANY
    end
  end
end

class PreferencesController
  def edit
    @available_languages = Language.available_languages(country: current_country)
  end
end

ここでこっそり AVAILABLE_LANGUAGESAVAILABLE_LANGUAGES_IN_JAPAN に rename していることにも注目してください。クラス内に定数を隠蔽したからこそ、影響範囲を恐れずに変更ができます。

さらに言うと、クラス内に隠蔽されていればデータ構造も変えやすいです。もちろん利用者側には影響しません。以下の方がコードとしてはすっきりします。

# B-3 データ構造も変えやすい

class Language
  AVAILABLE_LANGUAGES = {
    japan: [:ja, :en],
    germany: [:de, :en],
  }

  def self.available_languages(country:)
    AVAILABLE_LANGUAGES[country]
  end
end

class PreferencesController
  def edit
    @available_languages = Language.available_languages(country: current_country)
  end
end

ここまで見て、鋭い人は実は B-3 のようなデータ構造に変えることは、定数に直接アクセスする形でもできるのではと思うかもしれません。

# A-3 実は定数に直接アクセスする形でもデータ構造の変更はできる (ある程度までは)

class Language
  AVAILABLE_LANGUAGES = {
    japan: [:ja, :en],
    germany: [:de, :en],
  }
end

class PreferencesController
  def edit
    @available_languages = Language::AVAILABLE_LANGUAGES[current_country]
  end
end

ただ、サポートする国が増えることになったときに、A-2 のように変更するか A-3 のように変更するか、あるいは慧眼でもって B-2 / B-3 のようにリファクタするかは、その変更をすることになった開発者次第です。
実際、僕の今の会社のコードベースを見る限り、A-2 のように変更されているケースが一番多いように思います(一応断っておきますが、今の会社のエンジニアのレベルは決して低くはないと思っています)。

一方で、最初からメソッドになっていれば、引数で制御しようというのは比較的自然に思いつくのではないでしょうか。

何が言いたいかというと、大切なのは将来の変更のしかたを導くということです。

(補足) Ruby で定数を private にする方法

Ruby では、デフォルトで定数はクラス外から直接参照できます。したがって、仮に仲介メソッドを提供していたとしても、それに気づかずに定数を直接参照してしまう可能性は防げません。

定数のクラス外からのアクセスを防ぐには private_constant というメソッドを使うことができます。

class Language
  AVAILABLE_LANGUAGES = [:ja, :en]
  private_constant :AVAILABLE_LANGUAGES

  def self.available_languages
    AVAILABLE_LANGUAGES
  end
end

class PreferencesController
  def edit
    @available_languages = Language::AVAILABLE_LANGUAGES
    # => NameError: private constant Language::AVAILABLE_LANGUAGES referenced
  end
end

追記

この記事を同僚に読んでもらって、拡張することがほぼないようなものは定数のまま参照してもいいのではないかという意見をもらいました。
個人的な経験ではビジネスロジックに絡むものはけっこうな確率で後から拡張されることがあるような気がします。一方で、config 系のものは後からランタイムで分岐したくなることはめったにないと思うので定数のままでもよさそうです。

3
0
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
3
0