【しばらく編集不可モードで運営します】 編集(管理者用) | 差分 | 新規作成 | 一覧 | RSS | FrontPage | 検索 | 更新履歴

WritingTestableCodeFlaw2DiggingIntoCollaborators - テストできるコードの書きかた::まずいのその2: 深い仲になってしまっている

目次

テストできるコードの書きかた::まずいのその2: 深い仲になってしまっている

この文書について

Misko Hevery

"holder"とか、"context"とか、「全部入り(kitchen sink)」オブジェクト(あるクラスの関連オブジェクトをまとめるために、各種のオブジェクトを保持しておくオブジェクト)を使うのは止めよう。必要なオブジェクトを保持しているオブジェクトを渡すのではなく、本当に必要になるオブジェクトそれだけを引数として渡すようにしよう。あるオブジェクトを持ってくるために、別のオブジェクトに手を伸ばすのは止めよう。必要なオブジェクトを持ってくるために、オブジェクトグラフを辿ってはいけない(これによって、必要なオブジェクトにたどり着くまでの中間のオブジェクト全てと不必要に深い仲になってしまう)。呼び出しチェインを深くする必要はないはずだ。呼び出しチェインの中にピリオドが複数あるなら、以下のだめな例をちゃんと見ておこう。必要なオブジェクトを直接注入しよう。必要なオブジェクトをあちこち探してまわるのは止めよう。

あぶない予兆

なにがまずいの?

うそつきAPI

あなたの作ったAPIが「必要なのはString型のクレジットカード番号だけだよ」と書いてあるにも関わらず、実際には裏でCardProcessor?オブジェクトを探しに行っていたりPaymentGateway?オブジェクトを探しに行っていたりしたとしよう。こうなると、このAPIは本当は何のオブジェクトに依存しているのかが不明瞭になってしまう。API(メソッドのシグネチャやオブジェクトのコンストラクタ)では、関連するオブジェクトとして本当に必要なものだけを依存先として定義しよう。

コードの柔軟性を失わせる

