STORES Product Blog

こだわりを持ったお商売を支える「STORES」のテクノロジー部門のメンバーによるブログです。

iOSDC2021にて公開した「約3年続くリファクタリングを引き継いで見えたテクニック」のご紹介

はじめに

はじめまして。heyでiOSエンジニアをしているk-koheyです。弊社は去年からiOSDCというiOS開発者向けのカンファレンスのスポンサーをしており、さらに今年度は私含め3名のiOSエンジニアのプロポーザルが採択されました。私は「約3年続くリファクタリングを引き継いで見えたテクニック」という題目にて、原稿を書かせて頂きました。

hey.jp

本稿ではその原稿の内容に一部修正を加えたものを公開します。

さて、題目にもあるリファクタリングとはなぜ必要なのでしょうか。これは現代においてソフトウェアは一度作ったら終わりではなく、その後もソースコードを保守管理しなければいけないという事が影響していると考えられます。さらにイテレーティブにコードを改修するにあたって、テストも無く読み解くのも難しいレガシーコードはボトルネックです。したがって、継続的に開発をし続ける必要があり、更に同一以上の速度で開発をし続けたいという要求があります。この要求を満たすための手段としてリファクタリングを使えます。

本稿では3年続いたObjective-CからSwiftへ移行するプロジェクトを引き継いだ際に直面した課題から着想を得て、学んだリファクタリングの手法についてトピック形式で紹介します。トピック形式のため、どのセクションから読んで頂いても理解できるようになっています。

リリース可能な状態を保つ

コードを常にリリース可能な状態にしておく事は非常に重要です。つまりは継続的インテグレーションされている状態を保つべきです。具体的には、リファクタリング後のコードは細かくリリースを行うブランチ(以降、mainブランチと呼びます)と統合されるべきということです。

前提として、レガシーコードは単体テストを書くのが難しい事が多いです。よって、動作確認のテストには手作業による結合テスト(以降、便宜上手動テストと呼びます)に頼る事が多くなります。手動テストを実行する際にはリファクタリング後のコードが実行されるコードと結合されている必要があります。つまり、結合されている事自体がテスト可能な状態を保つことに繋がります。

この状態を早期から保てていない場合、リリース前に大きなコストを支払うことになります[1]。具体的には、リリース前にGitを使って巨大なリファクタリングブランチをmainブランチにマージすることになります。その結果、次のような事が懸念されます。

  • リファクタリング後のコードをmainブランチに結合するのに途方もなく時間がかかる。このタイミングで新たに不具合が発見され、さらに工数が膨らむ。
  • 巨大な変更をQAチームに依頼することになり、フィードバックも巨大になる。
  • 開発時には動作確認が取れていた処理が壊れる。

実際に私がリファクタリングを引き継いだときの初めの仕事は、リファクタリング後のコードを結合する仕事でした。そして、結合する際には多くの知識と不具合の解決が必要でした。最後に大きな工数を支払わないためにも、細かく結合していく作業は重要です。

リファクタリングを進めながら別の修正も加えたい

リリース可能な状態を保つべきと先述しました。これは、リファクタリング中のコードに急遽バグ修正や機能追加をする必要が出てきた際にも役立ちます。修正を加える必要があるコードがリファクタリング中であっても、コンフリクトを最小限にできます。

しかし、コードを結合しようにも、既存のコードがレガシーすぎて結合できない場合もあります(フルスクラッチで書き直すしか無い、という状態です)。加えて、テストも不可能なくらいレガシーなコードに対して、機能追加や修正を行っていくことに危険な香りを感じます。

このような状態において機能追加を迅速に行いリリースをしていくために、一旦既存のコードのリファクタリングを諦めるという方法もあります。例えば、スプラウトクラス [2] がその方法です。

スプラウトクラスはあるクラスあるいはメソッドに対して機能を追加する際に直接それらに変更を加えるのではなく、追加機能を実装した別のクラス(以降、これもスプラウトクラスと呼びます)を新たに作る方法です。スプラウトクラスはもともと機能追加しようとしていたクラスあるいはメソッドから呼び出します。

では、スプラウトクラスを作り、そして使用する例を示します。showTagsメソッド(図3.1)は取得したタグをUIに反映させるメソッドです。このメソッドに対してタグクラウドのようにタグが持つ情報を基に各タグに表示する大きさを付けるという機能を追加するとします。手順は以下の3つの工程から成ります。

  1. スプラウトクラスTagAppearanceCaculatorを作る(図3.2)
  2. TagAppearanceCaculatorをshowTagsメソッドの中にてインスタンス化する(図3.3(1))
  3. TagAppearanceCaculatorのsize(of:)メソッドを呼び出す(図3.3(2))
