Rxなインスタンスを返却するメソッドを実装する時の注意点

そういえばよくレビューで指摘してたなぁと思い出したのでメモ書き。

例えばこういうコード。

fun findContent(): Maybe<Content> {
    val connectivityManager = application.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
    return if (connectivityManager.activeNetworkInfo.isConnected) {
        apiRepository.find()
    } else {
        localRepository.find()
    }
}

Content を取得したい時に、オンラインだったら apiRepository を使って、オフラインだったら localRepository を使うというコード。

このコードにはいくつか問題があって、 as じゃなくて as? を使った方が安全とかは間違いなくそうなんだけど、それよりも オンラインかオフラインか の判定ロジックが Rxな世界に包まれていないところが問題だと思っている。

オンラインかオフラインか の場所で Throwable が投げられる可能性がある場合、 これを使っている側がしっかり onError を実装していたとしてもで落ちてしまう。それも、悪質なのは subscribe している箇所ではなく findContent を呼んでいる箇所で、だ。 いうまでもなく subscribe されなくてもこの判定ロジックは処理されてしまう。

もし Throwable が投げられる可能性がないとしても、判定に時間がかかったりすることもあり、想定と違うものになりかねない。

なので、最終的に返すもの以外のロジックすべてをちゃんとRxな世界で包んであげましょう。

自分の場合だと下記みたいに書くと思う。

fun findContent(): Maybe<Content> =
    isConnected().flatMapMaybe { isConnected ->
                    if (isConnected) {
                        apiRepository.find()
                    } else {
                        localRepository.find()
                    }
                }

private fun isConnected(): Single<Boolean> = Single.create { emitter ->
    val connectivityManager = application.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager
    if (connectivityManager != null) {
        emitter.onSuccess(connectivityManager.activeNetworkInfo.isConnected)
    } else {
        emitter.onError(RuntimeException("cannot get connectivity"))
    }
}