Kyash iOSアプリの大規模リファクタリングの話

こんにちは、クライアントエンジニアの@kobakeiです。元々KyashのAndroidアプリを立ち上げから担当しており、昨年末よりiOSアプリを開発しています。

Kyashは3/5 (月)に初のメジャーバージョンアップとなる2.0.0をリリースし、大幅にデザインをリニューアルしました。実はiOSチームはそれよりも前、昨年末から大幅なアーキテクチャの見直しとリファクタリングを並行して行っていました。今日は皆様にその裏側をご紹介したいと思います。

当時のiOSアプリが抱えていた課題

KyashのiOSアプリは2017年の4月にリリースされましたが、開発期間は意外に長く2016年2月に最初のコミットがGitHubに入りました。そこから様々なスクラップ&ビルドやiOSチームのメンバーの増減を経てリリース、そして現在に至るのですが、その結果「品質が安定しない」、「普段の開発効率が上がらない」という2つの問題に直面しました。

これらの問題をさらに深掘りすると以下のような図になります。様々な問題の積み重ねが結果としてプロダクトの品質やチームの開発効率に悪影響を与えていました。

f:id:keisukekobayashi:20180315175421p:plain

リファクタリングの開始

以上のような状況にあって、「流石にそろそろ何とかしよう!」とチーム内で協議し、ついに大規模なリファクタリングに着手しました。Kyashでは会社的な「◯月△日にこの機能のリリースを目指そう」という大きめのマイルストーンがある以外は、エンジニアが比較的裁量を持って細かいバグ修正やリファクタリングのタスクを次のリリースに積んでいく開発スタイルになっており、CEOやプロダクトマネージャも技術的負債の返済にコストを割くことに理解があります。リファクタリングにおいては、以下のような方針で作業を進めました。

未使用コードの削除

まず、未使用のコードの削除にとりかかりました。

どこからも参照されていないクラスや関数は、Xcodeの「Find Selected Symbol in Workspace」を使って使われていないことを確認してから単純に消すだけでよいので簡単です。Kyashの場合はこれだけでかなりのクラスを消すことができました。この頃のGitHubの統計を見ると、書いた行より消した行の方が多くなってました。

問題なのはコード上は参照が残っているけど実行時には通ることがないコードです。仕様書があれば仕様書を正として不要なコードを見つけることも可能ですが、なかなか仕様書がきちんと整理されていることは稀ですよね。Kyashの場合は、仕様書の代わりにAndroidアプリの実装と、Androidアプリを実装したときの自分の記憶を手がかりに不要なコードを見つけていきました。KyashのAndroidアプリはiOSリリース後に開発がスタートしたこともあり、リリース時の仕様が整理されていたたため、仕様書代わりにAndroidの実装を参照しながらコードを消していくことができました。

MVVM & 階層化アーキテクチャの採用

また、アーキテクチャも再検討しました。基本的な考え方はMVVM + 階層化アーキテクチャという、最近のアプリ界隈ではオーソドックスな設計です。

変更前後のアーキテクチャを表した図が以下になります。

f:id:keisukekobayashi:20180315174328p:plain

従来の巨大なViewControllerからビューモデルを分離したことで、ViewControllerはビューの操作に専念する小さいクラスになります。ビューモデルはViewControllerが画面を表示するのに必要なデータやモデルの呼び出しなど、UIKitに依存しないクラスになりました。1画面あたりのファイル数は増えますが、1ファイルあたりの行数が短くなるのとクラスの責務が分離されるので可読性は向上します。

モデルはこれまでUserクラスが巨大な神クラスとなっていて、あらゆる操作のエントリーポイントとなっていました。下記のようなインタフェースになっており、Facebook SDKのようなイメージです。

// in SomeViewController
let user = KyashUser.currentUser()
user?.createTransaction(...) // 内部でAPIコールしたりキャッシュを更新したりする

こちらもUserクラスが巨大すぎて可読性を下げる要因となっていたため、クラスをリポジトリ層とインフラ層に分離しました*1