f:id:rivemo:20211001181005p:plain:w520 f:id:rivemo:20211001181009p:plain:w520 f:id:rivemo:20211001181011p:plain:w520

この方法のメリットを3つ紹介します。

  1. 既存のコードを壊さずに、テスト可能なコードを追加できる点です。TagAppearanceCaculatorは任意のTagインスタンスを渡すことによって、単体テストが可能です。
  2. スプラウトクラスを使う場合はシグネチャも完全に維持されます。showTagsメソッドに新しく仮引数が増えたり、返り値が追加されることはありません。
  3. iOS開発においてはObjective-Cのコードに対して機能追加する際に有効だと考えています。スプラウトクラスはSwiftで書き、Objective-Cのコードからはそれを参照することによって、手の入れにくいObjective-Cに対して安全かつ容易に機能追加することができます。加えて、プロジェクト内のSwiftファイルを増やすことにも繋がります。

しかし、既存のコードの事は棚に上げている点と新しいクラスを追加することによってコードが複雑化する点はデメリットとなります。クラスの責務を考えたときに、この方法を取ることがすべての時間において正しいかはわかりません。その場しのぎの選択になる可能性もあります。

よって、定期的に細かくリファクタリングすることで常に最善の形を追求する必要はあります。

巨大クラスを解体するための第一歩

巨大なクラスは格好のリファクタリング対象です。しかし、内部からグローバルなオブジェクトの参照を行っている、あるいは実行時によって異なる挙動をする実装が組み込まれている場合は単体テストが書けず、安全にリファクタリングすることは難しいです。このような状態でシグネチャを破壊するようなリファクタリングはリスクが伴う上に、時間もかかります。

手間のかかるクラスの大解体をする前に、手軽に始められるのは、まずはインタフェースの抽出 [2] を行い、そのインタフェースを分解するというやり方です。具体的な手順を下記に示します。

  1. 対象とするクラスが外部から参照されるメソッドおよびプロパティの全てをプロトコルに定義する(図 4.1A)
  2. 必要に応じてそのプロトコルを分解する(図 4.1B)
  3. 対象とするクラスの型を直接参照していたところを、プロトコルを介した参照へと切り替えます(図 4.1C)。

この作業の中でも、最も考える部分はどのようなまとまりのインタフェースに分割していくかです。分割の指針は、対象とするクラスの使われ方を観察して決めます。巨大なクラスであれば、そのクラスを参照している各箇所で見ると呼ばれているプロパティやメソッドはごく一部なはずです。そして、それらの使われ方に類似点を見るけることができれば、その使われているプロパティやメソッドを絞って抽出したインタフェースを作ることができます。

この作業を繰り返すと、いくつかの小さいインタフェースに分けられるはずです。反対に、インタフェースを分けられない場合はそのクラスは既に偶発的ではない凝縮をしており、仮に巨大なクラスであっても然るべき姿になっていると考察できます。

f:id:rivemo:20211001181013p:plain:w520 f:id:rivemo:20211001181015p:plain:w520 f:id:rivemo:20211001181018p:plain:w520

この方法の利点を3つ紹介します。

  • 分割したインタフェースを呼び出す側は参照しているクラスが小さく(なったように)見えます。巨大なクラスを利用していた利用者は、まるでリファクタリング後のコードを使用しているような体験を得られます。必要としないプロパティやメソッドには触ることができないので、リファクタリング中に意図しない副作用を起こす心配も無くなります。
  • インタフェースを参照するようになるためテスト可能な領域が広がります。具体実装を入れ替えやすくなり単体テストが書きやすくなるため、リファクタリングを進めながらリファクタリングしやすい環境も作ることができます。
  • 実際に巨大クラスを分割していく際の分割指針になります。結果として、実質実運用されている状態でクラスの分割を始められます。

ダイナミックなリファクタリングを行う安全策や時間が十分ではない場合におすすめな方法です。

privateなメソッドにテストを書いていく

難解なレガシーコードに手を加えていくと、たとえその変更がprivateなアクセス修飾子によってマークされた関数であったとしても、その関数に対してテストを書きたくなります。しかし、テストのためにメソッドからprivateなアクセス修飾子を取り除くのは、せっかく構築したカプセル化が崩れるためナンセンスに感じる方が多いでしょう。

このような場合は、Extract Classパターン [3] が有効です。このパターンでは、既存のクラスからテストしたい対象のメソッドを切り出し、そのメソッドをpublicとして持つ別のクラスを作ります。

具体的な方法を図5.1に示します。ViewControllerが持っているparse(data:)メソッドに対してテストを書きたいとします。現状ではprivateアクセス修飾子が付いているため、XCTestからアクセスできません。ここで、Extract Classパターンを適応し、ViewControllerからparse(data:)メソッドの定義を消し、同名のメソッドを持つ新しいクラスParserを定義します。Parserクラスはparse(data:)メソッドをpublicで定義しているため、XCTestからアクセスできます。

