リクルートのAndroid リファクタリングコンテストで審査員賞をもらいました

先日(2020/3/20)、リクルートのHOT PEPPER Beautyが主催するAndroidリファクタリングコンテストに参加してきました。

『HOT PEPPER Beauty』 Android リファクタリングコンテスト｜Recruit for engineers

出されたコードを3人1チームで半日ほどリファクタリングしていくコンテストで、ありがたいことに僕たちのチームは審査委員賞を頂きました。

今回は、そこで行ったリファクタ、学んだこと等をまとめたいと思います。

お題のコードが業務で実際に使っているコードをベースにしているらしく、お題のコード、また最終的なコードを見せることは出来ません。ご了承ください。

お題

今回のお題は、HOT PEPPER Beautyの実際のアプリからいくつかの機能を抜粋し、意図的に汚く書き直されたものでした。

非常に規模も小さく、またそこまで汚くもなかったので、わざわざリファクタするほどでも無いのではないかと思ったくらいでしたが、半日で行う規模としては適切だったのかもしれません。

具体的には、3つくらいの画面があり、Activity / Fragmentから直接APIやDBを叩いており、recycler viewのadapter等も含めて全て同一packageに入っている、といった状態でした。

予めIssueに、このあたりを直してほしい等の問題も記載されていました。

戦略

まず、出されたコードを分析し、誰がどこから手を入れていくかの戦略を練りました。

予めIssueに挙げられていたものも含め、Issueを以下の3つに分類しました。

バグ系
- fragmentが再生成されるとclick listenerが外れる
- 詳細画面でブックマークを解除しても、一覧画面に反映されない
アーキテクチャ系
- packageを適切に切りたい
- ViewModel / Repositoryを追加したい
- multi module化したい
- diで配りたい
改善系
- HttpsURLConnectionからretrofitに乗り換えたい
- SQLiteOpenHelperからroomに乗り換えたい
- 強制unwrapをやめたい
- BottomNavigationViewに置き換えたい

方針としては、まず最初にpackageを切り、同時に作業できるようにします。

その次にバグ系は確実に潰していくこと（バグがあるのは良くないことです）、手が空いた人からアーキテクチャ系、改善系のタスクを進めていく、というふうに決めました。

最終的には、大方のIssueは片付けられた形になりました。

僕は主にアーキテクチャ周りを担当したので、そのあたりについてまとめたいと思います。

アーキテクチャとマルチモジュール

基本的には、googleのアプリのアーキテクチャガイドに従った形になります。

viewからロジックを引き剥がすため、ViewModel層を追加し、local dataとremote dataを隠蔽するため、Repository層を追加しました。

各layerは koin で配るようにしてあります。

また、multi moduleも基本的にはそのレイヤー毎に区切るようにしました。

今回は、アプリのサイズ的にもfeatureで切ることはしていません。

ここでポイントとなるのは、RepositoryとRepositoryImplは別のmoduleであるということです。

これにより、UI moduleはrepositoryの実装に影響されず、開発を進めることが出来ます。

また、RepositoryImpl内は全てinternal修飾子を付けており、koinのmoduleだけ公開してあります。

// HogeRepositoryImpl.kt
internal class HogeRepositoryImpl : HogeRepository {
    …
}

// Mapper.kt
internal fun FugaResponse.toModel(): Fuga {
    …
}

// module.kt
val repositoryModule = module {
    single<HogeRepository> { HogeRepositoryImpl() }
}

こうすることで、repositoryの実装に関して、誰も(app moduleすらも)知らないという状態を作り出すことが出来ます。

依存関係の強制、そしてbuildの高速化に役立ってると思います。（実際に計測したわけではありません）

その他細かいこと

いくつかリファクタする上でこだわったtopicsについて紹介します。

resource class

表示内容をAPIから取得する場合、取得されたデータだけでなく、ローディング中やエラー等、様々なデータを複合的に扱わなければならなくなります。

これは難しいので、googleのサンプルにもあるよう、Resource Classを作ることを強くおすすめします。

今回は、sealed classを活用し、以下のようなresource classを用意しました。

sealed class Resource<out T> {
    object Loading : Resource<Nothing>()

    data class Success<out T>(
        val value: T
    ) : Resource<T>()

    data class Fail(
        val error: Throwable
    ) : Resource<Nothing>()
}

このような感じでViewModelで生成します。

class HogeViewModel(
    private val repository: HogeRepository
) : ViewModel() {
    private val _resource = MutableLiveData<Resource<List<Fuga>>>()
    val resource: LiveData<Resource<List<Fuga>>> = _resource

    init {
        fetch()
    }

    fun fetch() {
        viewModelScope.launch {
            _resource.value = Resource.Loading
            _resource.value = try {
                val result = repository.getSalonList()
                Resource.Success(result)
            } catch (e: Throwable) {
                Resource.Fail(e)
            }
        }
    }
}

activityの方ではこんな感じでobserveします。

class HogeActivity : AppCompatActivity() {
    private val viewModel: HogeViewModel by viewModel()

    override fun onCreate(savedInstanceState: Bundle?) {
        viewModel.resource.observe(this) {
            loading.setVisibleGone(it is Resource.Loading)
            network_error.setVisibleGone(it is Resource.Fail)
            recycler.setVisibleGone(it is Resource.Success)
            if (it is Resource.Success) {
                showCount(it.value.size)
                showList(it.value)
            }
        }
    }
}

初回エラー時と2回目以降のエラー時で挙動を変えたかったり、refreshを考慮する場合、もう少しresource classの形は変わると思います。

UseCase層がある場合は、そこでresourceに変換しても良いと思います。

kotlin coroutines flow

LiveDataの登場によって、ViewModelとViewのbindingが劇的に簡単になりました。

一方、LiveDataが苦手とするのが、イベントの通知です。

LiveDataはobserve時に最後の値をreplayしてしまうため、toastの表示等のイベントを流すのには向いていません。

googleも SingleLiveEvent のようなクラスを作ることでお茶を濁してきましたが、kotlin coroutinesがlifecycleに対応した今なら、flowを使うのが良いかもしれません。

今回は、ブックマーク後、ブックマーク解除後に表示するtoastの表示をflowを使ってViewModelからViewにイベントを流すようにしました。

class DetailViewModel(
    private val repository: BookmarkRepository
) : ViewModel() {
    private val _toastMessage = BroadcastChannel<String>(Channel.BUFFERED)
    val toastMessage get() = _toastMessage.asFlow()

    fun switchBookmark() {
        viewModelScope.launch {
            if (isBookmarked) {	
                repository.unBookmark()
                _toastMessage.send(“ブックマークから削除しました”)
            } else {
                repository.bookmark()
                _toastMessage.send(“ブックマークに追加しました”)
            }
        }
    }
}

activityでは LifecycleScope をつかってObserveします。

class DetailActivity : AppCompatActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        viewModel.toastMessage.onEach {
            Toast.makeText(this, it, Toast.LENGTH_SHORT).show()
        }.launchIn(lifecycleScope)
    }
}

lifecycle scopeはonDestory時に正しくscopeをcancelしてくれるため、メモリリーク等の心配はありません。