【DroidKaigi 2023】Code Review Challengeの概要と解説まとめ

LINE Engineering2023-09-21

LINE Engineering Blog official account

2022年9月14日(木)〜16日(土)の3日間にわたって開催された、DroidKaigi 2023にてLINEはゴールドスポンサーを務めました。

会期中はLINEのブースにたくさんの方にお越し頂き、誠にありがとうございました。

本ブログでは、イベント当日2日間実施した「Code Review Challenge」の解説をはじめ、LINEのコードレビューの取り組みや文化についてご紹介いたします。

「Code Review Challenge」に参加してくださった方も、そうでない方も、ぜひ本ブログで再チャレンジしてみて下さい。

Code Review Challengeの概要

DroidKaigi 2023 のイベント当日、「Bad Code」を「Good Code」に変える、Code Review Challengeを実施しました。
この問題は、LINEの社内でおこなっているレビュー会で紹介されたBad Codeの書き方を参考に、エンジニアが一から考えたオリジナルの問題※になります。

2日間、来場される方に何度も楽しんでいただけるように全部で5つの問題を出題しました。

Schedule

Day1
- 10:00-12:30 出題1
- 12:30-15:00 出題2
- 15:00-18:00 出題3
Day2
- 10:00-14:00 出題4
- 14:00-17:00 出題5

※この企画のために、LINEのエンジニアが考えたコードです。LINEで提供されるサービスの実際のコードではありません。

Code Review Challenge 問題について

解説は後日順次公開とさせていただく予定です。解説ブログが出るまでは、「Bad Code」を「Good Code」に変える、Code Review Challenge に取り組んでみてください。

Reviewの内容はハッシュタグ #CodeReviewChallenge をつけてTwitterでぜひ呟いてください。みんなでReviewをしてより良いコードを世に増やしていきましょう。

【出題1】Day1 10:00-12:30

この問題は「 Yuta Takanashi 」が作りました！

所属：LINEアプリ基盤機能開発チーム

解説ブログはこちら

https://engineering.linecorp.com/ja/blog/droidkaigi2023_code_review_challenge_1

object BackupCreator {
    private val localDataSource = LocalDataSource()
    private val remoteDataSource = RemoteDataSource()
    private lateinit var progressDialog: ProgressDialogForBackup

    @WorkerThread
    fun createBackup(target: BackupTarget, since: Date) {
        showProgressDialog()
        uploadBackup(target, since)
        hideProgressDialog()
    }

    private fun uploadBackup(target: BackupTarget, since: Date) {
        compressBackupData(target, since)
        remoteDataSource.upload(File("path/compressed"))
    }

    private fun compressBackupData(target: BackupTarget, since: Date) {
        collectBackupData(target, since)
        compress(source = File("path/raw"), destination = File("path/compressed"))
    }

    private fun collectBackupData(target: BackupTarget, since: Date) {
        val data = when (target) {
            BackupTarget.IMAGE -> localDataSource.selectImageData()
            BackupTarget.VIDEO -> localDataSource.selectVideoData()
            BackupTarget.MESSAGE -> localDataSource.selectMessageData(since)
        }
        writeToFile(File("path/raw"), data)
    }

    private fun showProgressDialog() {
        progressDialog = ProgressDialogForBackup()
        progressDialog.showOnMainThread()
    }

    private fun hideProgressDialog() {
        progressDialog.hideOnMainThread()
    }

    private fun writeToFile(file: File, data: BackupData) {
        val outputStream = file.outputStream()
        try {
            outputStream.write(data.toByteArray())
        } catch (e: IOException) {
            // Ignore
        }
        outputStream.close()
    }

    private fun compress(source: File, destination: File): Unit = // Throws [IOException] If any I/O error occurred.
        TODO("Write data from `source` to `destination` with compression.")
}

enum class BackupTarget { IMAGE, VIDEO, MESSAGE }

