reek での Repeated Conditional 警告への対処は継承か委譲を用いる

業務で主に Ruby On Rails での開発を行っていますが、そこで得た知見や、失敗への対応などについて記します。

今回は「reek から `Repeated Conditional` の警告を受けた」場合にどのように対処するか、についてです。

## 現状(要件)

[reek](https://rubygems.org/gems/reek) は Rubyプログラムの [コードの臭い](https://ja.wikipedia.org/wiki/コードの臭い) を検知するツールとして、弊社の開発でも活用しています。

その reek が発する警告の一つに [Repeated Conditional](https://github.com/troessner/reek/blob/master/docs/Repeated-Conditional.md) というものがありますが、以下のように「一つのクラス内で、同じ条件判定を繰り返し定義している場合」に起こるものとされています。

```ruby
# https://github.com/troessner/reek/blob/master/docs/Repeated-Conditional.md から引用
class RepeatedConditionals
  attr_accessor :switch

  def repeat_1
    puts "Repeat 1!" if switch
  end

  def repeat_2
    puts "Repeat 2!" if switch
  end

  def repeat_3
    puts "Repeat 3!" if switch
  end
end
```

この警告の理由、および対策について同ページでは以下のように説明しています。
> If you get this warning then you are probably not using the right abstraction or even more probable, missing an additional abstraction.

([DeepL](https://www.deepl.com/translator) による翻訳)
> この警告が表示された場合は、正しい抽象化を使用していないか、さらには追加の抽象化を見逃している可能性があります。

ただ、これだけの説明だと「具体的にはどう解決すれば良いのか?」という疑問が生じます。

そこで、上記の例を「実際に解決する方法」を考えてみます。

## 1. まずは「抽象クラス」を切り出す

説明文には「抽象化を使用していない、または足りない」とあります。

「抽象化」の方法としてすぐに考えられるのは「継承を用いる方法」です。

上述の例の `RepeatedConditionals` クラスに、抽象クラスとして `AbstractConditionals` クラスを追加してみましょう。

```ruby
# まずは単純に抽象クラスを追加します
class AbstractConditionals
  attr_accessor :switch

  def repeat_1
    raise NotImplementedError
  end

  def repeat_2
    raise NotImplementedError
  end

  def repeat_3
    raise NotImplementedError
  end
end

# 抽象クラスを継承した具象クラスとする
class RepeatedConditionals < AbstractConditionals
  def repeat_1
    puts "Repeat 1!" if switch
  end

  def repeat_2
    puts "Repeat 2!" if switch
  end

  def repeat_3
    puts "Repeat 3!" if switch
  end
end
```

後の説明のために「この実装の使い方」も載せます

```ruby
conditionals = RepeatedConditionals.new
conditionals.switch = true
conditionals.repeat_1

conditionals.switch = false
conditionals.repeat_2
```

まだこの時点では reek の `Repeated Conditional` 警告が出されたままです。

ではもう一歩、警告解消に向けて進むにはどうすれば良いでしょうか。

## 2. 具象クラスを(条件結果ごとに)分割する

警告解消のためには、これまでの説明通り「同じ条件判定を繰り返し定義している」のを解消すれば良いのですから、条件判定を「メソッド内」で行わずに「クラス単位」、すなわち「インスタンス生成時点」で行うようにしてみます。

そのためには `switch` が真の場合のクラスと、偽の場合のクラスに分ければ良いことになります。

```ruby
# switch というアクセサ自体が無くなっています
class AbstractConditionals
  def repeat_1
    raise NotImplementedError
  end

  def repeat_2
    raise NotImplementedError
  end

  def repeat_3
    raise NotImplementedError
  end
end

# こちらは「swith が真だった場合の動作」を表すクラスです
# switch が真であるという前提なので、 `if switch` が無くなっています
class SwitchedOnConditionals < AbstractConditionals
  def repeat_1
    puts "Repeat 1!"
  end

  def repeat_2
    puts "Repeat 2!"
  end

  def repeat_3
    puts "Repeat 3!"
  end
end

# こちらは「swith が偽だった場合の動作」を表すクラスです
class SwitchedOffConditionals < AbstractConditionals
  def repeat_1; end

  def repeat_2; end

  def repeat_3; end
end
```

この時点で、reek の `Repeated Conditional` 警告は解消されます。

しかし、この対処の場合、インスタンスを生成する時点で「どちらのクラスを使うのか」を決定する必要が有るので、以下のような使い方になります。

```ruby
switch = true
conditionals = (switch ? SwitchedOnConditionals : SwitchedOffConditionals).new
conditionals.repeat_1

switch = false
conditionals = (switch ? SwitchedOnConditionals : SwitchedOffConditionals).new
conditionals.repeat_2
```

これはベタに言えば「イケてない」感じがヒシヒシとします。

なぜなら、使用する側にとっては「どのクラスからインスタンスを生成したか」が関心事なのではなく、**「`switch` の値に応じて正しい結果が返されるインスタンスである」ことだけが関心事**だと考えられるからです。

## 3. どのクラスのインスタンスが生成されるかを隠蔽する

とすれば、ここで思い出されるのが「GoFによるデザインパターン」の一つである [Factory Method パターン](https://ja.wikipedia.org/wiki/Factory_Method_パターン) です。

このパターンでは、何らかの方法で「具体的にどのクラスのインスタンスが生成されるのか」を隠蔽します。

今回は「生成メソッドの引数に switch 引数を与える」ことで、適切なクラスのインスタンスが生成されるようにします。

```ruby
# 生成メソッドをクラスメソッドで定義するため、Abstract の prefix を取り止め、
# 実際に呼ばれることを目的としたクラス名とする
class Conditionals
  class << self
    def build(switch)
      (switch ? SwitchedOnConditionals : SwitchedOffConditionals).new
    end
  end

  def repeat_1
    raise NotImplementedError
  end

  def repeat_2
    raise NotImplementedError
  end

  def repeat_3
    raise NotImplementedError
  end
end

# 以下はこれまでと同じですが、後述のコードと比較しやすいように再度全体を載せます
class SwitchedOnConditionals < Conditionals
  def repeat_1
    puts "Repeat 1!"
  end

  def repeat_2
    puts "Repeat 2!"
  end

  def repeat_3
    puts "Repeat 3!"
  end
end

class SwitchedOffConditionals < Conditionals
  def repeat_1; end

  def repeat_2; end

  def repeat_3; end
end
```

使い方は以下のようになります。

```ruby
conditionals = Conditionals.build(true)
conditionals.repeat_1

conditionals = Conditionals.build(false)
conditionals.repeat_2
```

随分と簡単になりました。

ちょっと廻り道をしたような形になりましたが、しかし結果的にこれで

* reek の `Repeated Conditional` 警告を解消し、
* かつ、使い方はこれまでとほぼ同じ(switch の値を使うのがインスタンス生成時になっただけ)

という、ほぼ理想に近い状況になったことと思います。

## 4. (さらに考えて)インスタンス生成時に条件判定が出来ない場合は?

これまでの記事では

> インスタンス生成時に条件判定をすることによって、メソッド実行時の条件判定を省ける

ということを書いてきましたが、必ずしも「インスタンス生成時に条件判定が出来る」とは限らないと思います。

この記事の最初の例(`Repeated Conditional` 警告の説明ページの例)に立ち返って考えると、たとえば

```ruby
conditionals = Conditionals.new
conditionals.switch = true
conditionals.repeat_1

conditionals.switch = false
conditionals.repeat_2
```

のように、インスタンスの生成後に条件判定が変わる場合が有ります。これでは「インスタンスの属するクラスを分ける」対処が出来ません。

これの対処として考えられることは「委譲を使う」ことになります。

すなわち、条件判定を要するメソッドを別クラスに分離して、switch の値が切り替わるごとに差し替えてしまうことです。

以下にコード例を示します。

```ruby
class Conditionals
  attr_reader :switch

  def switch=(value)
    @repeater = value ? SwitchedOnRepeater.new : SwitchedOffRepeater.new
    @switch   = value
  end

  # 実際は `activesupport` gem の delegate で実装する方が簡単です
  # ```ruby
  # delegate :repeat_1, to: :repeater
  # ```
  # https://api.rubyonrails.org/classes/Module.html#method-i-delegate
  def repeat_1
    repeater.repeat_1
  end

  def repeat_2
    repeater.repeat_2
  end

  def repeat_3
    repeater.repeat_3
  end

  private

  attr_reader :repeater
end

# 以下は「親クラスが Conditionals になっていない」だけで、これまでの例と同じです
class SwitchedOnRepeater
  def repeat_1
    puts "Repeat 1!"
  end

  def repeat_2
    puts "Repeat 2!"
  end

  def repeat_3
    puts "Repeat 3!"
  end
end

class SwitchedOffRepeater
  def repeat_1; end

  def repeat_2; end

  def repeat_3; end
end
```

## まとめ

reek の `Repeated Conditional` 警告を解消するには

* 具象クラスを作成するなど「継承」を用いるのが良い
* あるいは、その部分を「委譲」するのが良い

といったことを書きました。

他にも reek の警告には「説明を読んでも、どう対処したら良いのかすぐには分からない」ものがありますので、何か解決策が見つかれば、その都度、記していきたいと思います。