『オブジェクト指向設計実践ガイド』 実践編

はじめに

この記事は『オブジェクト指向設計実践ガイド』の内容をもとにテストコード書いてたらよくわかんなくなったので助けてくれ、という内容です。だれか助けてください。コメントとかRe:記事とかのリアクションあると嬉しいです!

本の内容の抜粋

詳細は買って読んでね、ということでざっくりと。

www.amazon.co.jp

  • Dependency Injectionでテストしやすくしよう」
    • 依存オブジェクトの注入*1ってやつだよ
    • サンプルコード参照
  • 「送信コマンドメッセージはテストダブルを使うとよい」
    • コマンド: 副作用がある
    • 送信メッセージ: テスト対象のメソッドから呼ばれてる他のメソッド
    • 要は副作用があるなら副作用を呼び出せてるかはチェックすべきってこと

サンプルコード

サンプルコードの数値リテラルは適当な値なのでどんなギアだよって感じになってます。すみません。

# DependencyInjection 適用前
class Wheel
  attr_reader :rim, :tire
  def initialize(rim, tire)
    @rim = rim
    @tire = tire
  end
  
  def diameter
    rim * (tire * 2)
  end
end

class Gear
  attr_reader :ratio, :rim, :tire
  def initialize(ratio: nil, rim: nil, tire: nil)
    @ratio = ratio
    @rim = rim
    @tire = tire
  end

  def gear_inches
    ratio * Wheel.new(rim, tire).diameter
  end
end

Gear.new(ratio: 1, rim: 3, tire: 2) # 値はてけとー
# DependencyInjection 適用後
class Gear
  attr_reader :ratio, :wheel
  def initialize(ratio: nil, wheel: nil)
    @ratio = ratio
    @wheel = wheel
  end

  def gear_inches
    ratio * wheel.diameter
  end
end

Gear.new(ratio: 1, wheel: Wheel.new(3, 2)) # ここでDependency = Wheelを注入!

んで、これをやっておくとテストで Wheel クラスを参照しなくて済みます。『参照しなくて済む』というのは『GearのテストがWheelに依存しな』くなるということです。

# 旧
class GearTest < Minitest::Test
  # 受信メッセージのテスト
  def test_calculates_gear_iniches
    gear = Gear.new(ratio: 1, rim: 3, tire: 2)
    assert_equal(12, gear.gear_inches) # gear_inchesの中でWheel#diameterに依存
  end
end

# 新
class DiameterDouble # テストダブルの導入
  def diameter
    6 # 固定値で返すとバグらなくてあんしん
  end
end

class GearTest < Minitest::Test
  def test_calculates_gear_iniches
    gear = Gear.new(ratio: 1, wheel: DiameterDouble.new) # WheelじゃなくてDouble使う
    assert_equal(12, gear.gear_inches)
    # 実行してもWheelはどこにも必要とされてない = 依存していない ので変更につよい
  end
end

ほんとはこれ以降もダブルをどう扱うかとかが続くんですけどこのへんで。

実践してみた

タイトルが実践入門ですからね、実践していきます。
雰囲気を出すためにそれっぽいコードを書きます。記事をファイルに保存するような処理を考えましょう。
もらった文字列の1行目をタイトルとし、他を本文としてファイルに書き出す処理です。

vs クラスメソッドへの依存

DI前のコードがこちら。

module FileCreator
  def self.create(title, text)
    # いろんな処理とか
    File.write(title, text)
  end
end

class Article
  attr_reader :title, :body
  def initialize(body)
    @title, @body = body.split("\n", 2)
  end
  
  def save
    FileCreator.create(title, body)
  end
end

Article#save は明らかに FileCreator.craete に依存していますね。DIしましょう*2

DIする

DI版にしてみたのがこちら。

class Article
  attr_reader :title, :body, :creator
  def initialize(body, creator: FileCreator)
    @title, @body = body.split("\n", 2)
    @creator = creator
  end
  
  def save
    creator.create(title, body)
  end
end

でもこのコード、2点気になるところが。

  • creatorの値をほぼ変える予定がない場合にオプション引数取れる実装は、今後の変更に備えすぎてるように見える
    • KISSとかYAGNIとかそういう感じで
    • 変更する予定ないくらいに密結合になってるならテストでもそのままでいいような気がする
      • でも副作用は気になる……
    • 拡張性じゃなくてテスタビリティのための実装だと思えば仕方ない気もする……。
  • クラスをインスタンス変数にとるの、結構キモくないですか?
    • 「クラスもインスタンスもオブジェクトだよ。過度に区別すんな」という趣旨のことは同書の別の場面で言ってた気がする*3
      • あくまで「キモい」という感覚であって具体的に何が悪いのと言われるとよくわからない
        • Railsみたいな定数リロードがあると古いの残ったりはしそう
        • プロダクションでの実害は特にないと思う

DIはしないけど継承 + オーバーライドでなんとかしてみる

あるいはこういう感じにメソッドにしてみるとか。

class Article
  attr_reader :title, :body
  def initialize(body)
    @title, @body = body.split("\n", 2)
  end
  
  def save
    creator.create(title, body)
  end
  
  def creator
    FileCreator
  end
end

# テスト時にはオーバーライドしてどうにかする

class ArticleDouble < Article
  def creator
    Class.new { def self.create; end }
  end
end

うーん、まあ、ギリギリ……?テスト用のクラスができちゃうの、あんまり好きじゃないけど……。

スタブ使う

違うアプローチとして、アプリケーションコードをDIでよくするんじゃなくてスタブ使ってみるとか。

class ArticleTest < Minitest::Test
  def test_save
    article = Article.new(<<~EOS)
    たいとるでーす
    本文でーす
    2行目でーす
    EOS
    expected_args = [
      'たいとるでーす', 
      "本文でーす\n2行目でーす\n"
    ]
    creator = MiniTest::Mock.new.expect(:call, nil, expected_args)
    FileCreator.stub(:create, creator) do
      article.save
    end
    assert(creator.verify)
  end
end

これがすっきりしてていいけど、テスト側に思いっきり FileCreator って書かれてるのがうーん……。Dependency減らしたいんじゃなかったっけ?

まとまらないまとめ

  • クラス同士の依存が明らかな場合、スタブがよさそう
    • つまりFileCreator以外のcreatorが存在しそうにないとき
    • テストコードにクラス名がおもいっきり書かれるけど増えるかわかんないんだしいいんじゃないの
  • creatorが多数存在するとき
    • DIにするのがよさそう……なのか?
    • BaseArticleとFileArticleとDatabaseArticleと、みたいにして #creator をそれぞれ実装しちゃうのがよさそう
      • テストはArticleDoubleを作ってそこにスタブいれちゃう
      • オーバーライド版 + スタブ版って感じ
      • テスト用のクラスができちゃうのは仕方ない

あるいはなんかいい方法があるんでしょうか?実際のアプリケーションの成熟具合とも関連すると思いますがご意見お待ちしております 🙋


2017/02/06 04:24追記

クラスオブジェクトをインスタンス変数にもちたくない問題がー

これたぶん基本的には問題なくてArticleのインスタンスはFileCreatorよりも寿命が短いことが想定されるから。
DIにするのもそんなに高コストではないのでDI化しておいていいんじゃないのってのがいまの結論です

*1:依存性の注入って訳は好きじゃない

*2:書き終わってからFileCreator.createがtitleとbody取るっていうArticleに特化したものなのに名前が汎用的すぎるのでは、と思ったけど直してません

*3:UMLとかのあたりだったっけ?