class RemoteDataSource {
    fun upload(file: File): Unit = TODO("Upload file to server.") // Throws [IOException] If any I/O error occurred.
}

class LocalDataSource {
    fun selectImageData(): BackupData = TODO("Select image data.")
    fun selectVideoData(): BackupData = TODO("Select video data.")
    fun selectMessageData(since: Date): BackupData = TODO("Select message data created after `since`")
}

【出題2】Day1 12:30-15:00

この問題は「 Atsushi Mori 」が作りました！

所属：メッセンジャープロダクトアプリ開発1チーム

解説ブログはこちら

https://engineering.linecorp.com/ja/blog/droidkaigi2023_code_review_challenge_2

@Composable
fun LoginScreen() {
    Column(
        modifier = Modifier
            .padding(DEFAULT_SIZE.dp)
    ) {
        Spacer(
            modifier = Modifier.weight(2f)
        )
        title()
        Spacer(
            modifier = Modifier.weight(1f)
        )
        val email = email()
        Spacer(
            modifier = Modifier
                .height(DEFAULT_SIZE.dp)
        )
        val password = password()
        Spacer(
            modifier = Modifier.weight(2f)
        )
        loginButton(
            email = email,
            password = password
        )
    }
}
 
@Composable
private fun title() {
    Text(
        text = "DroidKaigi 2023",
        fontSize = LARGE_SIZE.sp,
        fontWeight = FontWeight.Bold
    )
    Spacer(
        modifier = Modifier
            .height(SMALL_SIZE.dp)
    )
    Text(
        fontSize = DEFAULT_SIZE.sp,
        text = "Let's enjoy!"
    )
}
 
@Composable
private fun email(): String {
    var email by remember {
        mutableStateOf("")
    }

    TextField(
        modifier = Modifier.fillMaxWidth(),
        value = email,
        onValueChange = { email = it },
        placeholder = { Text("Email") }
    )

    return email
}
 
@Composable
private fun password(): String {
    var password by remember {
        mutableStateOf("")
    }

    TextField(
        modifier = Modifier.fillMaxWidth(),
        value = password,
        onValueChange = { password = it },
        placeholder = { Text("Password") },
        visualTransformation =
            PasswordVisualTransformation()
    )

    return password
}
 
@Composable
private fun loginButton(
    modifier: Modifier = Modifier,
    password: String,
    email: String,
    viewModel: LoginViewModel = viewModel()
) {
    var clicked by remember {
        mutableStateOf(false)
    }

    val onBackPressedDispatcher = 
        LocalOnBackPressedDispatcherOwner.current
            ?.onBackPressedDispatcher

    if (clicked) {
        runBlocking {
            if (viewModel.login(email, password)) {
                onBackPressedDispatcher
                    ?.onBackPressed()
            }
        }
    }
 
    Button(
        modifier = modifier.fillMaxWidth(),
        onClick = { clicked = true }
    ) {
        Text(
            modifier = modifier,
            text = "Login"
        )
    }
}

const val LARGE_SIZE = 26
const val DEFAULT_SIZE = 16
const val SMALL_SIZE = 4
 
class LoginViewModel : ViewModel() {
    /**
     * @return true if login succeeded.
     */
    suspend fun login(
        email: String,
        password: String
    ): Boolean {
        /* ... */
        return true
    }
}

【出題3】Day1 15:00-18:00

この問題は「 Ishikawa Munetoshi 」が作りました！

所属：ディベロッパーエクスペリエンス開発チーム

※ 解説ブログは後日「ここ」にリンクが掲載されます。

/**
 * A repository class to save/load a [ShopItem] to the local storage.
 */
