Skip to main contentSkip to footer
Search
LINEヤフー Tech Blog

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

DroidKaigi 2024 Code Bug Fix Challenge 3問目

こんにちは。Androidアプリエンジニアの三浦と安藤としゅーぞーです。

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

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

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

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

Only Get One Shot at Compose

@Composable
fun ListScreen(
    logger: Logger = DefaultLogger(),
    viewModel: ListViewModel = hiltViewModel(),
    lifecycleOwner: LifecycleOwner = LocalLifecycleOwner.current
) {
    val uiState by viewModel.uiState.collectAsStateWithLifecycle()
    Scaffold(
        topBar = {
            // (3)
            var expanded by remember { mutableStateOf(false) }
            TopAppBar(
                title = {},
                modifier = Modifier
                    .animateContentSize()
                    .height(if (expanded) 200.dp else 100.dp)
                    .fillMaxWidth()
                    .clickable(
                        interactionSource = remember { MutableInteractionSource() },
                        indication = null
                    ) {
                        expanded = !expanded
                    },
                colors = TopAppBarDefaults.topAppBarColors(MaterialTheme.colorScheme.primaryContainer),
            )
        }
    ) { innerPadding ->
        LazyColumn(
            modifier = Modifier
                .fillMaxSize()
                .padding(innerPadding)
                .background(Color.White)
        ) {
            item {
                Button(
                    onClick = { viewModel.addItem() }
                ) {
                    Text("Add Item")
                }
            }
            items(uiState.item) { item ->
                ListItem(
                    imageUrl = item.imageUrl,
                    title = item.title,
                    subtitle = item.subtitle
                )
            }
        }
    }
    //  (1)
    SideEffect { logger.sendPageView() }

    // (2)
    LaunchedEffect(Unit) {
        val observer = LifecycleEventObserver { _, event ->
            when (event) {
                Lifecycle.Event.ON_RESUME -> {
                    viewModel.countDisplayed()
                }

                else -> {}
            }
        }
        lifecycleOwner.lifecycle.addObserver(observer)
    }
}

@Composable
fun ListItem(
    imageUrl: String,
    title: String,
    subtitle: String,
    context: Context = LocalContext.current,
    configuration: Configuration = LocalConfiguration.current,
    screenWidthDp: Dp = configuration.screenWidthDp.dp
) {
    val padding = if (screenWidthDp > 600.dp) 16.dp else 8.dp
    // (4)-1
    Row(
        modifier = Modifier
            .fillMaxWidth()
            .background(color = MaterialTheme.colorScheme.primaryContainer)
            .padding(padding)
    ) {
        AsyncImage(
            model = imageUrl,
            error = painterResource(id = R.drawable.ic_no_image),
            contentDescription = null,
            // (4)-2
            modifier = Modifier
                .size(64.dp)
                .background(color = Color.LightGray)
                .padding(12.dp)
                .clickable {
                    Toast.makeText(context, "Toast", Toast.LENGTH_SHORT).show()
                },
            contentScale = ContentScale.Crop,
        )
        Column(
            modifier = Modifier.padding(start = padding)
        ) {
            Text(
                text = title,
                color = Color.White,
                fontSize = 16.sp,
                fontWeight = FontWeight.Bold
            )
            Text(
                text = subtitle,
                color = Color.White,
                fontSize = 14.sp
            )
        }
    }
}

出題意図

Jetpack Composeを用いて画面を作成する際に使用するAPIにおいて"動作はするが何か挙動がおかしい"ようなケースを中心に出題しています。

コメント

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

この問題には多くのコメントをいただきました。以下にいくつかピックアップします。

  • Observerが正しくremoveされないため、メモリリークの可能性がありそう
  • 意図する動作と異なるSideEffectの使用
  • lifecycleOwner や context をComposableのデフォルト引数として定義しているのが気になる
  • 各Composableでの context の引き回し方が気になる
  • Composeのテストはスクリーンショットテストが有効
  • インテグレーションテスト・UIテストも実施できればさらに良さそう

ボードの写真

ボードの写真

不具合を引き起こしそうな箇所

実際の出題コードから課題とその原因を詳しく見てみましょう。

(1) side-effect APsIの選択ミスによる仕様と合わないログ送信

このアプリではロガーを用いてページの表示回数を取得しています。
logger.sendPageView関数を用いて画面起動時にログを1回送信することを意図してコードを作成しています。
しかし、SideEffectを使用しているため、recomposeの度にログを送信してしまう不具合が混入しています。

