チーム開発でリファクタリングをすすめるために試してみたこと

# はじめに

ここ一年くらいはなんとなく技術面でのリーダーっぽい役回りをしていたので、チーム内でプロセスや開発のやり方を改善するためにやってきたことをまとめてみました。チームビルディングの文脈での改善も、チームの一員としてやったりもしたので、技術面以外のも含めたまとめになります。

# チームの状況について

* エンジニアが5人くらい
* プロダクトマネージャー的な人もいる
* それぞれが異なる機能や改修をやることが多い
* リモートの人とオフィスに居る人、業務委託の人と社員の人と、いろいろな形態が混在している
* リリースの頻度は月に1~2回
* KPTとFun Done Learnを月1回ほどで実施

# チーム内のコミュニケーションの改善

チーム内で特定のグループ（同じ会社の業務委託メンバーなど）で絞らない限りは、全員が同じオフィスにいことはない状況です。基本的にはチームの仕事に関わるミーティングはZoomです。

## Zoomミーティングでの挨拶

私個人の主観のせいかもしれませんが、チームに入ったばかりのころは、なんとなく暗い雰囲気でした。
毎朝、進捗確認のミーティングをしてますが、司会役の人しかほとんど話していない（他の人達はほぼミュートのまま・画像もオフ）、コミュニケーションが一方通行になりがち、という印象でした。

なので、初心にかえって、開始・終了時になにかしら挨拶をいれるようにしました。
開始時は、「おはようございます」「おつかれさまです」、終了時は「ありがとうございました」「よろしくお願いします」などです。特に、終了時は司会役の人が、「よろしくお願いします」とかいったりするので、そういうときは、なるべく同じ言葉を返すようにしました。返報性の原理を露骨に狙いました。

その後、明るい雰囲気の人が増えたり、開始と終了時の挨拶がなんとなく他のメンバーに浸透したのか、朝のミーティングの雰囲気は変わったように思います。

仕事上のコミュニケーションが成立しないなど、仕事に具体的な影響がでるまでにはなっていなかったものの、意見交換や相談がしづらい雰囲気にはなっていた気がします。リモートワークの場合、挨拶という最低限のコミュニケーションをまず成立させるのが、意外と大事だなと感じた出来事です。


## よく聞かれること・繰り返されそうなことをドキュメンテーション

confluenceでドキュメントをつくる体制になっています。
新しい利用者を獲得するために、システムの制約（一度にアップロードできるファイルサイズなど）を説明することがあるので、そういった場合はFAQのページをつくったりしました。

また、機能の追加や、新しいユーザを追加するためなど、運用や開発時にある程度繰り返されそうな作業については、まずはたたき台の手順書のページをつくるようにしています。

## チーム内でのレビュー

一人一案件みたいなタスクの割当をしているのと、レビューはマネジャー向けがありました。
そのせいか、チーム内で検討や相談するということが、（ゼロではないが、）仕組み化されてませんでした。

私のタスクで、何回チーム内でレビューするミーティングを設けました。私自身は得るものがありましたが、他のメンバーでこれを真似する人がでてないのが、今後の課題です。

# コードレベルの改善

## IDEの設定の共有

フォーマッターやcopyrightなどの設定は、ドキュメンテーションされてチーム内で共有されていました。
ただ、不要なimport文、未使用な変数がそのまま残っているのが、結構ありました。
フォーマッターも、メソッドチェーンの改行が消されるなどの、若干の問題を抱えていました。

* フォーマッターの設定の見直しと共有
* Save Actionsの設定の共有

不要import文の整理は、IDEのSave Actionsの機能が便利です。
ファイル保存時に、import文の並び替え、不要なものの削除、未使用変数、未使用privateメソッドの削除、末尾のスペース削除など、ちょっとしたコードの整理をやってくれます。

https://qiita.com/jabaraster/items/96a61e768b3d51909166

また、静的解析のプラグインもいれると、書きながら静的解析のフィードバックを得られるようになります。
Javaであれば、

* sonarlint
* spotbugs
* pmd
* cpd

をあたりをいれれば、十分です。
プロジェクトの途中からいれる場合は、IDEで表示させるのは、ハイレベルのものだけに絞ると、管理しやすいです。


## パッケージ構成の見直し

フレームワークの機能単位から、業務単位でパッケージ構成に、段階的に移行するしようとしています。
いわゆる軽量DDD的なアプローチです。

```:befoe
├── controller
│   ├── OrderController.java
│   └── UserController.java
├── dto
│   ├── OrderDTO.java
│   └── UserDTO.java
├── entity
│   ├── Order.java
│   └── User.java
└── repository
    ├── OrderRepository.java
    └── UserRepository.java
```

実装している機能の数が少なければ、フレームワークの機能単位でも、理解するのにかかる時間はさほどかからないと思います。
ただ、機能が増えてくると、改修するとなったときに、改修が必要なコードを特定するのが難しくなってきます。
また、特定の機能のためだけのクラスをつくろうとすると、フレームワーク単位の構成では粒度があわず、置き場所に困ってきます。その結果、controllerやentity、場合によってはutilクラスに実装されて、それらがファットになる傾向があります。