class ShopItemListRepository(
    private val shopItemDao: ShopItemDao,
    private val shopItemThumbnailDao: ShopItemThumbnailDao,
    private val errorReporter: ErrorReporter
) {
    /** [CoroutineScope] for DAO requests. This is to make jobs non-cancellable and running IO dispatcher. */
    private val scope: CoroutineScope = CoroutineScope(NonCancellable + Dispatchers.IO)

    /**
     * Stores the given [ShopItem] to the local storage with a thumbnail. Then, returns true iff the item and thumbnail
     * have been stored successfully.
     *
     * Even if the caller coroutine is cancelled, this contitues to storing data to prevent a race condition between
     * `ShopItem` and the thumbnail. This function is safe to call concurrently because of the non-cancellable coroutine
     * scope.
     */
    suspend fun store(shopItem: ShopItem): Boolean {
        val thumbnail = createThumbnail(shopItem)

        val differed = scope.async {
            // Step 1: Store thumbnail.
            // `suspendCoroutine` is to convert the callback interface of `shopItemThumbnailDao` to coroutine.
            var thumbnailCallback: ((Boolean) -> Unit)? = null
            val isThumbnailStoreSuccessful = suspendCoroutine { continuation ->
                val callback = { isSuccessful: Boolean -> continuation.resume(isSuccessful)  }
                thumbnailCallback = callback

                shopItemThumbnailDao.registerCallbackFor(shopItem, callback)
                shopItemThumbnailDao.save(shopItem, thumbnail)
            }

            val callback = thumbnailCallback
            if (callback != null) {
                shopItemThumbnailDao.unregisterCallback(callback)
            }
            
            if (!isThumbnailStoreSuccessful) {
                errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store thumbnail")
                return@async false
            }

            // Step 2: Store shop item.
            shopItemDao.upsert(shopItem)
            return@async true
        }

        return try {
            differed.await()
        } catch (exception: DataBaseQueryException) {
            // At the step 2, this exception may be thrown.
            errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store an item", exception)
            false
        }
    }

    private suspend fun createThumbnail(shopItem: ShopItem): Bitmap {
        /* snip */
    }
}

// Old Java code
public class ShopItemThumbnailDao {
    public void registerCallbackFor(@NonNull ShopItem item, @NonNull Consumer<Boolean> callback) { /* snip */ }
    public void save(@NonNull ShopItem item, @NonNull Bitmap thumbnail) { /* snip */ }
    public void unregisterCallback(@NonNull Consumer<Boolean> callback) { /* snip */ }
}

【出題4】Day2 10:00-14:00

この問題は「 Ando Yuki 」が作りました！

所属：iOS/Androidエクスペリエンス開発チーム

※ 解説ブログは後日「ここ」にリンクが掲載されます。

class SomeActivity : AppCompatActivity() {
    private val logger: Logger = Logger()
    private val userDataRepository = UserDataRepository()
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_huge_legacy)
        val userId = intent.getStringExtra(USER_ID) ?: "failed"
        UpdateUserData(userId)
    }
 
    fun UpdateUserData(userId: String) {
        val textView = findViewById<TextView>(R.id.text)
        lifecycleScope.launch(Dispatchers.Main) {
            if (userId != "falled") {
                userDataRepository.getUserData(UserId(userId))
                    .onSuccess { userData ->
                        textView.text = userData.userName.value
                    }.onFailure {
                        Toast.makeText(
                            this@SomeActivity,
                            "Something wrong with it.",
                            Toast.LENGTH_SHORT
                        ).show()
                        logger.sendLog()
                    }
            }
        }
    }
 
    companion object {
        const val USER_ID = "user_name"
    }
}
 
interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData
 
    suspend fun findUser(userName: UserName): UserData
}
 
class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result<UserData> =
        runCatching { userDataApi.findUser(userId) }
 
    suspend fun getUserData(userName: UserName): Result<UserData> =
        runCatching { userDataApi.findUser(userName) }
}
 
data class UserData(val id: String, val userName: UserName, val userType: UserType,val profileImage: Bitmap)
 
value class UserId(val value: String)
 
value class UserName(val value: String)
 