リポジトリ層はエンティティのCRUDを担当する層で、API経由でデータを取得するかそれともキャッシュを返すかといった処理を隠蔽するのが責務です。キャッシュが不要なエンティティのリポジトリでは、シンプルにAPIクライアントを呼び出すだけになります。

インフラ層はRESTful APIやデータベースから取得した生の値をエンティティに変換するのが責務です。RESTful APIからEntityへの変換にはAlamofireを使用しました。自分の観測範囲ではAPIKitも人気のようですが、弊社ではセキュリティ対策としてSSL Pinningを導入しており、Alamofireの方がSSL Pinningの実装が簡単*2ということも採用の決め手でした。また、Keychainの読み書きにはKeychain-swiftを採用しています。

このように、モデル層もレイヤーごとに責務を明確にすることで可読性が上がり、またテストの時だけモックに差し替えることでテストの自動化もしやすくなります。例えば、会員登録フローのUIテストを自動化しようと思うと、普通にサーバーと通信しているとメールアドレスの重複などでエラーになってしまいますが、APIクライアントを固定のレスポンスを返すモックに差し替えることで、会員登録完了までのフローをテストすることができます。

一つだけ迷いどころだったのは、ビューモデルとリポジトリの間にユースケース層を挟むかどうかという点でした。Kyash Androidアプリではユースケース層を作っているのですが、ほとんどのユースケースではリポジトリをただ呼び出すだけになったこともあり、iOSアプリではユースケース層はなくしました。この点については、今後の機能開発の中で特定のパターンのリポジトリの呼び出しを繰り返し書くことがあるようなら、そのときに導入を検討したいと思っています。

RxSwiftの導入

階層化アーキテクチャのレイヤー間のインターフェースには、RxSwiftを採用しています。Kyashのサーバーサイドはクライアント向けにRESTful APIを提供していますが、ひとつの画面を描画するのに複数のAPIをコールする必要があるケースがよくあり、RxSwiftを使うことで非同期処理を直列や並列で結合しやすくなります。

例として、こちらの画面(ウォレットタブ)を見てみましょう。

f:id:keisukekobayashi:20180315174933p:plain

こちらの画面を描画するには、以下のAPIを叩く必要があります。*3

  • 残高の取得
  • Kyash VISAカード(バーチャルカード)の取得
  • ユーザーが登録したクレジットカードの取得

従来はこれらのAPIコールをDispatchQueueで待ち合わせていました。

let dispatchGroup = DispatchGroup()
dispatchGroup.enter()
dispatchGroup.enter()
dispatchGroup.enter()
DispatchQueue.global().async {
    // 残高の取得
    user.loadBalance { (balance) in
        // ...
        dispatchGroup.leave()
    }

    // クレジットカードの取得
    user.loadLinkedCards { (cards) in
        // ...
        dispatchGroup.leave()
    }

    // Kyash VISAカードの取得
    user.loadKyashCard { (kyashCard) in
        // ...
        dispatchGroup.leave()
    }
}
dispatchGroup.notify(queue: .main) {
    // 完了時の処理
}

これは辛いです。実際にDispatchGroupのleave漏れでバグが発生していたこともありました。

RxSwiftを採用したことで、このような複数の非同期処理の待ち合わせも以下のような感じですっきり書けます。

Single.zip(self.balanceRepo.loadBalance(),
        self.linkedCardRepo.loadLinkedCards(),
        self.kyashCardRepo.loadKyashCard(),
        resultSelector: { (d1, d2, d3) -> (Balance, [LinkedCard], KyashCard) in return (d1, d2, d3) })
    .observeOn(MainScheduler.instance)
    .subscribe(onSuccess: { (balance, linkedCards, kyashCard) in
        // データ取得成功 => 画面の更新へ
    }, onError: { (e) in
        // データ取得失敗 => エラー表示
    })
    .disposed(by: self.disposeBag)

Embedded Framework化

差分ビルドを利用してビルドを高速化するために、Embedded Framework化しました。こちらはアーキテクチャを階層化した後だったため、特に難なくできました。KyashではEmbedded Frameworkを以下の粒度にしています。

  • メインターゲット(プレゼンテーション層)
  • Repository
  • Infrastructure
  • Entity
  • Utility

