ソースコードのお掃除

# <i class="fa fa-child"></i> はじめに
SIerからWEB業界の某社に転職して早4ヶ月。
テストコードをほぼ書かずに毎週リリースをしています。(；´∀｀)
テスト書かなきゃなーと思いつつも、テストを書けない。
なぜ？と考えてみるとテストを書きにくいコードになってしまっているためかな？と。

もう年末なのでコードの整理をしましょうというお話になったので、テストを書きにくいコードの課題を洗い出して、テストコードを書く前にどう手を加えていくかを考えてみました。（サーバーサイドのみ）

※まとまりが無いのであとでまた手直しします。

# <i class="fa fa-coffee"></i> 技術的前提事項
- Scala
- Playframework

## <i class="fa fa-users"></i> 出演者(責務)
Playframework はMVCのフレームワークなので基本的にはMVCがそれぞれ分かれていれば良いのですが、テストを書くことを考えるとそれだけでは物足りません。最低限以下の分類が必要でしょう。

- Controller
- Service
- View
- Repository
- Util

以下、「僕はこう思ってます」っていう責務なので、世間一般の理解とは少し違うかもしれません。
違うと思った場合にはコメントでご意見いただけると嬉しいです。

### <i class="fa fa-code"></i> Controller
- ビジネスロジックは書かない
- Actionのメソッドのみとする
- Controller と Service は一対一で定義する

### <i class="fa fa-code"></i> Service
- ビジネスロジックを記述する
- DBへのアクセスは行わない
- implicit request を引数として受けてはダメ。
  - テストがめんどくさくなる。Controllerで処理しましょう。
- ServiceはRequestを作ってどうこうしてテストをすべきではない
  - ServiceのテストにFakeApplicationとかは必要ない。
- Service で共通利用できるものは trait で SubService 的に定義して Service に extend させる

### <i class="fa fa-code"></i> View
json渡して描画させればいい

### <i class="fa fa-code"></i> Repository
DB I/Oなどの永続化処理を記述する
Service と Repository も"出来れば"一対一で定義する

### <i class="fa fa-code"></i> Util
汎用的なもののみを記述する

## よさげな資料
https://terasolunaorg.github.io/guideline/public_review/Overview/ApplicationLayering.html
綺麗にまとまっててすごく良いです。


# <i class="fa fa-frown-o"></i> 課題
1. Controllerが肥大化している。
  - Controllerにビジネスロジックが大量に乗っている。

```scala:E.g.
object Application extends Application {
  val serviceA: XXServiceA = XXServiceAObj
  val serviceB: XXServiceB = XXServiceBObj
}

trait Application extends Controller {
  val serviceA: XXServiceA
  val serviceB: XXServiceB

  def index = Action {
    Ok(views.html.index()).withNewSession
  }

  private def add(a: Int, b:Int) = a + b

  private def save(a: Int) = DBアクセスしてる

  // なんかビジネスロジックが乗ってる

}
```

1. javadoc書いてない
  - コード読めば大体わかるけどやっぱりjavadocはあったほうが便利だよね

1. メソッド名がイケてない
  - 処理内容と若干あってないとコードを追うときに混乱しますね。

1. 一部責務配置が残念
  Serviceが実はRepositoryでしたとか

```scala:XXService.scala
// サービスと言いつつ中身はRepository
trait XXService {

  def save(XXName: String, XXId: String)(implicit s: Session) = {
    XX += XXRow(XXId, XXName, false)
  }

  def delete(XXId: String)(implicit session: Session) = {
    XX.filter(_.XXId === XXId.bind).delete
  }
}
```

1. DIのために考えられた（？）作りが崩壊しているため

Controller部分で例示したように

```scala
val service: Service = ServiceObj
```

でServiceを差しこむような作りになっているが、Serviceの中でServiceを差し込んだり、色んな所で色んなServiceを差し込んでいるので実はServiceが重複していたりする。
テストコードを書く際に気が滅入る。(｡ŏ﹏ŏ)

# <i class="fa fa-graduation-cap"></i> 対策
1. Controllerにビジネスロジックを記載すべきではない。
  Controllerはできるだけシンプルに。
  ビジネスロジックはServiceに集約する。

1. javadoc書いてない
  書こう
  http://qiita.com/maku77/items/6410c67ce95e08d8d1bd

1. メソッド名がイケてない
  リファクタリングしよう
  http://qiita.com/KeithYokoma/items/2193cf79ba76563e3db6
  http://qiita.com/KeithYokoma/items/ee21fec6a3ebb5d1e9a8

1. 一部責務配置が残念
  責務配置を整理しよう
  DDDの概念を学んで活かせるのがいいんだろうけど、DDDは如何せん難しい＞＜
  誰か教えてください

1. DIのために考えられた（？）作りが崩壊しているため
  リファクタリングしよう
  http://qiita.com/pab_tech/items/1c0bdbc8a61949891f1f

# <i class="fa fa-smile-o"></i> 手立て

## <i class="fa fa-user-md"></i> その１
Controllerのダイエットを行います。
ビジネスロジックはServiceに移します。

http://qiita.com/pab_tech/items/1c0bdbc8a61949891f1f
を参考にDIを見直すなどして考えなおす必要があるでしょう。
そして、チームでコードの作りについて考えを共有する必要があるかと。

## <i class="fa fa-user-md"></i> その２
責務配置を整理します。

責務配置は上記した Controller, Service, Repository, Util で十分でしょう。
※テストコードを書く分には、という意味です。

きちんとやるならDDD学んでやるといいかもしれないですね。（やったことないですけど）

## case classはどこに置くか
Service?Repository?Controller?
modelsパッケージ配下にぶち込みましょう。

## requestをどう扱うか
```scala
def something() = Action { implicit request =>
  // something
}
```
↑こいつをserviceに持ち込みたくないのでControllerで変換の処理が必要だけれど変換処理をControllerに直接書くのはイケてない。
じゃあどうしようかという話し合いの結果、controllersパッケージの配下にhelpersパッケージを作成し、そこに変換処理を記述したヘルパーを作成し、Controllerにmix-inさせようという方針にしました。

この辺で何かいい案があればぜひ教えてもらえると嬉しいです。

# <i class="fa fa-user-md"></i> リファクタリング時のPR
コミットごとに修正内容がわかるとレビューしやすいとのアドバイスいただきました。

# 注意点
## ドメインモデル貧血症に陥らないように
処理をひたすらにServiceに記述していくと次はおそらくこの問題にぶち当たることになるかと思います。注意して進めましょう。

http://www.glamenv-septzen.net/view/1286
http://rrf-codex.net/?p=762
http://varmil.hateblo.jp/entry/2015/04/06/113325

## 共通化時
処理の共通化に構造的部分型を使用したくなりますがあまり性能が良くないそうなので、使いドコロには注意しましょう。
http://tech-blog.tsukaby.com/archives/849

# 以上
年末にソースコードの大掃除をして新たな気持ちで新年を迎えましょう。

# 参考
ドメイン層の実装
http://terasolunaorg.github.io/guideline/5.0.1.RELEASE/ja/ImplementationAtEachLayer/DomainLayer.html
http://d.hatena.ne.jp/j5ik2o/20101229/1293642673

DDD
https://www.ogis-ri.co.jp/otc/hiroba/technical/DDDEssence/chap2.html#TOP
https://www.ogis-ri.co.jp/otc/hiroba/technical/DDDEssence/chap1.html
http://www.infoq.com/jp/articles/ddd-in-practice