レガシーコードによる技術的負債
長年に渡りアプリのアップデートを続けていると、作りがレガシーのまま機能が追加されていくことが良くあると思います。
いつしか、機能追加する際には、影響範囲の考慮や回りくどい処理を考慮しながらデグレを発生させないよう慎重に改修をしなければならなくなり、気づいたら生産性が低下していた、なんてことが発生します。
これが、技術的負債が溜まってしまっている状況なのですが、生産性を上げるためリファクタリングしようという声が上がっても、どこから手をつけて良いのか？という問題にぶち当たります。
私自身、仕事においてもそうですが、個人で開発しているAndroidアプリにおいてもレガシーコードに悩まされていました。
リファクタリング対象リポジトリ
今回は、レガシーコードに悩まされていた個人開発のアプリをリファクタリングした時のプロセスを紹介しようと思います。
タグv3.1.0からv4.2.0までの変更点になりますので、コミットログも参照しながら読んでもらえればと思います。
リファクタリング前のレガシーコード
上記のようにクラス図ではシンプルですが、問題となっていたのは以下の点です。
- Javaで記載されたNull Safetyでないコード
- DB操作のためのクエリ処理実装の肥大化
- DBデータ取得後の格納先がHashMap型による扱いづらさ
- 各Activityに描画ロジックと機能ロジックが実装されたFatActivity状態
- 画面遷移をIntentとputExtraで値渡しによる結合度の高い画面遷移手法
- データ更新後のnotifyDataSetChangedに頼ったグローバルな更新通知
リファクタリングのプロセスを定義
上記の問題に対応していくために、以下の順番でリファクタリングを進めていくことを定義しました。
- JavaコードをKotlinコード化によるNullSafety化
- SQLiteOpenHelperをRoomDatabaseへ変更
- ViewModel+CoroutinesFlow適用によるロジック分離
- SingleActivity+MultiFragment＋Navigation適用による画面遷移時の依存性を解消
- ViewBinding+DataBinding適用による画面更新処理の簡素化
- DaggerHilt適用によるDI処理の簡素化
- JUnitでテストコードを作成して品質の担保
このプロセスの大きな流れとして、下位レイヤーからリファクタリングしていくというところがポイントです。
クラスの依存関係として、基本的にUI層からData層へと依存していくので、依存先であるData層からUI層へリファクタリングをしていきます。
依存先のData層をリファクタリング
↓
依存元であるUI層をData層のリファクタリングに合わせた型変更
↓
依存元であるUI層のリファクタリング
といった流れで順番に改修を進める事ができ、手戻りも防ぐ事ができます。
ここからはそれぞれの対応について記載していきたいと思います。
1. JavaコードをKotlinコード化によるNullSafety化
- Javaで記載されたNull Safetyでないコード
この問題への対処について記載していきます。
まず、javaからKotlinにマイグレーションする方法は
プロジェクト全体に対して、メニューの「Convert Java File to Kotlin File」を実施することで、一気に出来ます。
ただし、１発ではビルドが通らず、以下のような点を直す必要があります。
- findViewByIdで取得した変数にはセーフコール演算子を付ける
groupFab = findViewById(R.id.groupFab)
groupFab.setOnClickListener(View.OnClickListener {
↓
groupFab = findViewById(R.id.groupFab)
groupFab?.setOnClickListener(View.OnClickListener {
findViewByIdで取得出来なかった場合は、nullが返却されるのでnull許容型の変数となります。なので、kotlinの場合には、セーフコール演算子を付けてメソッドを呼び出す必要があります。
- クラス変数に画面部品の変数を定義する場合にはnull許容型にする
var imageButton: ImageButton
var title: TextView
↓
var imageButton: ImageButton? = null
var title: TextView? = null
画面部品を格納する変数は、onCreate等で初めて値が代入されるため、null許容型として定義をしておく必要があります。
2. SQLiteOpenHelperをRoomDatabaseへ変更
- DB操作のためのクエリ処理実装の肥大化
- DBデータ取得後の格納先がHashMap型による扱いづらさ
この問題への対処について記載していきます。
- 改修前
SQLiteOpenHelperを継承して、ListDataOpenHelperというクラスでDBの定義をしました。
ListDataManagerクラスでDBからデータを取得するのですが、取得するためにCursorクラスを使ったり、処理の中でqueryを書いたりして複雑な処理が多くありました。
また、取得したテーブル自体も、List<Map<String, String>>という型に入れているので、データ自体を加工して扱うのも手間がかかるような状態になっていました。
- 改修後
RoomDatabaseを継承して、PasswordMemoRoomDatabaseというクラスでDBの定義をしました。
PasswordMemoRepositoryクラスでDBアクセスのためのIFを全て集約します。
Daoクラスを定義することで、@Query、@Insert、@UpdateなどのアノテーションでDB操作を定義ができるため、queryを書く処理が大幅に省略されます。
また、取得したテーブルの値は、@Entityアノテーションを定義したEntityクラスを定義する事で、型としての安全性も補償した扱いやすいデータとなります。
3. ViewModel+CoroutinesFlow適用によるロジック分離
- 各Activityに描画ロジックと機能ロジックが実装されたFatActivity状態
この問題への対処について記載していきます。
ロジックを分離すること、Flowによるデータ変更を検知する事で、検知した変更を画面に反映するための処理が集約されます。
- Fragmentではデータ変更を待ち合わせて画面に反映
- ViewModelでは選択されたグループに合わせてパスワードリストデータを更新
といった形で処理を分割する事ができます。
@ExperimentalCoroutinesApi
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
lifecycleScope.launchWhenStarted {
// パスワードリストデータの変更待ち合わせ
passwordListViewModel.passwordList.collect { list ->
adapter?.setList(list)
// セットしたデータで画面反映
reflesh()
}
}
}
/** 選択中のグループID */
private val _groupIdFlow = MutableStateFlow(1L)
/** 選択されたグループに応じてパスワードリストデータを更新 */
@ExperimentalCoroutinesApi
val passwordList: Flow<List<PasswordEntity>> = _groupIdFlow.flatMapLatest { id ->
repository.getPasswordList(id)
}
4. SingleActivity+MultiFragment+Navigation適用による画面遷移時の依存性を解消
- 画面遷移をIntentとputExtraで値渡しによる結合度の高い画面遷移手法
この問題への対処について記載していきます。
アプリケーションを通して継続してデータを保持する必要があるものをActivityに集約出来ること、Fragmentにする事で画面遷移にNavigationを利用する事ができ、画面遷移を1つのxmlファイルに集約する事ができ、グラフィカルにも画面遷移を確認する事ができます。
また、画面遷移時のデータの受け渡しにはSafeArgsを利用する事ができるため、型安全性を保ったデータの受け渡しをする事ができます。
- プリミティブ型であれば、argTypeに型を指定して、defaultValueも利用可能
<argument
android:name="firstTime"
app:argType="boolean"
android:defaultValue="false" />
- 定義した型であれば、argTypeにクラスを指定して利用可能
<argument
android:name="editData"
app:argType="com.highcom.passwordmemo.ui.PasswordEditData" />
以下の記事で使い方をまとめているので参考にしていただければと思います。
5. ViewBinding+DataBinding適用による画面更新処理の簡素化
- データ更新後のnotifyDataSetChangedに頼ったグローバルな更新通知
この問題への対処について記載していきます。
ViewBindingを利用する事で、findViewByIdを利用しなくても、処理の中で画面の部品に対してアクセスする事ができます。
また、DataBindingを利用する事で、ボタン押下時の処理などのリスナー関数の呼び出しをxml上に直接記載する事ができ、ViewModel側でユースケースのロジックを実行した後で、画面にFlowで通知といった流れを作ることができます。
以下の記事で使い方をまとめているので参考にしていただければと思います。
6. DaggerHilt適用によるDI処理の簡素化
通常の処理では、下位レイヤーで利用するインスタンスは、上位レイヤーでインスタンス化して引数などで渡すことで依存するクラスの処理に対してインスタンスを注入する処理となります。
なので、Repositoryで利用するためのDataBaseクラスをApplicationクラスで生成したり、ViewModelクラスを生成するためのFactoryクラスを用意する必要があったりと実装するのが面倒な事が多いです。
そうのような面倒な処理に対して、DaggerHiltを利用する事でアノテーションを付けるだけで省略して実装する事ができます。
以下の記事で使い方をまとめているので参考にしていただければと思います。
7. JUnitでテストコードを作成して品質の担保
ここまでクラスが分割できればJUnitを利用してテストコードを書く事ができるようになり、今後の改修における品質の担保にも繋がっていきます。
テストコードについては、リポジトリを参照して頂ければと思いますが、ViewModelに対してJUnitを作成する記事を以下でまとめていますので、こちらも参考にして頂ければと思います。
リファクタリング後のコード
リファクタリング後のクラスの全体図は以下のようになります。
修正前よりもクラス数は増えていますが、それぞれが機能としての責務で分離されており、一つひとつのクラス自体はコード量を減らす事ができました。
このように、クラスを責務で分割すること、Jetpackライブラリを利用する事で変更容易性の高い実装に仕上げる事ができ、JUnitを作成する事で品質を担保し続けられるコードとなりました。
まとめ
リファクタリングを進める場合、既存機能と品質を担保しながらどう進めていけば良いのか悩みますが、今回の記事のように下位レイヤー側から上位レイヤー側に向けて順番にリファクタリングしていき、都度動作確認を進めていく事で担保する事ができると思いますので、参考にして頂ければと思います。