そういった問題を、比較的低コスト、段階的に導入できる、かつ、実装側の都合だけで解消するために、下記のような構成に見直しました。

```:after
└── domain
    ├── order
    │   ├── Order.java
    │   ├── OrderController.java
    │   ├── OrderDTO.java
    │   └── OrderRepository.java
    └── user
        ├── User.java
        ├── UserController.java
        ├── UserDTO.java
        └── UserRepository.java
```


## CI/CDの設定

Jenkinsを使って、jar作成+アプリケーションサーバにdeployをやっています。
ビルドツールはgradleです。

JenkinsサーバのOSのバージョンが古いせいで、JUnit5を実行するためのライブラリが、SSL通信の問題でダウンロードができず、テストコード実行が除外されていました...

* jenkins用のサーバを新規作成 + テスト実行を復活
* build.gradleで、テストカバレッジのしきい値の設定追加
    * https://stackoverflow.com/questions/35540823/minimum-code-coverage-threshold-in-jacoco-gradle
* spotbugs,pmdなどの静的解析のgradleタスクを追加
    * https://github.com/spotbugs/spotbugs-gradle-plugin
    * https://docs.gradle.org/current/userguide/pmd_plugin.html 
* `gradle build` に、テスト実行+レポートファイル作成+カバレッジのしきい値チェック+静的解析を含める 
* テストカバレッジ・静的解析の結果をレポートするjenkins pluginを追加
    * https://plugins.jenkins.io/code-coverage-api/
    * https://plugins.jenkins.io/warnings-ng/
* Teamsにジョブの開始・成否や、カバレッジを通知

これによって、

* テストカバレッジは現状維持がほぼ自動で担保
    * 下回るとビルドが失敗し、デプロイできなくなる
    * build.gradleのしきい値の設定を下げれば、コードレビューで気づく
    * チャットに、カバレッジやビルドの失敗が通知されるので、そこでも気づく
* Jenkinsジョブに結果がたまるので、前回と比べて良くなったかどうかがわかる
* localでgradle buildすれば、テストカバレッジや静的解析の結果を確認できる
    * リポジトリにpushして、CI/CDのためのジョブを実行しなくても、コードの品質を改善するためのサイクルをlocalだけでもある程度まわせる

と、ある程度、コードの品質やデプロイについて可視化されるようになりました。
gradleタスク+IDEの補助で、静的解析のチェックと改善の最低限のサイクルを、localでもまわせるようにしました。


## テストコードの書き方のガイドライン策定

プロジェクトの途中からテストコードを導入していました。
新規や改修時に、その部分に対してはテストコードをいれるという形でやっていました。

* controllerクラスばかりにテストが書かれている
* assertionが業務ロジックとは関係がすくなく、ほぼカバレッジのためだけになっている
    * カバレッジ率をあげようとすると、ファットコントローラーを助長しかねない
* 階層構造がない
* テストメソッドの名前に統一感もなく、なにをassertionしているかもよくわからない
* パラメタのパターンの網羅をしていないか、どこまでできているかがよくわからない

といった問題がみうけられました。
これを解消するために、

* 参考にできるテストコードを作成
* テストクラスに大枠の構造を説明したドキュメントの作成
    * テストメソッドの命名規則などの策定
* 上記を共有するミーティングを開催

を実施しました。
それらの内容をまとめたのがこちらの記事になります。

https://qiita.com/n_slender/items/fb4c32716902baac36a1

https://qiita.com/n_slender/items/6f5082d4a9862d611ff8

## mutation testの導入