f:id:rivemo:20211001181020p:plain:w520

このパターンの利点はテストを書けるだけではなく、改めてクラスの責務を考えることができる点です。そもそも、privateなメソッドをテストしたいという欲求は、そのメソッドが所属しているクラスとは別の責務を持っている可能性があります[4]。言い換えると、現実装では都合が良いから同じスコープにメソッドが定義されているだけで、責務は全く異なっているという可能性です。例えば、ViewControllerやViewModelの中にある何らかのパース処理やオブジェクトの比較処理がこれに該当します。このメソッドを適用していくことで、テストを書けるだけでなく、先述したような責務過多なクラスを整理していくことにも繋がります。

強制アンラップを無くす際のアンチパターン

モバイルアプリはウェブアプリと比較するとアプリをリリースするまでに時間が掛かります。App StoreGoogle Play等のマーケットにアプリを公開するには、マーケットプロバイダーからの審査があるからです。よって、予期できないクラッシュは避けたいと考える開発者は多く、Swiftでは言語仕様としても if letや guard let等のクラッシュを避けつつ開発体験を高める書式が提供されています。

しかし、無闇にこれらを使うことは、開発時に気づける例外を見落とすリスクを増加させる側面もあるため、注意が必要です[5]。既にある強制アンラップを剥がすのも同様です。この場合は更に、新たに安全なアンラップを施す対象の変数がnil時の仕様を考える必要もあります。大げさに言うと、ちょっとしたコードの変更で済んだとしても、ビジネス的な判断が必要な場合もあります。

例えば、図 6.1のような変更は一見安全な変更に見えますが、if文が制御する処理のスコープを間違えると何らかの不具合が生じる可能性があります。また、このような変更箇所単体では不具合が生じるかを網羅的に判断しにくいため、コードレビューでも見落とされがちです。

f:id:rivemo:20211001181023p:plain:w520

仮にコードの改修中に先述したような安全にアンラップした結果、不具合が生じた場合を考えます。この場合はクラッシュが発生した場合より解決は難しくなります。理由は原因の特定のためにコードを読む必要があるからです。クラッシュした場合、問題が発生したところでスタックトレースが止まるためその必要はありません。対して、クラッシュしない場合は、勘を頼りにレガシーコードを読み解くしかありません。

先述したようなケースを嫌うなら、素直にそのまま強制アンラップのままにしておく方がリファクタリングは捗るでしょう。もちろん、リリースする前に強制アンラップを剥がすのは重要ですが、問題なのはタイミングです。

最後にnilをアンラップする際に、開発時に限ってクラッシュするオペレータ [6] を図 6.2に掲載します。このようなオペレータを使うことで、意図しない挙動に気づきつつも、リリース後にアプリがクラッシュすることを防げます。

f:id:rivemo:20211001181025p:plain:w520

なんとなくの安全なアンラップは逆にアンチパターンなのかもしれません。

おわりに

本稿では、リファクタリング業務を行う中で自身が有益だと感じた考えや手法について紹介しました。自明な内容も多く含まれたと思いますが、これからリファクタリングを始める方の足がかりになれば幸いです。

また、弊社のiOSメンバによるレギュラートークが後日公式YouTubeチャンネルに公開される予定です。ぜひ見てください。

参考文献

  1. David Scott Bernstein、吉羽 龍太郎、永瀬 美穂、原田 騎郎、有野 雅士、レガシーコードからの脱却―ソフトウェアの寿命を延ばし価値を高める9つのプラクティス(2019/9/19) オライリージャパン
  2. マイケル・C・フェザーズ、平澤章 、越智典子 、稲葉信之 、田村 友彦 、小堀真義、レガシーコード改善ガイド(2009/7/14)翔泳社
  3. MartinFowler、児玉公信、友野晶夫、平澤章、梅澤真史、リファクタリング ( 第 2 版 ):既存のコードを安全に改善する( 2 0 1 9 / 1 2 / 1 )オーム社
  4. t-wada「プライベートメソッドのテストは書かないもの?」 (2021/07/14 最終閲覧) https://t-wada.hatenablog.jp/entry/should-we-test-private-methods
  5. Elvis Shi「いいから!を使え! / Shut up and use ! !」 (2021/07/14 最終閲覧) https://speakerdeck.com/lovee/shut-up-and-use
  6. バグに気づきやすくなる??演算子を改造したカスタムオペレータ 【Swift】 (2021/07/14 最終閲覧) https://qiita.com/k-kohey/items/edfadeb338246042bbff