ペイモのリファクタリングとの戦い
割り勘アプリ「ペイモ」がリリースされて早１年。僕がペイモiOSの開発に関わり初めたのはリリース直後だった。驚くべきことに、当時、弊社AnyPayにはアプリエンジニアが誰もいなかった。
誰がペイモを作ったのか。そう、完全に外部のチームがペイモiOS/Androidを作りあげたのだ。それもおよそ3ヶ月という短い期間で。さらに、プレスリリースの日付も決まっていたので、決して遅れることは許されなかった。今、当時の仕様書を振り返ってみても、よくこれをそんな短時間で作り上げたな…となってしまう。
では、そんな短時間でこんなハードな開発を行うとどうなるのか…。ブクブクに太ってしまったViewController。APIから受け取った配列データをDictionaryにマッピングした結果期待通りの順番が保証されていないデータ群。急に登場するTableViewController。どこで値が更新されているのかわからないシングルトンたち。Storyboardの管理問題。ロジックの散在問題。RxSwiftの書き方が人によって違いすぎる問題。等々。正確には、リリースした直後のものは良かったのだが、内製化していく過程で、新機能を付け加えたりバグを修正する方法が結果的に良くなかった。なので、決して外部のチームは悪くないということを明言しておく(むしろ3ヶ月でリリースしきった方がすごい…)。この１年の間に、外注から完全に内製化する過程で行ったことや個人的に感じたこと・つらみを書いていくことにする。
まずはViewControllerのダイエット
幸いなことにリリースしてから大きなバグは起こらなかった。だが、リリースして1ヶ月してから急にクラッシュが増え始めた。どうやらトップページのタイムライン部分でクラッシュしているらしい。原因は強制アンラップ。必ずタイムラインのViewController（便宜的にTimelineViewControllerと呼ぶことにする）が保持しているはずのものが渡されていないケースがあるよう。対応はすぐされたものの、なぜそういうケースが起こってしまったのか。コードを追うことがだんだん難しくなっている兆候だった。
4月のこと。段々と利用が増え、タイムラインに返ってくるデータが増えてくると、データ表示がおかしいことがあると気付いた。めちゃくちゃ早くスクロールするとデータが二重に表示されたりするのだ。どうやらページネーションのロジックがおかしくなるケースがある。この時にはすでにTimelineViewControllerは1000行近いバケモノになっていた。
ペイモはクリーンアーキテクチャのプロジェクト構成をしていたのだが、この当時、Presenterが薄っぺらく、Viewに多くのUIロジックが書かれていた。表示するデータ(ここではTimelineの配列)もTimelineViewControllerが保持しており、さらにDictionaryにマッピングしていたため、TableViewの描画時に都度sortをして順番を保証していた。
この画像は、その時のプロジェクト構成の概観だ(ボックスの大きさがコードの大きさだと思ってほしい)。View(正直こんなものではなかったが図面の関係で抑えめに描いている)とDataStore、ViewModelが大きくなっていた。UIロジックだけでなくビジネスロジックもほとんどがViewに書かれていた。これは、内製化しようとしている時でコード規約も定まっておらず、かつ時間に対して実装するべきことが多かったことが招いてしまった結果だと思う。
そこで時間をしっかりとり、UIロジックはPresenter層に移すべきという議論をAndroidチームと行い、実装案を出した上で、とにかくUIについてMVPを徹底しましょうという思想で設計していく合意を取った。iOS/Androidともに同じ方針のもとリファクタを進めていったことで、人数が少ないながらも、知見の共有が活発に行われてスムーズに開発が進んだ。段階的にリファクタを進めていき、7月頃にはTimelineViewControllerは3~400行程度になった。複数画面でタイムラインを表示する、という要件が出てきた時も結果的にスムーズに実装できたので非常に効果的だった。
これが7月頃(UIロジックのリファクタ後)の概観。この時はUIロジックについてリファクタだけだったので、ドメイン層より右は触れていないが、Presenterの存在感が出てきたのが分かる。
ViewからPresenterにロジックを移す、という方針の背景にはユニットテストを書きたいというニーズもあり、夏頃からテストが書かれ始めた。とはいえ、UIロジックに対する最低限のテストだったので、テスト用の外部ライブラリを利用せずに、XCTestで書いていた。
薄っぺらいドメイン層にロジックを移していく
これまでアーキテクチャイメージ図を見て、ビジネスロジックはどこに書かれているのか、と思った人がいるかもしれない。クリーンアーキテクチャには、ビジネスロジックを明確に書けて、なおかつユニットテストも行いやすい、というメリットがある。にもかかわらずペイモでは、そのメリットを享受できていなかった。ビジネスロジックは、ペイモリリース直後はそこまで複雑ではなく、速さ優先でViewやViewModelに書かれていた。それが、機能を改善していくにつれて途端に複雑化していった。これは他のアプリにも共通すると思う。
リリースして半年ほどした8月頃に一気にビジネスロジックを整理することにした。具体的には、View：Presenter：UseCaseが1 : 1 : 1で定義されていたところを、機能単位でUseCaseを定義してPresenterは複数のUseCaseを持つようにした。
他にもDataStoreの重複コードをまとめていったり、曖昧に扱われていたドメインモデルの定義を明確にした結果、12月頃には以下のようなイメージになった。
以上のようなアーキテクチャ周りのリファクタをしつつ、テストが書かれていないところを実装していった。Quick、Nimble、OHHTTPStubs、RxBlockingあたりを利用して通信周りのテストを実装している。
シングルトンとの戦い
シングルトン、便利ですよね。ただ、ペイモにおけるシングルトンは、多くのプロパティを持っていた。支払いや請求をする時に入力した値を他のページに渡す必要があるのだが、シングルトンを利用していたのだ。たしかにシングルトンを利用すれば、値の受け渡しが簡単にできるように見えるが、どこで値が上書きされているのかも把握できなくなり、思わぬバグが発生してしまう。特に、支払いや請求への動線が増える時に、他の箇所でいじった値によって上書きされてしまう等々、考慮すべきことが倍増した。このように、仕様が複雑になっていくにつれて、不必要にシングルトンを利用してしまうと対応が苦しくなることを身をもって痛感した。
UseCaseの整理をする時に、シングルトンが出てくるところは削除していった。入力値の受け渡しには、DTO的な振る舞いをするモデルを作成するように変更した。支払い周りのシングルトンを消して、ビジネスロジックも１つのUseCaseにまとめたため、複数の支払い動線ができてもDRYに書けるようになった(これが当たり前だと思うが)。
まとめ
色々リファクタ周りのことを書いたが、機能開発をせずにリファクタのみをする、ということはほとんどない。基本的に新機能実装と並行してこれらの修正を行ってきた。弊社の開発チームは、テストが好きなエンジニアや設計・アーキテクチャ好きなエンジニアもいるので、全体がコードの質にこだわっているという印象がある。興味がある方、絶賛エンジニア募集中(特にiOS!!)なので、是非話を聞きに来てください!!