[pitest](https://pitest.org/) を導入しました。
単体テストコードの質を図るための手法のひとつで、ざっくりいうと、「productionコードにバグを仕込んだときに、failするテストコードがあるかどうか」という考え方です。
現在ある単体テストコードの質を客観的な指標でみるために、試験的に導入してみました。

これの[gradle plugin](https://gradle-pitest-plugin.solidsoft.info/) をいれただけなので、自動で定期的に実行まではいけていないです。実行にかなり時間がかかるのと、マシンリソースをくってしまうためで、実行頻度を決めかねています。リリース後やmasterブランチにマージするくらいでできればと考えています。

## Field InjectionからImplicit Constructor Injectionへの移行

Spring Framework固有のトピックです。
随分前から、Field Injectionは非推奨になり、constructor injectionも、コンストラクタに`@Autowired`なしでもされるようになっています。

* https://pppurple.hatenablog.com/entry/2016/12/29/233141
* https://spring.io/blog/2016/03/04/core-container-refinements-in-spring-framework-4-3

私が入ったときは、Field Injectionしかなかった上に、injectionされている数もやたら多いという、テスト容易性や安全性を考えると、ちょっと悩ましい状況でした。

```java:before
// Field Injection
@Controller
public class SampleController;

  @Autowired
  private ServiceA serviceA;

  @Autowired
  private ServiceB serviceB;

  @Autowired
  private ServiceC serviceC;

```

こういったField Injectionを以下のように書き換える or　新規のコードはこのやり方にする、という方針転換をしました。

```java:after
@Controller
public SampleController {

  private final ServiceA serviceA;

  private final ServiceB serviceB;

  private final ServiceC serviceC;

  SampleController(ServiceA serviceA, ServiceB serviceB, ServiceC serviceC) {
      this.serviceA = serviceA;
      this.serviceB = serviceB;
      this.serviceC = serviceC;
  }

}
```

単純なコード量はField Injectionの方が少ないのですが、immutabilityと依存性注入しやすさのバランスがとれるので、Constructor Injectionの方が、やはり無難です。Constructor Injectionの形にしておけば、DIフレームワークに頼らずとも、やろうと思えば依存性を注入できるので、言語やフレームワークによらず、採用できるやり方です。

## First collection classパターンの適用

対症療法として採用してしまったのですが、既存のコードで、ListからStream APIで、絞り込んだリストをとったり、特定のものがあるかどうかのフラグに変換で、本質的に同じものがいろんな箇所にでているのがありました。根本的な見直しなしに、それらのロジックを一箇所にまとめるには、First collection classパターンが使えそうということで、導入してみました。

First collection classパターン自体は、こちらの本から知りました。

https://www.amazon.co.jp/%E7%8F%BE%E5%A0%B4%E3%81%A7%E5%BD%B9%E7%AB%8B%E3%81%A4%E3%82%B7%E3%82%B9%E3%83%86%E3%83%A0%E8%A8%AD%E8%A8%88%E3%81%AE%E5%8E%9F%E5%89%87-%E3%80%9C%E5%A4%89%E6%9B%B4%E3%82%92%E6%A5%BD%E3%81%A7%E5%AE%89%E5%85%A8%E3%81%AB%E3%81%99%E3%82%8B%E3%82%AA%E3%83%96%E3%82%B8%E3%82%A7%E3%82%AF%E3%83%88%E6%8C%87%E5%90%91%E3%81%AE%E5%AE%9F%E8%B7%B5%E6%8A%80%E6%B3%95-%E5%A2%97%E7%94%B0-%E4%BA%A8-ebook/dp/B073GSDBGT/ref=sr_1_5?adgrpid=135146062523&gclid=Cj0KCQiAtICdBhCLARIsALUBFcHwdi_qrHh0uiWIrZzVmWECASCh3liee_hskXSs9oEQKvAr3RFRJ6YaAuRrEALw_wcB&hvadid=611407255124&hvdev=c&hvlocphy=1009298&hvnetw=g&hvqmt=e&hvrand=10861622522858683697&hvtargid=kwd-1653336597295&hydadcr=26043_13550199&jp-ad-ap=0&keywords=%E8%89%AF%E3%81%84%E8%A8%AD%E8%A8%88+%E6%82%AA%E3%81%84%E8%A8%AD%E8%A8%88&qid=1671459267&sr=8-5

https://www.amazon.co.jp/%E8%89%AF%E3%81%84%E3%82%B3%E3%83%BC%E3%83%89%EF%BC%8F%E6%82%AA%E3%81%84%E3%82%B3%E3%83%BC%E3%83%89%E3%81%A7%E5%AD%A6%E3%81%B6%E8%A8%AD%E8%A8%88%E5%85%A5%E9%96%80%E2%80%95%E4%BF%9D%E5%AE%88%E3%81%97%E3%82%84%E3%81%99%E3%81%84-%E6%88%90%E9%95%B7%E3%81%97%E7%B6%9A%E3%81%91%E3%82%8B%E3%82%B3%E3%83%BC%E3%83%89%E3%81%AE%E6%9B%B8%E3%81%8D%E6%96%B9-%E4%BB%99%E5%A1%B2-%E5%A4%A7%E4%B9%9F-ebook/dp/B09Y1MWK9N/ref=sr_1_1?adgrpid=135146062523&gclid=Cj0KCQiAtICdBhCLARIsALUBFcHwdi_qrHh0uiWIrZzVmWECASCh3liee_hskXSs9oEQKvAr3RFRJ6YaAuRrEALw_wcB&hvadid=611407255124&hvdev=c&hvlocphy=1009298&hvnetw=g&hvqmt=e&hvrand=10861622522858683697&hvtargid=kwd-1653336597295&hydadcr=26043_13550199&jp-ad-ap=0&keywords=%E8%89%AF%E3%81%84%E8%A8%AD%E8%A8%88+%E6%82%AA%E3%81%84%E8%A8%AD%E8%A8%88&qid=1671459267&sr=8-1


# まとめ

これらの改善をはじめて、まだ数ヶ月というところです。
地味な改善をすこしずつ積み上げているという正直なところで、チーム内の合意をとりながらとなると、改善が反映されるまでにも時間がかかります。まだまだ継続が必要なのと、目に見えた結果がでるにはやはり時間がかかると思っています。