現在AbemaTVでiOSアプリ開発を担当してます、shoheiyokoyamaです。
本記事では、AbemaTVでの「コードの品質について」チームでの考え方や取り組みについて紹介していきたいと思います。
AbemaTV
AbemaTVは、オリジナルの生放送コンテンツや、ニュース、音楽、スポーツなど多彩なジャンルのコンテンツを見れるインターネットテレビでiOS、Android、PCなどで利用できます。
チームにジョインした当初は私を含め5人体制でiOSの開発を進めていましたが、現在は倍の10人体制で開発を進めています。チーム増員に伴い、開発スピードは上がりますがその分コードの品質を維持し続けることが難しくなります。
早い開発スピードを長期的に継続していくためには、高品質なコードを維持することが重要です。そのため、早い開発サイクルの中でも高品質なコードを維持する取り組みが必要となってきます。実際のチームでの取り組みをいくつか紹介します。
コードスタイルガイドライン
私たちiOSチームはコードスタイルガイドラインを設け、基本的なルール、設計に関することやコードスタイルの統一をしています。
ガイドラインはAppleが作成したAPI Design Guidelinesというコード規約に関する内容や、AbemaTVのiOSアプリで採用されているフレームワーク・アーキテクチャに関するTipsやベストプラクティス、チーム特有のルールなども追加されています。チームでガイドラインを設けることは、様々なメリットがあります
- 新しくチームにジョインしてきたメンバーがコードやプロジェクトを理解しやすくなる
- コードベースの整合性が保ちやすくなる
- コードレビュー時の議論の手間がはぶける
- チーム内で知識を共有できる
iOSチームでは週に一回定例会があり、開発をしていて気になる点や改善したほうがよい点などを共有する機会が設けられています。そこで認識を合わせた上で必要があればガイドラインを更新するというフローで、ガイドラインは日々改善されています。
お互いの知識の共有やよりよい問題解決をチームで考えることにもつながっているのでガイドラインを設けることのメリットは大きいと思います。
開発言語であるSwiftは、言語仕様上自由度が高いプログラミング言語なのでチームによって様々なコード規約が設けられていることが多いです。
Lint
SwiftLintというフレームワークを用いて、コード品質チェックの自動化も行われています。これはRealm社が開発したlinterでコードの構文などをチェックをしてくれる便利なツールです。SwiftLintはカスタムルールを設定することもでき、AbemaTVのガイドラインもいくつか設定されています。ローカルでのビルド時とCI上で構文チェックをおこなっているのでレビューコストを削減することができます。
コードレビュー
プルリクエストがマージされる前のコードレビューは頻繁に行われています。基本的に3人以上からApproveを貰ってからマージしますが、軽微な変更であれば2人ほどでマージしたり、依存箇所が多い大きな変更、コアな部分であればレビュー担当をAssignすることもあります。
デザインが関わるプルリクエストであればデザイナーが直接レビューを行い、デザインの指摘をすることもあります。
iOSチームは新卒を含め多くのメンバーがレビューをしてくれる文化があるので、コードや設計に関する多くのフィードバックを貰うことができます。これは本当に素晴らしい文化だと思っています。
メンバーの toshi0383さんがLGTMツールを開発してることもあり、それを使ってユーモアあふれたLGTM画像やgif付きのApproveを頂くことも多いです。
また、Botを用いてマイルストーン設定や、レビュアーチェックの自動化も行なわれています。(K.I.T.Tはmarty-suzukiさんの好きな車だということで名付けられました)
コードベースでの取り組み
チームでの取り組みについていくつか紹介しましたが、「コードの品質について」普段開発の中で気をつけてる点や改善した点など、設計やコードに関する内容を紹介していきたいと思います。
まずは私が担当した案件の話が次に続きます。
検索機能改善
AbemaTVは検索機能があり、出演者や番組・ビデオなどを検索することができます。
主に検索履歴や候補が表示されるリスト上の画面と検索結果画面で構成されていていますが、私は検索結果画面を改善する案件を担当しました。
検索機能の設計
まずは簡単に、検索機能の設計について説明します。iOSアプリはFluxに加えてMVVMのアーキテクチャが採用されています。これらのアーキテクチャについては、marty-suzukiさんの資料が参考になります。
検索機能は検索履歴や検索結果候補を表示する画面ListViewControllerと、検索結果を表示する画面ResultViewController、その2つの画面の画面切り替えを行うContainerViewControllerの主に３つのViewControllerで構成されています。
おおまかな設計を図で表すと以下のような構成になります。（厳密にはDataSource用のクラスや画面切り替え用のViewControllerもあるのでもう少し複雑な構成です）
箇所によってはFluxも使われてますが、ここでは基本的にMVVMベースな設計となっています。
それぞれのViewControllerがViewModelを保持し、ViewModelはデータの操作やViewへのデータバインドなどを行なっています。ContainerViewControllerはListViewControllerとResultViewControllerを保持し、画面の状態の制御などを行なっています。例えば、ユーザーがキーワードを入力すれば候補となるキーワードを表示させ、検索を行えば検索結果の画面に切り替えます。
密結合なクラスの関係
今回の対応は検索結果画面の改善なので、ResultViewController周りの変更が主となりますが、それ以上に考慮しなければならない問題点がありました。
検索の結果に応じてContainerViewControllerが画面制御を行う必要があったため、以下の様にResultViewModelとContainerViewControllerが密結合な関係にありました。
final class ResultViewController: UIViewController {
let viewModel = ResultViewModel()
...
final class ContainerViewController: UIViewController {
let viewModel = ContainerViewModel()
let resultViewController = ResultViewController()
...
viewModel.value.asObservable()
.bind(to: resultViewController.viewModel.value)
.disposed(by: disposeBag)
resultViewController.viewModel.value.asObservable()
.bind(to: viewModel.value)
.disposed(by: disposeBag)
密結合な関係性は柔軟に拡張することが難しくなってしまうため、修正コストは高く様々な問題があります。
- 既存コードの修正による影響範囲が大きい
- そのクラスと依存しているクラスの情報を把握する必要がある
- 参照されてる側は、どこから参照されてるか把握しにくい
MVVMの性質上、特にViewModelはViewControllerと密結合な関係性になりやすいため、複数のViewControllerから参照されてる場合は、非常に複雑な構成になってしまいます。
実際の研究や調査結果からも、この設計は少し危険であることがわかります。
オブジェクト思考における研究では、クラスのfan-out¹が多いほどエラー率が上昇することが明らかとなっています²。また、今回のようなクラスの間接的呼び出しはエラー率が高くなるという調査結果³もあります。これに関してはデメテルの法則 (Law of Demeter, LoD)が参考になります。
¹ クラスが参照しているクラス数。一般的に高いfan-outとは7つ以上のクラス数を指す
² Victor R. Basili, L. Briand, Walcelio L. Melo 「A Validation of Object-Oriented Design Metrics as Quality Indicators」(『IEEE Transactions on Software Engineering』)SE-22, No. 10(October 1996)pp.751-761
拡張性を考慮したMVVMの設計パターン
今回の検索結果画面の対応と合わせて以下のリファクタリングを行いました。（機能追加・修正とリファクタリングは分離して行います）
- ViewModelのプロパティはprivateで保持し、ViewControllerとViewModelが1対1の関係性を保つ(ロジックを複数のViewModelで分けてる場合は1対n)
- Input / Output(I / O)はViewControllerを介して行う
ResultViewControllerは以下の様なコードになります。
final class ResultViewController: UIViewController {
private let viewModel = ResultViewModel()
// Output
var value: Observable<Value> {
return viewModel.value
}
// Input
func setValue(_ value: Value) {
viewModel.setValue(value)
}
...
final class ContainerViewController: UIViewController {
private let viewModel = ContainerViewModel()
private let resultViewController = ResultViewController()
...
viewModel.value.asObservable()
.subscribe(onNext: { [weak resultViewController] value in
resultViewController?.setValue(value)
})
.disposed(by: disposeBag)
resultViewController.value.asObservable()
.bind(to: viewModel.value)
.disposed(by: disposeBag)
ViewModelの独立性を保つことで、検索結果の修正はResultViewControllerとResultViewModel内でおさまるようになります。また、公開されている情報量が大幅に削減されたため全体的な設計が把握しやすくなります。
保守・拡張性や設計のわかりやすさの観点ではこの方法が良いと思っていますが、ViewControllerはGetter、Setterが増え冗長化します。また、これらはRxCocoaのbind(to:)のAPIも使えないので、スマートな記述がしにくくなります。
これらはトレードオフなのでケースによって設計を使い分けるべきだと思っています。
今回改善した内容のまとめです
- クラスの密結合な関係性は、拡張がしにくく既存コードの修正コストが高い
- ViewModelのプロパティはprivateで保持する
- ViewModelの独立性を保つために、必要なI/OはViewControllerが行う
開発が進むにつれて、コードの品質も変化していきます。修正や変更が加わることにより、最適な設計なども変化していくので、既存のコードにも焦点を当てて定期的に設計を見直していくことが必要だと思います。
命名について
普段の開発ではコードを書く時間より、読む時間のほうが圧倒的に時間を使います。読む時間を最小限におさえるためにも、コードを書く時は自分を含め、他の人に意図が伝わりやすい命名付けをすることが重要です。
ここでは、普段開発の中で命名付けについて心がけていることをいくつか紹介します。
API Design Guidelines
AppleはAPI Design Guidelinesを作成し、Swiftの命名規則についてのガイドラインを設けています。ガイドラインに沿ったいわば「Swiftライクな命名」は、コードの一貫性を保ち可読性を向上させる上で重要です。
以下に基本的な内容をいくつか抜粋し紹介します。
Include all the words needed to avoid ambiguity for a person reading code where the name is used.
extension List {
public mutating func remove(at position: Index) -> Element
}
employees.remove(at: x)
より明確に意図を伝えるために、必要な単語は全て含めることが推奨されています。Swiftの関数は外部引数名が使用でき、引数を表現する適切な名前をつけます。
上の例で外部引数名をつけない場合は、employees.remove(x)という表現になっていまい、xを取り除くという違った意味の解釈もできてしまいます。そのため、positionの前置詞を表すatが外部引数名として定義されてます。
Omit needless words. Every word in a name should convey salient information at the use site.
public mutating func remove(_ member: Element) -> Element?
allViews.remove(cancelButton) // clearer
必要ない情報は冗長になってしまうので省略しましょう。上の例ではremoveElement(cancelButton)だとElementとcancelButtonで同じ情報が繰り返されてしまうので、remove(cancelButton)のようにElementを省略したほうがいいと説明されています。
ただ、これは全てにあてはまるわけではありません。次に続きます。
Compensate for weak type information to clarify a parameter’s role.
func addObserver(_ observer: NSObject, forKeyPath path: String)
grid.addObserver(self, forKeyPath: graphics) // clear
NSObject、Any、AnyObjectなどの抽象度が高い型やInt、Stringなどの意味が伝わりにくい型については、省略せず役割を明示しましょう。
役割が明確な型については型情報を省略してもいいですが、明確ではない抽象度が高い型については役割まで明示するとコードを読んだ時に意図が伝わりやすいということですね。
Begin names of factory methods with “
make”, e.g.x.makeIterator().
let foreground = Color(red: 32, green: 64, blue: 128)
let newPart = factory.makeWidget(gears: 42, spindles: 14)
ファクトリーメソッドはx.makeIterator()のようにmake~という関数名にしましょう。また、引数ラベルはイニシャライザと同様変数名をつけましょう。
Follow case conventions. Names of types and protocols are
UpperCamelCase. Everything else islowerCamelCase.
型名やprotocol名はUpperCamelCaseで表し、それ以外はlowerCamelCaseで表現しましょう。また、頭字語は小文字のみか大文字のみで表現しましょう。
var utf8Bytes: [UTF8.CodeUnit]
var isRepresentableAsASCII = true
var userSMTPServer: SecureSMTPServer
OSの特性に合った命名
OSや特性に合わせた命名をすることで、コードを見たときにどんな挙動をするか推測しやすくなります。
例えばiOSのViewControllerを用いたモーダル画面遷移では、表示はpresent(_:animated:completion:) 非表示はdismiss(animated:completion:) といった関数名で表現されています。
そのため、自作したコンテナviewなどを下から上に移動するような動きで表示する際には、present~、上から下に移動するような動きで非表示にする処理はdismiss~という命名をすることで、コードを見ただけでどんな動きをするのか推測しやすくなります。
それと同様にpush画面遷移のような動きではpush~とpop~、タブ遷移のような挙動ではselect~などiOSの特性に合った命名をすることで、挙動や処理がわかりやすくなります。また、これらの命名はpushとpopのような適切な対比表現をすることが重要です。
表示・非表示の処理でいうと、他にもnavigationBarやstatusBarのようにisHiddenで表現されているクラスもあるので、それぞれの特性に近い命名をするとわかりやすいと思います。
最後に
今回は、コードの品質を維持するためのチームでの取り組みやコードでの取り組みをいくつか紹介しました。
品質を高めるためのオーバーヘッドと、長期的なメリットの折り合いをつけることは難しいことですが、長期的な開発スピードを考えてこれらの取り組みは必要だと思います。
AbemaTVではコードの品質を重要視し、それらに取り組む十分な環境が整っています。コードの品質やチームのレベルも進化し続けていて、日々良い刺激を受けながら楽しんで開発をしています。
これからもより良いエンジニアライフを楽しんでいきたいと思います。