ページビューなどの初回生成時や特定の値の変更時に動作してほしい場合はLaunchedEffectを、画面のパフォーマンスチェックなどの理由やrecomposeの度に動作してほしい場合はSideEffectを使用しましょう。

LaunchedEffect(Unit) { logger.sendPageView() }

Instrumentation testで実際に画面起動時以外にロガーの関数が呼び出されないことを確認することでUnit testからこの問題を検出することができそうです。
また、APIの仕様を理解していれば、レビュー時点で問題に気づくことができるかもしれません。

(2) Observerの破棄忘れによるメモリリークの可能性

この画面ではライフサイクルに依存した値のカウントをする必要があります。そのため、ステートがRESUMEに変化する度にViewModelの関数を用いて値を変化させようとしています。
そのため、LaunchedEffectとLifecycleEventObserverを用いてviewModel.countDisplayedを呼び出します。
しかし、LifecycleEventObserverを破棄していません。これはメモリリークの原因になる可能性があります。正しくobserverの破棄を行いましょう。
このように画面の破棄されるタイミングでコードを実行する必要がある場合、DisposableEffectを使用することで簡単に記述することができます。

DisposableEffect(lifecycleOwner) {
    val observer = LifecycleEventObserver { _, event ->
        when (event) {
            Lifecycle.Event.ON_RESUME -> { viewModel.countDisplayed() }
            else -> {}
        }
    }
    lifecycleOwner.lifecycle.addObserver(observer)
    onDispose {
        lifecycleOwner.lifecycle.removeObserver(observer)
    }
}

この問題はUnit testでの検出が難しそうです。
1つのアイデアとしてlinterを用いてaddObserver, removeObserverがそれぞれ呼び出されているかどうかを確認することはできるかもしれませんが、複数のobserverが混ざったケースを考えると頭が痛くなりそうです。

(3) remember APIsの選択ミスによる回転時の意図しない画面初期化

この画面にはTopAppBarが実装されており、タップすると表示領域が拡大・縮小します。
この挙動はrememberを用いてexpandedという変数で表現されています。
しかし、ユーザが意図して拡張したヘッダーが端末の回転などを行った際に勝手に縮小してしまいます。
これはrememberで保持されている変数が画面の回転時（configuration change）に初期化されるためです。
このような場合にはrememberではなくrememberSavableを使用する必要があります。

var expanded by rememberSaveable { mutableStateOf(false) }

rememberはrecompose時に値を保持するのに対し、rememberSavableはconfiguration change時に値を保持します。仕様に合わせて正しく使用しましょう。
この問題もUnit Testでの検出が難しそうです。なぜならAPIとその使用方法自体は間違っておらず、仕様とコードに相違があるかどうかを検知しなくてはならないからです。
1つのアイデアとしてはInstrumentation testにて回転時の挙動をチェックすることで仕様との相違に気づくことはできると思います。

(4) Modifierの順序による画面装飾のミス

この画面で意図しているUIの仕様は次の通りです。

  • List Itemについて
    • List Itemの上下左右のpaddingは8.dp（画面幅が大きい場合は16.dp）
    • List Itemのbackgroundに対してマテリアルテーマのprimaryContainerに設定
  • List Item内の画像部分について画像部分の全体をタップ可能領域とする

しかし、Modifierの順序ミスにより、以下の挙動となっており仕様と乖離（かいり）してしまっています。

  • List Itemのbackgroundは上下左右のpadding部分も含めてマテリアルテーマのprimaryContainerが設定されてしまっている
  • List Item内の画像部分は上下左右のpadding部分を除いた箇所のみがタップ可能領域になってしまっている
/*** AS-IS ***/
// (4)-1
modifier = Modifier
        .fillMaxWidth()
        .background(color = MaterialTheme.colorScheme.primaryContainer)
        .padding(padding)

// (4)-2
modifier = Modifier
        .size(64.dp)
        .background(color = Color.LightGray)
        .padding(12.dp)
        .clickable { Toast.makeText(context, "Toast", Toast.LENGTH_SHORT).show() }

/*** TO-BE ***/
// (4)-1
modifier = Modifier
        .fillMaxWidth()
        .padding(padding)
        .background(color = MaterialTheme.colorScheme.primaryContainer)

// (4)-2
modifier = Modifier
        .size(64.dp)
        .background(color = Color.LightGray)
        .clickable { Toast.makeText(context, "Toast", Toast.LENGTH_SHORT).show() }
        .padding(12.dp)

