LINEヤフー Tech Blog

LINEヤフー株式会社のサービスを支える、技術・開発文化を発信しています。

DroidKaigi 2024 Code Bug Fix Challenge 4問目

こんにちは。AndroidアプリエンジニアのOishi、Yamada、Murakamiです。

先日開催されたDroidKaigi 2024のLINEヤフー企業ブースでは、「Code Bug Fix Challenge」を実施しました。

「Code Bug Fix Challenge」とは、問題コードに含まれるバグを見つけ、そのバグを修正してもらったり、将来のバグを防ぐためにはどんなテストコードを書けばいいのかを来場者に考えてもらう企画です。

ブースに来ていただいた方には意見を書いた付箋を貼ってもらい、その意見をもとにLINEヤフーの開発者と交流しました。

本記事では、今回出題した「Code Bug Fix Challenge」の4問目の解説をします。

The Enigma of Initialization

出題コード

4問目では、以下のコードが出題されました。

class FooActivity : AppCompatActivity() {
 
    private val fooFeatureManager = FooFeatureManager(this)
 
    private val fooViewModel: FooViewModel by viewModels {
        viewModelFactory {
            initializer {
                FooViewModel(fooFeatureManager)
            }
        }
    }
 
    override fun onNewIntent(intent: Intent) {
        super.onNewIntent(intent)
        fooFeatureManager.handleNewIntent(intent)
    }
    // snip...
}
 
class FooViewModel(
    private val fooFeatureManager: FooFeatureManager
) : ViewModel() {
    init {
        viewModelScope.launch {
            fooFeatureManager.getEventFlow().collect { event ->
                handleEvent(event)
            }
        }
    }
 
    private val isReadyMutableFlow = MutableStateFlow(false)
    val isReadyFlow: StateFlow<Boolean> = isReadyMutableFlow.asStateFlow()
 
    private fun handleEvent(event: Event) {
        when (event) {
            is Event.Ready -> {
                isReadyMutableFlow.value = true
            }
            // snip...
        }
    }
}
 
class FooFeatureManager(private val context: Context) {
    fun handleNewIntent(intent: Intent) {
        TODO()
    }
    fun getEventFlow(): Flow<Event> = TODO()
}

出題意図

オブジェクトの生成やライフサイクルに関して起こりがちなミスを議題にして出題しました。

コメント

ブースでいただいたコメント

この問題でいただいたコメントを以下にいくつか抜粋します。

出題コードに含まれるバグについて

  • FooFeatureManagerActivityContextを渡しているのでメモリーリークする
  • isReadyMutableFlowがnullの可能性がある
  • ON_STARTED以降を想定していた場合にクラッシュする
  • collectを呼んですぐにイベントが来るとStateFlowinitより前にhandleEvent()が実行されてしまう
  • init前にhandleEvent()を呼ぶ

どんなテストコードを書けばいいのか

  • handleNewIntent()を呼んでhandleEvent()の処理が呼ばれるかのテストを書く
  • DIする

ボードの写真

ボードの写真

解説

出題したコード上のバグについて説明をします。

FooFeatureManager のインスタンスが Activity と ViewModel で共有されている

対象箇所(コード)

class FooActivity : AppCompatActivity() {

    private val fooFeatureManager = FooFeatureManager(this)

    private val fooViewModel: FooViewModel by viewModels {
        viewModelFactory {
            initializer {
                FooViewModel(fooFeatureManager)
            }
        }
    }
}

class FooViewModel(
    private val fooFeatureManager: FooFeatureManager
) : ViewModel()

解説

ViewModelActivityよりも生存期間が長く、recreateされても生き残るので、その場合はFooFeatureManagerのインスタンスがリークしてしまう問題が発生します。また、Activityで使われるロジックとViewModelで使われるロジックを同じFooFeatureManagerというクラスに書くのは設計的にあまりよい方法とは言えません。"~Manager"というクラス名自体を避けるよう設計をすることが推奨されます。

解決策

解決策としてFooViewModelfactoryFooViewModel自身のcompanion objectに持たせるようにすることで、誤ってActivityと同じライフサイクルのインスタンスをViewModelに渡してしまうことを防ぎ、同様の問題が発生しなくなるようにできます。

FooViewModelのinit内のFlowのcollectにて、Flowが即座に値をemitしたときにコンストラクタ中にhandleEvent()が呼ばれてNullPointerExceptionが発生する

対象箇所(コード)

class FooViewModel(
    private val fooFeatureManager: FooFeatureManager
) : ViewModel() {
    init {
        viewModelScope.launch {
            fooFeatureManager.getEventFlow().collect { event ->
                handleEvent(event)
            }
        }
    }

    private fun handleEvent(event: Event) {
        when (event) {
            is Event.Ready -> { 
                isReadyMutableFlow.value = true 
            }
            // snip...
        }
    }
}

解説

FooViewModelinit内でFlowcollectを行っていますが、このFlowが即座に値をemitしたときはコンストラクタ中にhandleEvent()が呼ばれることになります。その場合、FooViewModelのプロパティisReadyMutableFlowは未初期化なのでisReadyMutableFlow.value = trueのところでNullPointerExceptionが発生してしまいます。

解決策

解決策としてFooViewModelのコンストラクタでコルーチンをlaunchするのをやめ、別途start()関数を作ってそれをfactoryから呼び出すようにすると同様の問題が発生しなくなります。

想定解答

修正コード

混入させた不具合を修正したコードです。

class FooActivity : AppCompatActivity() {

  private val fooViewModel: FooViewModel by viewModels {
    FooViewModel.factory
  }
  // snip...
}

class FooViewModel(
  private val fooFeatureManager: FooFeatureManager
) : ViewModel() {
  // snip...

  private fun start() {
    viewModelScope.launch {
      fooFeatureManager.getEventFlow().collect { event ->
        handleEvent(event)
      }
    }
  }

  companion object {
    val factory: ViewModelProvider.Factory = viewModelFactory {
      initializer {
        val application = checkNotNull(get(APPLICATION_KEY))
        val fooFeatureManager = FooFeatureManager(context = application)
        FooViewModel(fooFeatureManager).also {
          it.start()
        }
      }
    }
  }
}

おわりに

「Code Bug Fix Challenge」の4問目について解説しました。

この記事では、オブジェクトの生成やライフサイクルに関してのよくあるミスとその解決策を紹介しました。

DroidKaigi 2024 Code Bug Fix Challengeのその他の問題の解説をまだ読んでない方はぜひそちらも確認してみてください。

全4回にわたり、問題解説をお届けしました。バグ視点の気づきは得られましたでしょうか?
いずれまたこのような企画を開催したいですが、その際は現地でお会いできることを楽しみにしております。

Name:村上 圭

Description:Androidエンジニア。Yahoo! メール Androidアプリの開発をしています。

Name:山田 真一

Description:Androidエンジニア。Yahoo! JAPANアプリの開発をしています。

Name:大石 将邦

Description:Androidエンジニア。Kotlinとマルチスレッドプログラミングが好きです。LINE Engineering blog 時代の記事は下のアイコンから