モデル層はすべてEmbedded Frameworkになりましたが、最も巨大なプレゼンテーション層はメインターゲットに残ったままです。プレゼンテーション層をさらに機能ごとにEmbedded Framework化していけば、より高速化する余地があるのではないかと思っています。しかし、SwiftGenが自動生成するファイルなど、リソース関連であらゆるViewControllerから参照されているクラスがいるので、いい分割方法が見つからずに先送りしています。いいノウハウをお持ちの方がいらっしゃいましたらこっそり教えてください。

CocoaPodsからCarthageへ移行

これまでライブラリの管理は原則CocoaPodsで管理していたのですが、ビルドを高速化するためにCarthage*4に移行できるものはすべてCarthageに移行しました。さらにCIを高速化するため、Carthageのビルド成果物のうち、 Carthage/Build/ 以下もすべてgitにコミットしています。これらのファイル群をgit管理下に置くべきかについては、GitHubのPull Requestでの差分が見にくくなるなどデメリットもあるので賛否が分かれるところかもしれません。弊社ではビルド時間の短縮を優先するという方針のもとコミットすることにしています。

Carthageの導入と先述のEmbedded Framework化で、BitriseでのCIのビルド時間は従来20分以上かかっていたところが、15分を切るまで短縮できました。

まとめ

以上をまとめますと、我々が抱えていた課題の原因に対して、以下のようなアプローチでリファクタリングを行いました。

  • 複数の責務を担う巨大クラス → MVVM + 階層化アーキテクチャの導入
  • 非同期処理が複雑 → RxSwiftの導入
  • 未使用のロジックが残っている → Androidの実装を参考にしつつ、地道に削除
  • ライブラリのビルドが遅い → CocoaPodsからCarthageへ移行
  • 差分ビルドが効いてない → Embedded Framework化

で、結局改善したの?

これらのリファクタリングを経て、当初の課題である「品質が安定しない」、「普段の開発効率が上がらない」の2つについてはある程度の改善を実感できています。品質面では、CrashlytyicsのCrash Free Rateが99.9%を超えるようになりました。また、これまではリリース前の動作確認で致命的なバグが見つかりAppleへのサブミットが計画より遅延するといったこともありましたが、最近ではめっきりなくなりました。開発速度についても、以前より高速に機能開発ができるようになり、より速いペースでのリリースサイクルを回せるようになりました。また、リファクタリングの副次的な効果として、リファクタリングを行うにあたってチーム内で「こういう設計がいいよね」「この実装はよくないよね」といった議論が行われたことによって、メンバー間の設計に対するコンセンサスが取れるようになり、お互いが書くコードが互いに理解しやすくなったことも大きい収穫だったと思います。

f:id:keisukekobayashi:20180319001627p:plain

俺たちの戦いはこれからだ!

この記事では、KyashのiOSアプリの大規模なリファクタリングについて紹介しました。参考になったでしょうか?

今日紹介した様々な施策を経て、Kyash iOSアプリが素晴らしくイケイケな状態になったかというと、未だ道半ばであるというのが実際のところです。自動テストの導入などは、ようやくテストが書ける状態になったところで、まだバリバリテストコードを書いているには至っていません。リファクタリングと並行して機能開発を進めているとリファクタリングばかりやっているわけにもいきませんが、「新規に作成するクラスは新しい設計に従う」、「既存のクラスの中でも特に主要機能を担うクラスを優先的に対応する」など、作業の優先度をチーム内で決めながら根気強く継続していくしかないと思っています。

また今日の記事では紹介できませんでしたが、Storyboardなどのリソース管理やローカライズ周りも大きく整理をしました。こちらは後日同僚氏が記事を書いてくれますので、楽しみにしていてください!

*1:各層のネーミングはKyashのアーキテクチャにおけるネーミングで、ドメイン駆動設計におけるネーミングとは一致しません。

*2:How to make your iOS apps more secure with SSL pinningを参照

*3:実際にはもっと沢山のAPIを叩いています。

*4:余談ですがKyash社内ではCarthageは「カルタゴ」と呼んでいます。「カーセージ」派の皆さんごめんなさい。