プログラムをどこか変更しなきゃならなくなったときのことを考えよう。ここで、必要なオブジェクトを探して持ってくるのにいろんな「仲買人(Middle Men)」を経由しなきゃならないとしよう。この場合、変更部分にあわせて、仲買人の中にも変更しなきゃならない部分がたくさんできる。そうではなくて、一番欲しいオブジェクトそれだけを、本当に必要な箇所でだけ取得するようにしてみよう。(このプロセスの中で、複数の責務を持ってしまっているがために分割が必要と思われるクラスが見つかることがあると思う。怖がらないで;単一責任原則(Single Responsibility Principle)(あわせて読みたい:http://ja.wikipedia.org/wiki/%E9%96%A2%E5%BF%83%E3%81%AE%E5%88%86%E9%9B%A2)に適合できるように頑張ってみよう。)また、本当に必要なオブジェクトを見つけるためにいろいろ処置をしている間に、関連クラスと深い仲になってしまっている場合があるけれど、この場合はそのクラスをコンパイル時に必要な依存先であるものとしてしまおう。

コードが膨れ上がり、本当に行われていることが何か分かりにくくなる

必要なオブジェクトをあちこち探し回るという中間的なステップを経ることによって、コードがややこしくなる。また、不必要に長くもなる。

テストが難しくなる

contextオブジェクトを引数にとるメソッドをテストしなきゃいけないとなって、そのメソッドを使うんだけど実際にはcontextから何を取ってくるのかぜんぜん推測できず、また気にしなくていい部分がどこなのかも分からないとしよう。典型的にはこのような場合、まず空のcontextを渡してみて、そうするとNull Pointer Exceptionが起こるから、そうしたらcontextに必要そうな値を詰めてもう一回試す、という手順を行うことになる。これをNullPointerException?が発生しなくなるまで続けるわけだ。あわせて読みたい:「デメテルの法則に違反するのは干草の山の中から針を探すようなものだ」 (http://misko.hevery.com/2008/07/18/breaking-the-law-of-demeter-is-like-looking-for-a-needle-in-the-haystack/)

理解してみよう

この症状は「列車事故」あるいは「デメテルの法則」に対する違反としても知られている。(あわせて読みたい:http://ja.wikipedia.org/wiki/%E3%83%87%E3%83%A1%E3%83%86%E3%83%AB%E3%81%AE%E6%B3%95%E5%89%87

違反しているコードの例:

 // これは言語道断(知りたいのは「ユーザーがAdminか」という点だけ)
 getUserManager().getUser(123).getProfile().isAdmin()
 
 // これもよろしくない
 context.getCommonDataStore().find(1234)

直してみよう

モノを探し回るのではなくて、欲しいオブジェクトはコンストラクタやメソッドのパラメタで渡してもらうようにしよう。依存先のオブジェクトをコンストラクタで受け取ることで、オブジェクトをファクトリを使って持ってくる責務をファクトリクラス(典型的な例としてはGUICE)に移すことができる。

    * 直接の友達とだけ会話するようにしよう。
    * 本当に必要になるオブジェクトそのものだけを注入する(渡す)ようにしよう。
    * オブジェクトを見つけ出したり設定したりする責務は、呼び出し側、つまりファクトリやGUICEに移してしまおう

実際のコードによる劇的ビフォアアフター

基本的に、"深い仲になってしまっている"状態は、本当に必要なオブジェクトが手に入っておらず、そのオブジェクトを探し回る必要がある際に発生する。必要なオブジェクトを探すために他のオブジェクトにアクセスし始めたら、これはテストが難しくなり始める糸口だと考えていい。関連するオブジェクトを他のオブジェクトから貰うのではなくて、関連オブジェクトをメソッドやコンストラクタのパラメタに指定して、関連オブジェクトへの依存関係を明示的に示すようにしよう。 (Don't look for things; Ask for things!)

問題: サービスオブジェクトが値オブジェクトの中身を漁っている

(ソースコードは http://misko.hevery.com/code-reviewers-guide/flaw-digging-into-collaborators/ の「Problem: Service Object Digging Around in Value Object」を参照)

この例では計算処理の中にオブジェクトの参照を混ぜ込んでしまっている。このクラスの主たる責務は総額(Amount)に税率(Tax Rate)を掛け算することのはずだ。

解決方法は、処理中で必要となるオブジェクトそのものをメソッドのシグネチャで宣言し、それ以外は要求しないことだ。

オリジナルのコードをテストするには、RPCClientとHttpRequest?のモックを作らなきゃならない。それに、オブジェクトグラフをトラバースできるように作らなきゃいけないから、作ったテストはどうしても実装と密に結合してしまう。 修正したコードだと、モックを作るときにトラバースは気にしなくていい。楽チンだし、コードも変更に強くしやすい。(なお、後述のバージョンでは、Authenticatorのモックを作る方法をとる。この方が楽だし、より疎結合な設計になる)

問題: Service Locatorの不適切な使用によりデメテルの法則に違反してしまう

オブジェクトを作るときは、責務をただ一つだけ持つように作りたい。つまり、他のオブジェクトのService Locatorとしての役割は持たせたくない。 Guiceを使えば、既存のService Locatorは全てなくすことができる。あるメソッドに、自分自身の責務の他にLocatorとしての役割も持たせようとすると、デメテルの法則に違反してしまう。

デメテルの法則に違反している問題と、Service Locatorみたいな役割を持っている問題がくっつくと、それぞれの問題が個別に起こるよりなお悪くなる。 Databaseの特質とは、リファレンスを他のサービスに配布することではなくて、エンティティを永続化された格納先に保存することだ。DatabaseのgetLock()メソッドは取り除いたほうがよい。 Databaseがlockのリファレンスを持つ必要があるとしても、それを他と共有できるようにはしない方がいい。setter/getterのモックを作らなくて済む。

解決法としては、State Based Testingを使う方法とBehavior Based Testingを使う方法がある。 State Based Testingでは、あるメソッドで処理が行われたあとの、オブジェクトの状態をassertする。処理の実装とは関係なく、オブジェクトの状態の期待値のみで結果を示す。 Behavior Based Testingは、System Under Test(SUT)が実行された際の内部の振る舞いを、モックを使ってassertする。どっちを使ってもよいが、ただ、どっちの方法にもいろいろ意見のある人がいるようだ。

問題: "Context"という名前は法則違反を見つける大きなヒント

理論上、コンテキストオブジェクトは良いものだと言われているかもしれないけれど(依存関係を変更する際にシグネチャを修正しなくて済むので)、これはテストがとてもしにくくなる。

コンテキストオブジェクトを使っているコードを修正するには、必要なオブジェクトそのものでコンテキストオブジェクトを置き換えることだ。これで本当の依存関係が明らかになり、どうやればオブジェクトを分割してより良い設計にできるかも分かりやすくなるだろう。

(以下翻訳中)