Scalaのコードレビューサービスを利用してみた話

こんにちは、VGMの岡村です。
前回はZIOというScalaライブラリに関する記事でしたが、今回は自分が所属しているアドテクノロジーDiv（以後アドテク）でのソフトウェアコードに対する取り組みとして、がくぞ (@gakuzzzz)さんにScalaコードをレビューしていただいた話をご紹介させていただきます。

導入の経緯・動機

アドテクではScalaを導入してから5年以上経っているのですが、実は「Scalaにすごく詳しい人」は少数のスペシャリストしかいません。
結果、レビューなどチーム内の動きだけでは改善や品質担保に限界がある状態になっており、四半期毎に「棚卸し」と称してScalaのスペシャリストが全てのGitLabマージリクエストをレビューしてフィードバックすることになっています。
また、そのスペシャリストもDDDやドメインモデリングについてはさほど詳しくなく、現在まで模索を続けている状態です。

Scalaに詳しいメンバーの一人が偉くなって開発の現場から離れつつあることも踏まえ、今後の品質担保やScala改善の動きを活発にさせるためのきっかけが欲しいと考えていました。そんな所にGMから「こんなサービスあるけどどう？」と紹介されたのが、がくぞさんのTech to Value社が提供しているScalaコードレビューサービスでした。

Scala Online Code Review
GitHubやそのClone等を利用したPull Requestベースのコードレビュー、並びにSlackやGitterなどのチャットツールを利用したFAQ
チームでScalaを導入してみたいけれど、メンバーに詳しい人間が居ない！そんな時に実務のコードを通してScalaならではのノウハウを共有いたします。

外部の有識者からレビューやフィードバックがもらえる面白い機会ですし、是非試してみようとさっそくウェブサイトから連絡を取りました。
Web会議などで相談させていただいた結果、四半期毎に実施しているScala棚卸しに参加してレビュー・フィードバックをしていただく形で契約を進めることになりました。

契約から実施までの道のり

私は中途入社なのですが、アドウェイズではNDAを結んだり稟議を通すのも初めてだったので、色々な方に助けられながら何とか進めることができました。
この場を借りて感謝の気持ちを伝えたいと思います。ありがとうございました。

どうやってレビューしてもらうか

アドテクの多くのプロジェクトでは社内に構築されたGitLabを使用して開発、バージョン管理を行っています。
このGitLabにはVPN経由でなければアクセスできないため、社外の方にGitLabのリポジトリを直接見てもらうことができません。
そのため、GitHubのAdways Organizationにリポジトリを作成、GitLabからミラーリングすることでコードを共有することになりました。

残念ながらこの方法ではGitLabのマージリクエストをGitHubのプルリクエストに自動で変換してくれるわけではないため、「マージリクエストに対応するマージコミット」のGitHubでのリンクをがくぞさんに共有してレビューしてもらうことになります。
四半期分のマージリクエストから一覧を作成するのはとても大変です。スクリプト書いて自動化しなきゃ…

実施、そして得られたもの

実際の棚卸しは、自分とがくぞさん、もう一人のScalaスペシャリストの3人でZoomを使って実施しました。
以下が過去4度実施して実際にいただいたフィードバックや議論についてです。

過剰な設計について

クリーンアーキテクチャに則って UseCase の Input/Output 用のクラスを定義していたが、これらを定義するほどの複雑性がない
「ただデータを積み直し」ているだけになっていて、メリットが薄いケースが多い

  case class UseCaseInput(promotionId: PromotionId, mediaId: MediaId)

  case class UseCaseOutput(promotionText: String)
  
  trait UseCase {
    def execute(input: UseCaseInput): Future[UseCaseOutput]
  }

cake patternがただDIしたいだけの一般的なアプリケーションにしては構成が重厚すぎる
cake patternとZIOのモジュールとの使い分け

DDDに関して

リポジトリなどDDDのようなコンポーネント分割がされているが、適切にドメイン設計されているわけではないためユースケース<->リポジトリで1:1になっていたり単純にDBの物理テーブルとリポジトリが1:1になっていたりする。

コードの可読性について

タプル、特に_1 _2が頻出して可読性が低い状態となっているため、一度変数に格納してわかりやすい名前をつけるか、better-monadic-forの利用を推奨する

  for {
    t <- returningTupleEffect()
    named1 = t._1
    other = t._2
    ... (use named1 or other)
  } ...

  // sbt
  addCompilerPlugin("com.olegpy" %% "better-monadic-for" % "0.3.1")

  // scala code
  for {
    (named1, other) <- returningTupleEffect()
    ...
  } ...

Booleanでパターンマッチするより、素直にif/elseした方が意図が伝わりやすい

  someCondition match {
    case true => a
    case false => b
  }
  ↓
  if (someCondition) a else b

その他

型の設計について
newtype / valueobjectの段階的な採用について
管理画面側のAPIにおける行動ログやevent sourcingのような仕組の導入提案
コードの凝集性と抽象化のバランス
型や変数のネーミングについて
標準ライブラリの便利なメソッドの紹介

  // 例: if/elseでSome/Noneにする場合、Option.whenが使える
  if (someCondition) {
    Some(value)
  } else None
    ↕︎
  Option.when(someCondition)(value)

…などなど、挙げた以外にも色々あるのですが、全て書いているときりがないため割愛させていただきます。
ドメイン分析の話からプロジェクト設計・実装の詳細に至るまで様々なフィードバックを頂き、非常に実のある議論ができました。

議論も元々自分ともう一人のスペシャリストで実施していた頃は、意見が分かれた際にどうしても1:1になってしまうためにうまく結論などが出せないことがありました。それががくぞさんに参加していただいたことで「がくぞさんが言うなら」的に納得感のある結論・決定を出すことができるのは大きなメリットだと感じました。
開発チームにフィードバックを伝えて改善してもらう際にも、がくぞさんのおかげで説得力が増しています！