こんにちは。AndroidアプリエンジニアのTakanashiとShimizuです。
先日開催されたDroidKaigi 2024のLINEヤフー企業ブースでは、「Code Bug Fix Challenge」を実施しました。
「Code Bug Fix Challenge」とは、問題コードに含まれるバグを見つけ、そのバグを修正してもらったり、将来のバグを防ぐためにはどんなテストコードを書けばいいのかを、来場者に考えてもらう企画です。
ブースに来ていただいた方には意見を書いた付箋を貼ってもらい、その意見をもとにLINEヤフーの開発者と交流しました。
本記事では、今回出題した「Code Bug Fix Challenge」の1問目の解説をします。
The Complete Works of Incorrect Elements
出題コード
1問目では、以下のコードが出題されました。このコードではIOスレッドでユーザーのデータを取得し、フィルタとソートをした後に、UIにデータを表示します。また、データの処理中はローディングのViewを表示したり、最近更新されたユーザーのデータをハイライトするといった処理も行います。
data class UserData(
val name: String,
val isBlocked: Boolean,
val updateTimeMillis: Long
)
suspend fun showFriends() {
showLoadingView()
val users = getUsers()
val friends = excludeBlocked(users)
sortUsers(friends.toMutableList())
showUI(friends)
highlightRecentlyUpdated(friends)
hideLoadingView()
}
private suspend fun getUsers(): List<UserData> = withContext(Dispatchers.IO) {
TODO("Returns user list")
}
private suspend fun excludeBlocked(
users: List<UserData>
): List<UserData> = withContext(Dispatchers.Default) {
val mutableUsers = users.toMutableList()
for (i in 0..<mutableUsers.size) {
if (mutableUsers[i].isBlocked) {
mutableUsers.removeAt(i)
}
}
mutableUsers
}
/**
* Sorts [users] by bubble sort in descending order of [UserData.updateTimeMillis].
*/
private suspend fun sortUsers(users: MutableList<UserData>) = withContext(Dispatchers.Default) {
for (i in 0..<(users.size - 1)) {
for (j in 1..(users.size - i)) {
if (users[j - 1].updateTimeMillis > users[j].updateTimeMillis) {
users.swap(j - 1, j)
}
}
}
}
private fun <T> MutableList<T>.swap(i: Int, j: Int) {
val tmp = get(i)
set(i, get(j))
set(j, tmp)
}
private fun showLoadingView(): Unit = TODO()
private fun hideLoadingView(): Unit = TODO()
private fun showUI(friends: List<UserData>): Unit = TODO("Show friends in UI")
/**
* Highlights friends updated from one week ago to today.
*/
private fun highlightRecentlyUpdated(friends: List<UserData>) {
val currentTimeMillis = System.currentTimeMillis()
val highlightTarget = friends.takeWhile {
(currentTimeMillis - it.updateTimeMillis).milliseconds.inWholeDays <= 7
}
// TODO: highlight data with highlightTarget
}出題意図
リストや配列などのCollectionを操作して表示する場面において、初心者が陥りやすいミスを中心に出題しています。
コメント
ブースでいただいたコメント
この問題には多くのコメントをいただきました。以下にいくつか抜粋します。
出題コードに含まれるバグについて
- 10行目(
val users = getUsers())~15行目(highlightRecentlyUpdated(friends))でエラーが発生したときにLoadingView()が消えない toMutableList()は別インスタンスを返すのでソートされないusers.filter { !it.isBlocked }で良いremoveAt()でlistのindexがずれる。それで全userに対して処理されないsortByDescending { it.updateTimeMillis }済むので手でソートを書かなくて良い
どんなテストコードを書けばいいのか
UserDataに関するロジックを内部にメソッドとして持たせれば、よりテスト容易設計になりそうSystem.currentTiemMillis()を関数内で使うとTestを書きにくい。DIや引数から時制を渡したい
ボードの写真
解説
出題したコード上のバグについて、関数ごとに説明をします。
showFriends()
showFriends() 関数はsuspend関数のため、処理の途 中で中断される可能性があります。中断が発生すると、 hideLoadingView() が呼ばれず、ローディングビューが表示されたままになります。この問題を解決するためには、suspend関数の中断を考慮し、try-finallyを使って必ず hideLoadingView() が呼ばれるようにします。
excludeBlocked()
リストをループ中に要素を削除すると、リストのサイズが変わり、インデックスがずれてしまいます。その結果、正しくない要素が削除される、または削除されるべき要素がスキップされる可能性があります。このバグは、逆順にインデックスを回したり、 filter() 関数を使って条件に合致する要素だけを残した新しいリストを返すことで防ぐことができます。
sortUsers()
バブルソートを用いてユーザーのリストをソートする関数ですが、以下の問題点があります。
- 問題点1: 最後のインデックスが範囲外を指してしまう
具体的には、内側のループでjが1からusers.size - iまで回るため、users[j]が範囲外になることがあります。内側のループの範囲を修正して、最後のインデックスが範囲外にならないようにします。 - 問題点2: 関数のコメントに「降順にソート」とあるが、実際には昇順でソートしている
if条件の不等号が逆になっているため、意図したソート順になっていません。降順にソートするために、不等号の向きを修正します。 - 問題点3: 引数のfriendsリストが変更されない
friends.toMutableList()を呼ぶと、インスタンスのコピーが作成され、それが引数として渡されるため、元のfriendsリ ストは変更されません。コピーを呼び出し側で保持するか、sortUsers()がソート済みのリストを返すように実装を変更します。
また、Kotlinの標準ライブラリを使ってソートを行うことで、コードを簡潔にし、バグを防ぐことができます。
highlightRecentlyUpdated()
現在の実装では、ミリ秒単位での差分を計算してから、それを日付の単位に変換しています。この方法では、ちょうど一週間前に更新されたデータ（たとえば7日と1時間前に更新されたデータ）が、時刻によってはハイライトされない可能性があります。先に日付の単位に変換してから差分を取ることで、正確な日付差分を計算し、ちょうど一週間前に更新されたデータも正しくハイライトされるようになります。
想定解答
修正コード
混入させた不具合を修正したコードです。
data class UserData(
val name: String,
val isBlocked: Boolean,
val updateTimeMillis: Long
)
suspend fun showFriends() {
showLoadingView()
try {
val users = getUsers()
val friends = excludeBlocked(users)
val sortedFriends = sortedUsers(friends)
showUI(sortedFriends)
highlightRecentlyUpdated(sortedFriends)
} finally {
hideLoadingView()
}
}
private suspend fun getUsers(): List<UserData> = withContext(Dispatchers.IO) {
TODO("Returns user list")
}
private suspend fun excludeBlocked(users: List<UserData>): List<UserData> =
withContext(Dispatchers.Default) { users.filterNot { it.isBlocked } }
/**
* Sorts [users] by sort in descending order of [UserData.updateTimeMillis].
*/
private suspend fun sortedUsers(users: List<UserData>): List<UserData> =
withContext(Dispatchers.Default) { users.sortedByDescending { it.updateTimeMillis } }
private fun showLoadingView(): Unit = TODO()
private fun hideLoadingView(): Unit = TODO()
private fun showUI(friends: List<UserData>): Unit = TODO("Show friends in UI")
/**
* Highlights friends updated from one week ago to today.
*/
private fun highlightRecentlyUpdated(friends: List<UserData>) {
val currentDate = LocalDate.now()
val highlightTarget = friends.takeWhile {
val updateDate = Instant.ofEpochMilli(it.updateTimeMillis).atZone(ZoneId.systemDefault()).toLocalDate()
ChronoUnit.DAYS.between(updateDate, currentDate) <= 7
}
// TODO: highlight data with highlightTarget
}テスト
どんなテストコードを書けばいいのかについても、関数ごとに説明をします。
excludeBlocked()
isBlocked が true のユーザーが除外され false のユーザーが含まれることを確認するだけでなく、空のユーザーリストの場合や全ユーザーがブロックされている（ブロックされていない）場合のテストデータを準備してテストを網羅的に実施します。
sortedUsers()
updateTimeMillis が降順にソートされていることを確認するほか、空のユーザーリストの場合や同じ値の updateTimeMillis を持つユーザーが複数いる場合のテストデータを準備してテストを実施します。
highlightRecentlyUpdated()
このバグは境界値を用いてテストをすれば検出できます。たとえば、境界値のテストデータとして、更新日がちょうど1週間前、1週間と1時間前、1週間と1時間後のユーザーを含めます。
更新日がちょうど1週間前のテストデータに対するテスト実行結果は、テスト対象コードのメソッド呼び出し時刻と、ユニットテストのメソッド呼び出し時刻に依存します。そこでこの依存性を排除するため、 highlightRecentlyUpdated() 関数内の LocalDate.now() の部分にDIを適用して、更新日がちょうど1週間前の場合のテストを実施します。
おわりに
「Code Bug Fix Challenge」の1問目について解説しました。
この記事では、Collection操作におけるよくあるミスとその解決策を紹介しました。ループ中の要素削除によるインデックスずれや、ソートの範囲外 アクセス、日付差分計算の誤り、suspend関数の中断処理における注意点を解説し、Kotlinの標準ライブラリを活用することで、コードの簡潔化とバグ防止が可能であることを示しました。
また後日、2問目以降の出題についても解説記事を投稿予定です。お楽しみに！