現状のJetpack ComposeのUnit testのライブラリではModifierの順序を検証する方法はないため、この問題もUnit testでの検出は難しいと思います。
不具合の検出方法は、Instrumentation testにてヴィジュアル仕様（Figmaなど）と照合するしかないかと思います。
ComposeのPreview機能を有効活用することで、コンポーネント単位で見た目を確認することができます。これによりアプリのビルドを行わずにUIの照合ができるので非常にオススメです。

想定解答

修正コード

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

@Composable
fun ListScreen(
    logger: Logger = DefaultLogger(),
    viewModel: ListViewModel = hiltViewModel(),
    lifecycleOwner: LifecycleOwner = LocalLifecycleOwner.current
) {
    val uiState by viewModel.uiState.collectAsStateWithLifecycle()
    Scaffold(
        topBar = {
            var expanded by rememberSaveable { mutableStateOf(false) }
            TopAppBar(
                title = {},
                modifier = Modifier
                    .animateContentSize()
                    .height(if (expanded) 200.dp else 100.dp)
                    .fillMaxWidth()
                    .clickable(
                        interactionSource = remember { MutableInteractionSource() },
                        indication = null
                    ) {
                        expanded = !expanded
                    },
                colors = TopAppBarDefaults.topAppBarColors(MaterialTheme.colorScheme.primaryContainer),
            )
        }
    ) { innerPadding ->
        LazyColumn(
            modifier = Modifier
                .fillMaxSize()
                .padding(innerPadding)
                .background(Color.White)
        ) {
            item {
                Button(
                    onClick = { viewModel.addItem() }
                ) {
                    Text("Add Item")
                }
            }
            items(uiState.item) { item ->
                ListItem(
                    imageUrl = item.imageUrl,
                    title = item.title,
                    subtitle = item.subtitle
                )
            }
        }
    }
    LaunchedEffect(Unit) { logger.sendPageView() }

    DisposableEffect(lifecycleOwner) {
        val observer = LifecycleEventObserver { _, event ->
            when (event) {
                Lifecycle.Event.ON_RESUME -> {
                    viewModel.countDisplayed()
                }

                else -> {}
            }
        }
        lifecycleOwner.lifecycle.addObserver(observer)
        onDispose {
            lifecycleOwner.lifecycle.removeObserver(observer)
        }
    }
}

@Composable
fun ListItem(
    imageUrl: String,
    title: String,
    subtitle: String,
    context: Context = LocalContext.current,
    configuration: Configuration = LocalConfiguration.current,
    screenWidthDp: Dp = configuration.screenWidthDp.dp
) {
    val padding = if (screenWidthDp > 600.dp) 16.dp else 8.dp
    Row(
        modifier = Modifier
            .fillMaxWidth()
            .padding(padding)
            .background(color = MaterialTheme.colorScheme.primaryContainer)
    ) {
        AsyncImage(
            model = imageUrl,
            error = painterResource(id = R.drawable.ic_no_image),
            contentDescription = null,
            modifier = Modifier
                .size(64.dp)
                .background(color = Color.LightGray)
                .clickable {
                    Toast
                        .makeText(context, "Toast", Toast.LENGTH_SHORT)
                        .show()
                }
                .padding(12.dp),
            contentScale = ContentScale.Crop,
        )
        Column(
            modifier = Modifier.padding(start = padding)
        ) {
            Text(
                text = title,
                color = Color.White,
                fontSize = 16.sp,
                fontWeight = FontWeight.Bold
            )
            Text(
                text = subtitle,
                color = Color.White,
                fontSize = 14.sp
            )
        }
    }
}

おわりに

Composeでの挙動やレイアウトの課題を中心に出題いたしましたが、ユニットテストだけでは担保しきれない問題が多くありました。
ユニットテストで検証できる部分をしっかりと行うことは当然としても、Screenshot testやInstrumentation test等を用いて多角的な検証が必要です。
また、複雑なロジックによって引き起こされるバグではない場合、実装上のルールやLintによるチェックも有効です。

次回の解説記事もお楽しみに！

Name:三浦 裕騎

Description:Android エンジニア。Yahoo! JAPANアプリを開発しています。デザインシステムの構築などにも携わっています。

Name:Yuki Ando / 安藤 祐貴

Description:Androidエンジニア。LINE Androidアプリに対してプラットフォームが提供する機能の適用をしています。車に乗ることが大好きです。

Name:しゅーぞー / shuzo

Description:Yahoo! JAPANアプリ Androidアプリエンジニア UIデザイナー。DroidKaigi 2024にてデザインシステムについて発表。