SIerで仕事やっていてよく見るコードと直し方 - デメテルの法則違反


仕事やっていてよく見るアンチパターンをまとめていこうと思ってます。今回はデメテルの法則違反です。
コードはScalaですが、RubyやPythonやJavaでも基本同じです。

トピックは以下。

* 違反したコード
* とりあえず直す
* とりあえず直したあとのテスト

## デメテルの法則に違反したコード

デメテルの法則とは、「直接インスタンス化したもの」か「引数として渡されたもの」以外のものを使ってはいけないという法則です。

```scala
class Profile(name: String, age: Int) {
  def showProfile(): Unit = println(s"my name is $name (age: $age)")
}

class Applicant(id: String, val profile: Profile)

object Order {
  // デメテルの法則に違反している
  def showApplicant(applicant: Applicant): Unit = {
    applicant.profile.showProfile()
  }
}
```
引数で`applicant`が渡されているけど、メソッド内では、
`applicant.profile`で`profile`を取得してから`showProfile()`メソッドを実行しているので違反しています。


`applicant.profile.showProfile()`がやりたいがために、
`class Applicant(id: String, val profile: Profile)`のように`val`をつけて**ゲッターでプロパティを公開**するのはいったん躊躇したほうがいいと思う。

呼び出したい形としては、以下。

```scala
  def showApplicant(applicant: Applicant): Unit = {
    applicant.showProfile()
  }
```

ここでは、以下について記載している。

* 継承による解決方法
* 転送メソッド(委譲)による解決法


## 継承による解決方法

```scala
class Profile(name: String, age: Int) {
  def showProfile(): Unit = println(s"my name is $name (age: $age)")
}

// ApplicantがProfileを継承する
class Applicant(id: String, name: String, age: Int) extends Profile(name, age)

object Order {
  def showApplicant(applicant: Applicant): Unit = {
    applicant.showProfile()
  }
}
```

↑だといろいろ違和感あるので、転送メソッドで修正する。


## 転送メソッド(委譲)による解決方法

```scala
class Profile(name: String, age: Int) {
  def showProfile(): Unit = println(s"my name is $name (age: $age)")
}

// profileはゲッターで公開する必要がなくなったので
// valを外した。
class Applicant(id: String, profile: Profile) {
  // 転送メソッド
  def showProfile(): Unit = profile.showProfile()
}

object Order {
  def showApplicant(applicant: Applicant): Unit = {
    applicant.showProfile()
  }
}
```

## テストついて

ここから、**転送メソッド**で修正した`Order.showApplicant()`メソッドのテストについて考える。


```scala
object SimpleTest extends App {
  // 必要なオブジェクトを作る
  val profile = new Profile("taro", 22)
  val applicant = new Applicant("001", profile)
  
  // テスト対象のメソッドを実行する
  Order.showApplicant(applicant)
}
```

上の例だと、`Applicant`インスタンスの生成は簡単だけど、例えば、下のように、`Applicant`が様々なクラスを集約していてインスタンス生成が億劫な場合を考える。

大事なことは、**テスト対象が`Order.showApplicant()`メソッドであり、転送メソッドなので、呼んだことを確認できればいい。ということ**。


```scala
object ComplexTest extends App {
  // インスタンスの生成が複雑
  val profile1 = ...
  val profile2 = ...
  val profile3 = ...
  val applicant = new Applicant("001", profile1, profile2, profile3, ...)

  // テスト対象のメソッドを実行したいだけだが、
  // インスタンス生成が億劫。
  Order.showApplicant(applicant)
}
```

## テストを簡単にできるようにするためにリファクタリング

`Order.showApplicant()`メソッドを実行出来ればいいので、引数の`applicant`の中身は何でもよいものとして考える。  
なので、インタフェースを挟み、`Order.showApplicant()`が抽象に依存するようにする。

```scala
// 抽象化しておく
trait IApplicant {
  def showProfile(): Unit
}

class Applicant(id: String, profile: Profile) extends IApplicant {
  def showProfile(): Unit = profile.showProfile()
}

object Order {
  // applicantの型をIApplicantとして抽象に依存するようにした。
  def showApplicant(applicant: IApplicant): Unit = {
    applicant.showProfile()
  }
}
```

`Order.showApplicant()`メソッドの引数に必要な`applicant`は以下のように生成できるので、テストも多少は楽になる。

```scala
object RefactorComplexTest extends App {
  // インスタンスの生成が複雑
  val applicant = new IApplicant {
    override def showProfile(): Unit = println("てきとう。てきとう。てきとう")
  }
  
  Order.showApplicant(applicant)
}
```

以下のようにデメテルの法則に違反したままだと、テストがやりづらいので、デメテルの法則違反があったら直したほうがいいと個人的には思う次第です。

モックを使うなどすればテストはできますが、モックを使う前に設計を見直すのがいいと 個人的には思う次第です。

```scala
  def showApplicant(applicant: Applicant): Unit = {
    // デメテルの法則違反したままだとテストがやりづらい
    applicant.profile.showProfile()
  }
```