sealed class UserType {
    object ADMIN : UserType()
    object NORMAL : UserType()
}
 
class UserDataApiImpl : UserDataApi {
    override suspend fun findUser(userId: UserId): UserData = TODO("Call remote api")
       
    override suspend fun findUser(userName: UserName): UserData = TODO("Call remote api")
}
 
class Logger {
    fun sendLog() {
        TODO("Send log to remote server")
    }
}

【出題5】Day2 14:00-17:00

この問題は「 Tamaki Hidetsugu 」が作りました！

所属：LINEコミュニケーションサービス開発チーム

※ 解説ブログは後日「ここ」にリンクが掲載されます。

class MainActivity : AppCompatActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
        val request: Request = if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
            @Suppress("DEPRECATION")
            intent.getParcelableExtra("request")
        } else {
            intent.getParcelableExtra("request", Request::class.java)
        } ?: return
        val viewModel = MainViewModel()
        lifecycleScope.launch {
            viewModel.createDataFlow(request).collect(::updateView)
        }
    }
 
    private fun updateView(viewData: String) {
        // TODO: Update Views by [viewData].
    }
 
    companion object {
        fun createIntent(context: Context, request: Request): Intent =
            Intent(context, MainActivity::class.java)
                .putExtra("request", request)
    }
}
 
class MainViewModel : ViewModel() {
    suspend fun createDataFlow(request: Request): Flow<String> {
        val localCacheDeferred = loadDataAsync(request, true)
        val remoteCacheDeferred = loadDataAsync(request, false)
        return flow {
            emit(localCacheDeferred.await())
            emit(remoteCacheDeferred.await())
        }
    }
 
    private suspend fun loadDataAsync(request: Request, local: Boolean): Deferred<String> =
        coroutineScope { async { loadData(request, local) } }
 
    private suspend fun loadData(request: Request, local: Boolean): String {
        // TODO: This is dummy. Load data from local cache or remote.
        delay(if (local) 1000L else 5000L)
        return "result of '$request'"
    }
}
 
data class Request(val word: String, val page: Int) : Parcelable {
    override fun writeToParcel(parcel: Parcel, flags: Int) {
        parcel.writeString(word)
        parcel.writeInt(page)
    }
 
    override fun describeContents(): Int = 0
 
    companion object {
        @JvmField
        val CREATOR = RequestCreator
    }
}
 
object RequestCreator : Parcelable.Creator<Request> {
    override fun createFromParcel(parcel: Parcel): Request =
        Request(checkNotNull(parcel.readString()), parcel.readInt())
 
    override fun newArray(size: Int): Array<Request?> = arrayOfNulls(size)
}

After DroidKaigi 2023 で解説します！

9月25日(月)19時よりDroidKaigi 2023に協賛している株式会社ZOZO、ヤフー株式会社、LINE株式会社の3社合同で「After DroidKaigi 2023」を開催いたします。

当日は、Code Review Challengeの作問を担当した安藤による、各社の社員による上記取り組みの解説とCode Review Challengeの裏側(問題準備など)とそれを支えたLINEのレビュー文化について紹介する予定です。

どんなふうに準備をして当日を迎えたのか。参考になる情報を盛り沢山でお届けする予定です。

エンジニアの方であればどなたでも参加可能です。ぜひお気軽にご参加ください。

Code Review Challengeの裏側とLINEのレビュー文化

安藤祐貴 (Yuki Ando) / Twitter / Software(Android) Engineer / LINE株式会社 iOS/Androidエクスペリエンス開発チーム

自己紹介文: 2021年にLINEに新卒入社し、LINEアプリのAndroidクライアントについて、主にAndroid OSの新機能を適用する業務に携わっています。車を走らせることが大好きです。

イベント情報

名称 : After DroidKaigi 2023
開催日時: 9月25日(月)19時
場所：オンライン + オフライン（LODGE）
申込URL: https://zozotech-inc.connpass.com/event/295096/

Android