$30 off During Our Annual Pro Sale. View Details »

Code Review Challenge: An example of a solution

Code Review Challenge: An example of a solution

DroidKaigi 2023: コードレビューチャレンジの問題解説

◾️ 発表者
安藤 祐貴
LINE株式会社 / iOS/Androidエクスペリエンス開発チーム Software(Android) Engineer
2021年にLINEに新卒入社し、LINEアプリのAndroidクライアントについて、主にAndroid OSの新機能を適用する業務に携わっています。車を走らせることが大好きです。
https://x.com/m1zyuk1

こちらは以下のイベントの発表資料です。
https://zozotech-inc.connpass.com/event/295096/

LINE Developers
PRO

September 25, 2023
Tweet

More Decks by LINE Developers

Other Decks in Technology

Transcript

  1. Yuki Ando, LINE Corporation
    Code Review Challenge
    An example of a solution
    DroidKaigi 2023: ίʔυϨϏϡʔνϟϨϯδͷ໰୊ղઆ

    View Slide

  2. Self-introduction
    Name: Yuki Ando / ҆౻ ༞و

    Twitter: @m1zyuk1

    Hobby: Car, Game

    Main activity: 

    Apply new Android feature to LINE Android

    View Slide

  3. Did you participate in


    DroidKaigi 2023 offline?

    View Slide

  4. Yes I am
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  5. Have you participated in the
    Code Review Challenge?

    View Slide

  6. View Slide

  7. View Slide

  8. What’s Code Review Challenge?

    View Slide

  9. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  10. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    Challengers
    Review
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  11. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    Challengers
    Review
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  12. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    Challengers
    Review
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  13. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    Review
    Review ideas
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  14. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    class Good {
    fun smartFunc() { /* */ }
    }
    Review
    Review ideas
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  15. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    class Good {
    fun smartFunc() { /* */ }
    }
    Review
    Review ideas
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  16. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    class Good {
    fun smartFunc() { /* */ }
    }
    Review
    Review ideas
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  17. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    class Good {
    fun smartFunc() { /* */ }
    }
    Review
    Review ideas
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  18. Code Review Challenge
    Review “Bad Code” to be a “Good Code”
    LINE Booth in DroidKaigi 2023
    class Ugly {
    fun dirtyFunc() { /* */ }
    }
    class Good {
    fun smartFunc() { /* */ }
    }
    Review
    Review ideas
    Challengers
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  19. Explain the assumed solutions

    View Slide

  20. Full codes are hereɹɹɹ
    Ref: https://engineering.linecorp.com/ja/blog/droidKaigi-2023
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/

    View Slide

  21. Questions
    1. Tip of the Ice-func

    2. A Composable for everything, and everything in its Composable

    3. Slip Through My Async

    4. Caught on Running

    5. Bitter Sweets

    View Slide

  22. Tip of the Ice-func
    An object which have a function to do the back up process
    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 showProgressDialog() {
    progressDialog = ProgressDialogForBackup()
    progressDialog.showOnMainThread()
    }
    private fun hideProgressDialog() {
    progressDialog.hideOnMainThread()
    }
    //...snip
    }

    View Slide

  23. Tip of the Ice-func
    An object which have a function to do the back up process
    object BackupCreator {
    private val localDataSource = LocalDataSource()
    private val remoteDataSource = RemoteDataSource()
    private lateinit var progressDialog: ProgressDialogForBackup
    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)
    }
    }

    View Slide

  24. Tip of the Ice-func
    An object which have a function to do the back up process
    private fun writeToFile(file: File, data: BackupData) {
    val outputStream = file.outputStream()
    try {
    outputStream.write(data.toByteArray())
    } catch (e: IOException) {
    // Ignore
    }
    outputStream.close()
    }

    View Slide

  25. Major issues
    1. OutputStream is not handled properly

    2. Signatures that can be used incorrectly

    3. Structure of functions with poor readability

    View Slide

  26. Tip of the Ice-func
    Issue: IOException should not be ignored
    private fun writeToFile(file: File, data: BackupData) {
    val outputStream = file.outputStream()
    try {
    outputStream.write(data.toByteArray())
    } catch (e: IOException) {
    // Ignore
    }
    outputStream.close()
    }

    View Slide

  27. Tip of the Ice-func
    Issue: IOException should not be ignored
    private fun writeToFile(file: File, data: BackupData) {
    val outputStream = file.outputStream()
    try {
    outputStream.write(data.toByteArray())
    } catch (e: IOException) {
    // Ignore
    }
    outputStream.close()
    }

    View Slide

  28. Tip of the Ice-func
    Issue: Exception may be thrown
    private fun writeToFile(file: File, data: BackupData) {
    val outputStream = file.outputStream()
    try {
    outputStream.write(data.toByteArray())
    } catch (e: IOException) {
    // Ignore
    }
    outputStream.close()
    }

    View Slide

  29. Tip of the Ice-func
    Solution: Use closable classes with `use` phrase
    // Throws [IOException] if any I/O error occurred.
    private fun writeToFile(file: File, data: BackupData) {
    file.outputStream().use { outputStream ->
    outputStream.write(data.toByteArray())
    }
    }

    View Slide

  30. Tip of the Ice-func
    Solution: Use closable classes with `use` phrase
    // Throws [IOException] if any I/O error occurred.
    private fun writeToFile(file: File, data: BackupData) {
    file.outputStream().use { outputStream ->
    outputStream.write(data.toByteArray())
    }
    }

    View Slide

  31. Tip of the Ice-func
    Solution: Handle IOException such as canceling process
    // Throws [IOException] if any I/O error occurred.
    private fun writeToFile(file: File, data: BackupData) {
    file.outputStream().use { outputStream ->
    outputStream.write(data.toByteArray())
    }
    }
    private fun collectBackupData(target: BackupTarget, since: Date) {
    val data = when (target) {
    BackupTarget.IMAGE -> localDataSource.selectImageData()
    BackupTarget.VIDEO -> localDataSource.selectVideoData()
    BackupTarget.MESSAGE -> localDataSource.selectMessageData(since)
    }
    try {
    //...snip
    writeToFile(File("path/raw"), data)
    } catch (e: IOException){
    // handle IO Exception
    }
    }

    View Slide

  32. Tip of the Ice-func
    Solution: Handle IOException such as canceling process
    // Throws [IOException] if any I/O error occurred.
    private fun writeToFile(file: File, data: BackupData) {
    file.outputStream().use { outputStream ->
    outputStream.write(data.toByteArray())
    }
    }
    private fun collectBackupData(target: BackupTarget, since: Date) {
    val data = when (target) {
    BackupTarget.IMAGE -> localDataSource.selectImageData()
    BackupTarget.VIDEO -> localDataSource.selectVideoData()
    BackupTarget.MESSAGE -> localDataSource.selectMessageData(since)
    }
    try {
    //...snip
    writeToFile(File("path/raw"), data)
    } catch (e: IOException){
    // handle IO Exception
    }
    }

    View Slide

  33. Major issues
    1. Might not be called OutputStream.close()

    2. Signatures that can be used incorrectly

    3. Structure of functions with poor readability

    View Slide

  34. Tip of the Ice-func
    Issue: Signatures that can be used incorrectly

    object BackupCreator {
    private val localDataSource = LocalDataSource()
    private val remoteDataSource = RemoteDataSource()
    private lateinit var progressDialog: ProgressDialogForBackup
    @WorkerThread
    fun createBackup(target: BackupTarget, since: Date) {
    //...snip
    }

    View Slide

  35. Tip of the Ice-func
    Issue: Signatures that can be used incorrectly

    object BackupCreator {
    private val localDataSource = LocalDataSource()
    private val remoteDataSource = RemoteDataSource()
    private lateinit var progressDialog: ProgressDialogForBackup
    @WorkerThread
    fun createBackup(target: BackupTarget, since: Date) {
    //...snip
    }
    enum class BackupTarget { IMAGE, VIDEO, MESSAGE }

    View Slide

  36. Tip of the Ice-func
    Issue: Signatures that can be used incorrectly

    object BackupCreator {
    private val localDataSource = LocalDataSource()
    private val remoteDataSource = RemoteDataSource()
    private lateinit var progressDialog: ProgressDialogForBackup
    @WorkerThread
    fun createBackup(target: BackupTarget, since: Date) {
    //...snip
    }
    `since` is not affected for image/video.
    But interface doesn’t indicate this.
    enum class BackupTarget { IMAGE, VIDEO, MESSAGE }

    View Slide

  37. Tip of the Ice-func
    Issue: Signatures that can be used incorrectly

    private fun collectBackupData(target: BackupTarget, since: Date) {
    val data = when (target) {
    BackupTarget.IMAGE -> localDataSource.selectImageData()
    BackupTarget.VIDEO -> localDataSource.selectVideoData()
    BackupTarget.MESSAGE -> localDataSource.selectMessageData(since)
    }
    try {
    //...snip
    writeToFile(File("path/raw"), data)
    } catch (e: IOException){
    // handle IO Exception
    }
    }
    `since` is not affected for image/video.
    But interface doesn’t indicate this.
    enum class BackupTarget { IMAGE, VIDEO, MESSAGE }

    View Slide

  38. Tip of the Ice-func
    Issue: Signatures that can be used incorrectly

    private fun collectBackupData(target: BackupTarget, since: Date) {
    val data = when (target) {
    BackupTarget.IMAGE -> localDataSource.selectImageData()
    BackupTarget.VIDEO -> localDataSource.selectVideoData()
    BackupTarget.MESSAGE -> localDataSource.selectMessageData(since)
    }
    try {
    //...snip
    writeToFile(File("path/raw"), data)
    } catch (e: IOException){
    // handle IO Exception
    }
    }
    `since` is not affected for image/video.
    But interface doesn’t indicate this.
    enum class BackupTarget { IMAGE, VIDEO, MESSAGE }

    View Slide

  39. Tip of the Ice-func
    Solution: Use sealed class to represent behavior correctly by interface
    private fun collectBackupData(target: BackupTarget) {
    val data = when (target) {
    BackupTarget.IMAGE -> localDataSource.selectImageData()
    BackupTarget.VIDEO -> localDataSource.selectVideoData()
    BackupTarget.MESSAGE -> localDataSource.selectMessageData(target.since)
    }
    try {
    //...snip
    writeToFile(File("path/raw"), data)
    } catch (e: IOException){
    // handle IO Exception
    }
    }
    sealed class BackupTarget {
    data object Image : BackupTarget()
    data object Video : BackupTarget()
    data class Message(val since: Date) : BackupTarget()
    }

    View Slide

  40. Tip of the Ice-func
    Solution: Use sealed class to represent behavior correctly by interface
    private fun collectBackupData(target: BackupTarget) {
    val data = when (target) {
    BackupTarget.IMAGE -> localDataSource.selectImageData()
    BackupTarget.VIDEO -> localDataSource.selectVideoData()
    BackupTarget.MESSAGE -> localDataSource.selectMessageData(target.since)
    }
    try {
    //...snip
    writeToFile(File("path/raw"), data)
    } catch (e: IOException){
    // handle IO Exception
    }
    }
    sealed class BackupTarget {
    data object Image : BackupTarget()
    data object Video : BackupTarget()
    data class Message(val since: Date) : BackupTarget()
    }

    View Slide

  41. Major issues
    1. Might not be called OutputStream.close()

    2. Signatures that can be used incorrectly

    3. Structure of functions with poor readability

    View Slide

  42. Tip of the Ice-func
    Issue: Structure of functions with poor readability
    fun createBackup() {
    uploadBackup()
    }
    fun uploadBackup() {
    compressBackupData()
    }
    fun compressBackupData() {
    collectBackupData()
    }
    fun collectBackupData() {
    // collect backup data
    }

    View Slide

  43. Tip of the Ice-func
    Issue: Structure of functions with poor readability
    fun createBackup() {
    uploadBackup()
    }
    fun uploadBackup() {
    compressBackupData()
    }
    fun compressBackupData() {
    collectBackupData()
    }
    fun collectBackupData() {
    // collect backup data
    }

    View Slide

  44. Tip of the Ice-func
    Solution: Avoid unnecessary nesting and make it easier to understand
    fun createBackup(target: BackupTarget) {
    val tmpFiles = mutableListOf()
    try {
    val rawDataFile = collectBackupData(target).also(tmpFiles::add)
    val compressedDataFile = compress(rawDataFile).also(tmpFiles::add)
    remoteDataSource.upload(compressedDataFile)
    } catch (e: IOException) {
    // TODO: Handle error.
    } finally {
    tmpFiles.forEach(File::delete)
    }
    }

    View Slide

  45. Tip of the Ice-func
    Solution: Avoid unnecessary nesting and make it easier to understand
    fun createBackup(target: BackupTarget) {
    val tmpFiles = mutableListOf()
    try {
    val rawDataFile = collectBackupData(target).also(tmpFiles::add)
    val compressedDataFile = compress(rawDataFile).also(tmpFiles::add)
    remoteDataSource.upload(compressedDataFile)
    } catch (e: IOException) {
    // TODO: Handle error.
    } finally {
    tmpFiles.forEach(File::delete)
    }
    }

    View Slide

  46. Tip of the Ice-func
    Other issues

    • Implicit dependencies in
    fi
    le paths

    • createBackup function is not thread safe

    • BackupCreator object should be class with injecting properties

    • ProgressDialog

    • Should be nullable var instead of lateinit var

    • Might be shown incorrectly in case of calling `createBackup` twice

    • Should not display by BackupCreator

    • Temporary
    fi
    les should be deleted

    View Slide

  47. Questions
    1. Tip of the Ice-func

    2. A Composable for everything, and everything in its Composable

    3. Slip Through My Async

    4. Caught on Running

    5. Bitter Sweets/

    View Slide

  48. A Composable for everything, and everything in its Composable
    A login screen which constructed by Composable function
    @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
    )
    }
    }

    View Slide

  49. A Composable for everything, and everything in its Composable
    A login screen which constructed by Composable function
    @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
    }

    View Slide

  50. A Composable for everything, and everything in its Composable
    A login screen which constructed by Composable function
    @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(/* ...snip*/) {
    //...snip
    }
    }

    View Slide

  51. Major issues
    1. Composable functions that draw UI must not return values

    2. Implementing the login process in an unnecessarily complex way

    3. Do not use `runBlocking` for asynchronous processing

    View Slide

  52. A Composable for everything, and everything in its Composable
    Issue: Composable functions that draw UI must not return values
    @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
    )
    }
    }

    View Slide

  53. A Composable for everything, and everything in its Composable
    Issue: Composable functions that draw UI must not return values
    @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
    )
    }
    }

    View Slide

  54. A Composable for everything, and everything in its Composable
    Issue: Composable functions that draw UI must not return values
    @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 email(): String {
    var email by remember {
    mutableStateOf("")
    }
    TextField(
    modifier = Modifier.fillMaxWidth(),
    value = email,
    onValueChange = { email = it },
    placeholder = { Text("Email") }
    )
    return email
    }

    View Slide

  55. A Composable for everything, and everything in its Composable
    Issue: Composable functions that draw UI must not return values
    @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 email(): String {
    var email by remember {
    mutableStateOf("")
    }
    TextField(
    modifier = Modifier.fillMaxWidth(),
    value = email,
    onValueChange = { email = it },
    placeholder = { Text("Email") }
    )
    return email
    }

    View Slide

  56. A Composable for everything, and everything in its Composable
    Solution: Handle value on the caller side and pass value as an argument
    @Composable
    private fun Password(
    password: String,
    onPasswordChange: (String) -> Unit,
    modifier: Modifier = Modifier
    ) {
    TextField(
    modifier = modifier.fillMaxWidth(),
    value = password,
    onValueChange = onPasswordChange,
    placeholder = { Text("Password") },
    visualTransformation = PasswordVisualTransformation()
    )
    }
    @Composable
    private fun Email(
    email: String,
    onEmailChange: (String) -> Unit,
    modifier: Modifier = Modifier
    ){
    TextField(
    modifier = modifier.fillMaxWidth(),
    value = email,
    onValueChange = onEmailChange,
    placeholder = { Text("Email") },
    singleLine = true
    )
    }

    View Slide

  57. A Composable for everything, and everything in its Composable
    Solution: Handle value on the caller side and pass value as an argument
    @Composable
    private fun Password(
    password: String,
    onPasswordChange: (String) -> Unit,
    modifier: Modifier = Modifier
    ) {
    TextField(
    modifier = modifier.fillMaxWidth(),
    value = password,
    onValueChange = onPasswordChange,
    placeholder = { Text("Password") },
    visualTransformation = PasswordVisualTransformation()
    )
    }
    @Composable
    private fun Email(
    email: String,
    onEmailChange: (String) -> Unit,
    modifier: Modifier = Modifier
    ){
    TextField(
    modifier = modifier.fillMaxWidth(),
    value = email,
    onValueChange = onEmailChange,
    placeholder = { Text("Email") },
    singleLine = true
    )
    }

    View Slide

  58. Major issues
    1. Composable functions that draw UI must not return values

    2. Implementing the login process in an unnecessarily complex way

    3. Do not use `runBlocking` for asynchronous processing

    View Slide

  59. A Composable for everything, and everything in its Composable
    Issue: Implementing the login process in an unnecessarily complex way
    @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"
    )
    }
    }

    View Slide

  60. A Composable for everything, and everything in its Composable
    Issue: Implementing the login process in an unnecessarily complex way
    @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"
    )
    }
    }

    View Slide

  61. A Composable for everything, and everything in its Composable
    Issue: Implementing the login process in an unnecessarily complex way
    @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"
    )
    }
    }

    View Slide

  62. A Composable for everything, and everything in its Composable
    Solution: Apply `state hoisting` strategy
    fun LoginScreen() {
    //...snip
    var email by remember { mutableStateOf("") }
    var password by remember { mutableStateOf("") }
    LoginButton(
    onClick = { viewModel.login(email, password) }
    )
    }
    @Composable
    private fun LoginButton(
    onClick: () -> Unit,
    modifier: Modifier = Modifier
    ) {
    Button(
    modifier = modifier.fillMaxWidth(),
    onClick = onClick
    ) {
    Text(text = "Login")
    }
    }

    View Slide

  63. A Composable for everything, and everything in its Composable
    Solution: Apply `state hoisting` strategy
    fun LoginScreen() {
    //...snip
    var email by remember { mutableStateOf("") }
    var password by remember { mutableStateOf("") }
    LoginButton(
    onClick = { viewModel.login(email, password) }
    )
    }
    @Composable
    private fun LoginButton(
    onClick: () -> Unit,
    modifier: Modifier = Modifier
    ) {
    Button(
    modifier = modifier.fillMaxWidth(),
    onClick = onClick
    ) {
    Text(text = "Login")
    }
    }

    View Slide

  64. Major issues
    1. Composable functions that draw UI must not return values

    2. Implementing the login process in an unnecessarily complex way

    3. Do not use `runBlocking` for asynchronous processing

    View Slide

  65. A Composable for everything, and everything in its Composable
    Issue: Do not use runBlocking for asynchronous processing
    @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"
    )
    }
    }

    View Slide

  66. A Composable for everything, and everything in its Composable
    Issue: Do not use runBlocking for asynchronous processing
    @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"
    )
    }
    }

    View Slide

  67. A Composable for everything, and everything in its Composable
    Solution: Use appropriate CoroutineScope
    fun LoginScreen() {
    //...snip
    var email by remember { mutableStateOf("") }
    var password by remember { mutableStateOf("") }
    LoginButton(
    onClick = { viewModel.login(email, password) }
    )
    }
    @Composable
    private fun LoginButton(
    onClick: () -> Unit,
    modifier: Modifier = Modifier
    ) {
    Button(
    modifier = modifier.fillMaxWidth(),
    onClick = onClick
    ) {
    Text(text = "Login")
    }
    }

    View Slide

  68. A Composable for everything, and everything in its Composable
    Solution: Use appropriate CoroutineScope
    fun LoginScreen() {
    //...snip
    var email by remember { mutableStateOf("") }
    var password by remember { mutableStateOf("") }
    LoginButton(
    onClick = { viewModel.login(email, password) }
    )
    }
    @Composable
    private fun LoginButton(
    onClick: () -> Unit,
    modifier: Modifier = Modifier
    ) {
    Button(
    modifier = modifier.fillMaxWidth(),
    onClick = onClick
    ) {
    Text(text = "Login")
    }
    }

    View Slide

  69. A Composable for everything, and everything in its Composable
    Solution: Use appropriate CoroutineScope
    fun LoginScreen() {
    //...snip
    var email by remember { mutableStateOf("") }
    var password by remember { mutableStateOf("") }
    LoginButton(
    onClick = { viewModel.login(email, password) }
    )
    }
    @Composable
    private fun LoginButton(
    onClick: () -> Unit,
    modifier: Modifier = Modifier
    ) {
    Button(
    modifier = modifier.fillMaxWidth(),
    onClick = onClick
    ) {
    Text(text = "Login")
    }
    }
    val scope = rememberCoroutineScope()
    scope.launch {
    // Do something
    }

    View Slide

  70. A Composable for everything, and everything in its Composable
    Other issues

    • Should follow the convention for Composable

    • Naming

    • Argument Order

    • Only one UI Composable should be placed at the root

    • Modi
    fi
    er argument is speci
    fi
    ed only for the outermost UI component

    • Meaningless constants should be stopped

    • Use rememberSavable or SavedStateHandle instead of remember

    • Use keyboardOptions and keyboardActions

    View Slide

  71. Questions
    1. Tip of the Ice-func

    2. A Composable for everything, and everything in its Composable

    3. Slip Through My Async

    4. Caught on Running

    5. Bitter Sweets/

    View Slide

  72. Slip Through My Async
    A repository class which handles shop item
    /**
    * 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 { /* snip */ }
    private suspend fun createThumbnail(shopItem: ShopItem): Bitmap { /* snip */ }
    }

    View Slide

  73. Slip Through My Async
    A repository class which handles shop item
    // Old Java code
    public class ShopItemThumbnailDao {
    public void registerCallbackFor(@NonNull ShopItem item, @NonNull Consumer callback) { /* snip */ }
    public void save(@NonNull ShopItem item, @NonNull Bitmap thumbnail) { /* snip */ }
    public void unregisterCallback(@NonNull Consumer callback) { /* snip */ }
    }

    View Slide

  74. Slip Through My Async
    A repository class which handles shop item
    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
    }
    //…snip
    }
    }

    View Slide

  75. Slip Through My Async
    A repository class which handles shop item
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val differed = scope.async {
    // …snip
    // 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
    }
    }

    View Slide

  76. Major issues
    1. Cause the app to crash

    2. Hard to understand what this function does

    3. Race condition occurs

    View Slide

  77. Slip Through My Async
    Issue: Causes the app to crash
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val differed = scope.async {
    // …snip
    // 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
    }
    }

    View Slide

  78. Slip Through My Async
    Issue: Causes the app to crash
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val differed = scope.async {
    // …snip
    // 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
    }
    }

    View Slide

  79. Slip Through My Async
    Solution: Make try-catch scope inside `async`
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val deferred = scope.async {
    // …snip
    // Step 2: Store shop item.
    try {
    shopItemDao.upsert(shopItem
    true
    } catch (exception: DataBaseQueryException) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store an item", exception)
    false
    }
    }
    return deferred.await()
    }

    View Slide

  80. Slip Through My Async
    Solution: Make try-catch scope inside `async`
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val deferred = scope.async {
    // …snip
    // Step 2: Store shop item.
    try {
    shopItemDao.upsert(shopItem
    true
    } catch (exception: DataBaseQueryException) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store an item", exception)
    false
    }
    }
    return deferred.await()
    }

    View Slide

  81. Major issues
    1. Cause the app to crash

    2. Hard to understand what this function does

    3. Race condition occurs

    View Slide

  82. Slip Through My Async
    Issue: Hard to understand what this function does
    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
    }
    //…snip
    }
    }

    View Slide

  83. Slip Through My Async
    Issue: Hard to understand what this function does
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val differed = scope.async {
    // …snip
    // 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
    }
    }

    View Slide

  84. Slip Through My Async
    Issue: Hard to understand what this function does
    suspend fun store(shopItem: ShopItem): Boolean {
    val thumbnail = createThumbnail(shopItem)
    val differed = scope.async {
    // …snip
    // 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
    }
    }

    View Slide

  85. Slip Through My Async
    Solution: Split functions to ensure readability
    suspend fun store(shopItem: ShopItem): Boolean {
    val differed = scope.asyncWithLock(mutex) {
    val isThumbnailStoreSuccessful = storeThumbnail(shopItem)
    if (!isThumbnailStoreSuccessful) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store thumbnail")
    return@asyncWithLock false
    }
    val isShopItemStoreSuccessful = storeShopItem(shopItem)
    if (!isShopItemStoreSuccessful) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store item")
    deleteThumbnail(shopItem)
    return@asyncWithLock false
    }
    true
    }
    return differed.await()
    }

    View Slide

  86. Slip Through My Async
    Solution: Split functions to ensure readability
    suspend fun store(shopItem: ShopItem): Boolean {
    val differed = scope.asyncWithLock(mutex) {
    val isThumbnailStoreSuccessful = storeThumbnail(shopItem)
    if (!isThumbnailStoreSuccessful) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store thumbnail")
    return@asyncWithLock false
    }
    val isShopItemStoreSuccessful = storeShopItem(shopItem)
    if (!isShopItemStoreSuccessful) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store item")
    deleteThumbnail(shopItem)
    return@asyncWithLock false
    }
    true
    }
    return differed.await()
    }

    View Slide

  87. Slip Through My Async
    Solution: Split functions to ensure readability
    suspend fun store(shopItem: ShopItem): Boolean {
    val differed = scope.asyncWithLock(mutex) {
    val isThumbnailStoreSuccessful = storeThumbnail(shopItem)
    if (!isThumbnailStoreSuccessful) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store thumbnail")
    return@asyncWithLock false
    }
    val isShopItemStoreSuccessful = storeShopItem(shopItem)
    if (!isShopItemStoreSuccessful) {
    errorReporter.report(ErrorTag.SHOP_ITEM, "Failed to store item")
    deleteThumbnail(shopItem)
    return@asyncWithLock false
    }
    true
    }
    return differed.await()
    }

    View Slide

  88. A Composable for everything, and everything in its Composable
    Other issues

    • NonCancellable is not needed

    • Too wide scope of Dispatcher.IO

    • Too wide scope of thumbnailCallback

    • Unregistering callback does not work properly

    • Thumbnail
    fi
    le remains if upsert fails

    • Unnecessary return@async

    View Slide

  89. Questions
    1. Tip of the Ice-func

    2. A Composable for everything, and everything in its Composable

    3. Slip Through My Async

    4. Caught on Running

    5. Bitter Sweets/

    View Slide

  90. Caught on Running
    A repository class which gets user data from remote server
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData
    suspend fun findUser(userName: UserName): UserData
    }
    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 UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }

    View Slide

  91. Caught on Running
    A repository class which gets user data from remote server
    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()
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData
    suspend fun findUser(userName: UserName): UserData
    }
    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 UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }

    View Slide

  92. Caught on Running
    A repository is used from SomeActivity
    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(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" }
    }

    View Slide

  93. Major issues
    1. Do not use `runCatching`

    2. Do not overload functions

    3. Data class has value that doesn’t have implementation of equals

    View Slide

  94. Caught on Running
    Issue: Do not use `runCatching`
    class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }

    View Slide

  95. Caught on Running
    Issue: Do not use `runCatching`
    class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }

    View Slide

  96. Caught on Running
    Issue: Do not use `runCatching`

    .equals( )
    class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }
    suspend fun getUserData(userId: UserId): Result = try {
    Result.success(userDataApi.findUser(userId))
    } catch (e: Throwable) {
    Result.failure(e)
    }

    View Slide

  97. class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }
    Caught on Running
    Issue: Do not use `runCatching`

    .equals( )
    suspend fun getUserData(userId: UserId): Result = try {
    Result.success(userDataApi.findUser(userId))
    } catch (e: Throwable) {
    Result.failure(e)
    }
    Throwable
    Exception
    Error
    RuntimeException
    IOException
    NullPointerException CancellationException

    View Slide

  98. class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }
    Caught on Running
    Issue: Do not use `runCatching`

    .equals( )
    suspend fun getUserData(userId: UserId): Result = try {
    Result.success(userDataApi.findUser(userId))
    } catch (e: Throwable) {
    Result.failure(e)
    }
    This is System Error.
    Don’t catch!
    Throwable
    Exception
    Error
    RuntimeException
    IOException
    NullPointerException CancellationException

    View Slide

  99. class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }
    Caught on Running
    Issue: Do not use `runCatching`

    .equals( )
    suspend fun getUserData(userId: UserId): Result = try {
    Result.success(userDataApi.findUser(userId))
    } catch (e: Throwable) {
    Result.failure(e)
    }
    Don’t catch!
    This is used for
    coroutine cancellation.
    Don’t catch!
    Throwable
    Exception
    Error
    RuntimeException
    IOException
    NullPointerException CancellationException

    View Slide

  100. class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }
    Caught on Running
    Issue: Do not use `runCatching`

    .equals( )
    suspend fun getUserData(userId: UserId): Result = try {
    Result.success(userDataApi.findUser(userId))
    } catch (e: Throwable) {
    Result.failure(e)
    }
    Don’t catch!
    Usually, this is caused by bugs.
    Don’t catch!
    Don’t catch!
    Throwable
    Exception
    Error
    RuntimeException
    IOException
    NullPointerException CancellationException

    View Slide

  101. class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }
    Caught on Running
    Issue: Do not use `runCatching`

    .equals( )
    suspend fun getUserData(userId: UserId): Result = try {
    Result.success(userDataApi.findUser(userId))
    } catch (e: Throwable) {
    Result.failure(e)
    }
    Don’t catch!
    Don’t catch!
    Don’t catch!
    You should catch this!
    Throwable
    Exception
    Error
    RuntimeException
    IOException
    NullPointerException CancellationException

    View Slide

  102. Caught on Running
    Issue: Do not use `runCatching`

    class UserDataRepository(private val userDataApi: UserDataApi = UserDataApiImpl()) {
    suspend fun getUserData(userId: UserId): Result =
    runCatching { userDataApi.findUser(userId) }
    suspend fun getUserData(userName: UserName): Result =
    runCatching { userDataApi.findUser(userName) }
    }

    View Slide

  103. Caught on Running
    Solution: Use `withContext` and minimal `try-catch` scope
    class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }

    View Slide

  104. Caught on Running
    Solution: Use `withContext` and minimal `try-catch` scope
    class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }

    View Slide

  105. Major issues
    1. Do not use `runCatching`

    2. Do not overload functions

    3. Data class has value that doesn’t have implementation of equals

    View Slide

  106. Caught on Running
    Issue: Do not overload functions
    class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }

    View Slide

  107. Caught on Running
    Issue: Do not overload functions
    class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }

    View Slide

  108. Caught on Running
    Issue: Do not overload functions
    class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData?
    suspend fun findUser(userName: UserName): UserData?
    }

    View Slide

  109. class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    Caught on Running
    Issue: Do not overload functions
    public interface UserDataApi {
    public @Nullable UserData findUser(String userId);
    public @Nullable UserData findUser(String userName);
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData?
    suspend fun findUser(userName: UserName): UserData?
    }

    View Slide

  110. class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    Caught on Running
    Issue: Do not overload functions
    public interface UserDataApi {
    public @Nullable UserData findUser(String userId);
    public @Nullable UserData findUser(String userName);
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData?
    suspend fun findUser(userName: UserName): UserData?
    }

    View Slide

  111. class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    Caught on Running
    Issue: Do not overload functions
    public interface UserDataApi {
    public @Nullable UserData findUser(String userId);
    public @Nullable UserData findUser(String userName);
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData?
    suspend fun findUser(userName: UserName): UserData?
    }
    String userId = "userId";
    userDataRepository.getUserData(userId, new Continuation>() {
    @NonNull
    @Override
    public CoroutineContext getContext() {
    // ...snip
    }
    @Override
    public void resumeWith(@NonNull Object o) {
    // ...snip
    }
    });

    View Slide

  112. class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    Caught on Running
    Issue: Do not overload functions
    public interface UserDataApi {
    public @Nullable UserData findUser(String userId);
    public @Nullable UserData findUser(String userName);
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData?
    suspend fun findUser(userName: UserName): UserData?
    }
    String userId = "userId";
    userDataRepository.getUserData(userId, new Continuation>() {
    @NonNull
    @Override
    public CoroutineContext getContext() {
    // ...snip
    }
    @Override
    public void resumeWith(@NonNull Object o) {
    // ...snip
    }
    }); Ambiguous method call. Both

    getUserData (String,Continuation super Result extends UserData>>) in UserDataRepository and

    getUserData(String,Continuation super Result extends UserData>>)in UserDataRepository match

    View Slide

  113. class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserData(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserData(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    Caught on Running
    Issue: Do not overload functions
    public interface UserDataApi {
    public @Nullable UserData findUser(String userId);
    public @Nullable UserData findUser(String userName);
    }
    interface UserDataApi {
    suspend fun findUser(userId: UserId): UserData?
    suspend fun findUser(userName: UserName): UserData?
    }

    View Slide

  114. class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserDataById(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserDataByName(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    Caught on Running
    Solution: Make appropriate name for functions without overloading
    public interface UserDataApi {
    public @Nullable UserData findUserById(String userId);
    public @Nullable UserData findUserByName(String userName);
    }
    interface UserDataApi {
    suspend fun findUserById(userId: UserId): UserData?
    suspend fun findUserByName(userName: UserName): UserData?
    }

    View Slide

  115. Caught on Running
    Solution: Make appropriate name for functions without overloading
    public interface UserDataApi {
    public @Nullable UserData findUserById(String userId);
    public @Nullable UserData findUserByName(String userName);
    }
    class UserDataRepository(
    private val userDataApi: UserDataApi = UserDataApiImpl(),
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
    ) {
    suspend fun getUserDataById(userId: UserId): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userId)
    }
    } catch (e: IOException) {
    null
    }
    suspend fun getUserDataByName(userName: UserName): UserData? = try {
    withContext(ioDispatcher) {
    userDataApi.findUser(userName)
    }
    } catch (e: IOException) {
    null
    }
    }
    interface UserDataApi {
    suspend fun findUserById(userId: UserId): UserData?
    suspend fun findUserByName(userName: UserName): UserData?
    }

    View Slide

  116. Major issues
    1. Do not use `runCatching`

    2. Do not overload functions

    3. Data class has value that doesn’t have implementation of equals

    View Slide

  117. Caught on Running
    Issue: Data class has value that doesn’t have implementation of equals
    data class UserData(
    val id: UserId,
    val userName: UserName,
    val userType: UserType,
    val profileImage: Bitmap
    )

    View Slide

  118. Issue: Data class has value that doesn’t have implementation of equals
    Caught on Running
    data class UserData(
    val id: UserId,
    val userName: UserName,
    val userType: UserType,
    val profileImage: Bitmap
    ){
    fun equals(other: Any?): Boolean { }
    fun hashCode(): Int { }
    fun toString(): String { }
    }

    View Slide

  119. Issue: Data class has value that doesn’t have implementation of equals
    Caught on Running
    data class UserData(
    val id: UserId,
    val userName: UserName,
    val userType: UserType,
    val profileImage: Bitmap
    ){
    fun equals(other: Any?): Boolean { }
    fun hashCode(): Int { }
    fun toString(): String { }
    }

    View Slide

  120. Caught on Running
    Solution: Use ordinal class
    class UserData(
    val id: UserId,
    val userName: UserName,
    val userType: UserType,
    val profileImage: Bitmap
    )

    View Slide

  121. Caught on Running
    Solution: Use class for which equals is implemented.
    data class UserData(
    val id: UserId,
    val userName: UserName,
    val userType: UserType,
    val profileImageUrl: String
    )

    View Slide

  122. Caught on Running
    Other issues

    • Bug on intent value handling

    • UserType class is enough to be an enum class

    • UserData.id should use UserId class if we make this class

    • Need @JvmInline annotation to value class

    • Minimize the function's public range

    • Logger has ambiguous function name

    • Unnecessary deep nests

    • Unnecessary Dispatchers.Main

    View Slide

  123. Questions
    1. Tip of the Ice-func

    2. A Composable for everything, and everything in its Composable

    3. Slip Through My Async

    4. Caught on Running

    5. Bitter Sweets

    View Slide

  124. Bitter Sweets
    An activity which receives Parcelable request
    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)
    }
    }

    View Slide

  125. Bitter Sweets
    An activity which receives Parcelable request
    class MainViewModel : ViewModel() {
    suspend fun createDataFlow(request: Request): Flow {
    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 =
    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'"
    }
    }

    View Slide

  126. Bitter Sweets
    An activity which receives Parcelable 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 {
    override fun createFromParcel(parcel: Parcel): Request =
    Request(checkNotNull(parcel.readString()), parcel.readInt())
    override fun newArray(size: Int): Array = arrayOfNulls(size)
    }

    View Slide

  127. Major issues
    1. Unintentional synchronous execution of async functions

    2. OS bug regarding Parcelable that occurs only in Android Tiramisu

    View Slide

  128. Bitter Sweets
    Issue: Unintentional synchronous execution of async functions
    class MainViewModel : ViewModel() {
    suspend fun createDataFlow(request: Request): Flow {
    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 =
    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'"
    }
    }

    View Slide

  129. Bitter Sweets
    Issue: Unintentional synchronous execution of async functions
    class MainViewModel : ViewModel() {
    suspend fun createDataFlow(request: Request): Flow {
    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 =
    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'"
    }
    }

    View Slide

  130. Bitter Sweets
    Issue: Unintentional synchronous execution of async functions
    class MainViewModel : ViewModel() {
    suspend fun createDataFlow(request: Request): Flow {
    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 =
    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'"
    }
    }

    View Slide

  131. Bitter Sweets
    Solution: Use coroutine scope correctly
    class MainRepository {
    fun createDataFlow(request: Request): Flow = flow {
    coroutineScope {
    val localCacheDataDeferred = loadCacheDataAsync(request)
    val remoteDataDeferred = fetchRemoteDataAsync(request)
    emit(localCacheDataDeferred.await())
    emit(remoteDataDeferred.await())
    }
    }
    private fun CoroutineScope.loadCacheDataAsync(request: Request): Deferred =
    async { loadCacheData(request) }
    private fun CoroutineScope.fetchRemoteDataAsync(request: Request): Deferred =
    async { fetchRemoteData(request) }
    private suspend fun loadCacheData(request: Request): String {
    delay(1000)
    return "local cache result of '$request'"
    }
    private suspend fun fetchRemoteData(request: Request): String {
    delay(5000)
    return "remote result of '$request'"
    }
    }

    View Slide

  132. Bitter Sweets
    Solution: Use coroutine scope correctly
    class MainRepository {
    fun createDataFlow(request: Request): Flow = flow {
    coroutineScope {
    val localCacheDataDeferred = loadCacheDataAsync(request)
    val remoteDataDeferred = fetchRemoteDataAsync(request)
    emit(localCacheDataDeferred.await())
    emit(remoteDataDeferred.await())
    }
    }
    private fun CoroutineScope.loadCacheDataAsync(request: Request): Deferred =
    async { loadCacheData(request) }
    private fun CoroutineScope.fetchRemoteDataAsync(request: Request): Deferred =
    async { fetchRemoteData(request) }
    private suspend fun loadCacheData(request: Request): String {
    delay(1000)
    return "local cache result of '$request'"
    }
    private suspend fun fetchRemoteData(request: Request): String {
    delay(5000)
    return "remote result of '$request'"
    }
    }

    View Slide

  133. Major issues
    1. Unintentional synchronous execution of async functions

    2. OS bug regarding Parcelable that occurs only in Android Tiramisu

    View Slide

  134. Bitter Sweets
    Issue: OS bug regarding Parcelable that occurs only in Android Tiramisu
    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)
    }
    }

    View Slide

  135. Bitter Sweets
    Issue: OS bug regarding Parcelable that occurs only in Android Tiramisu
    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)
    }
    }

    View Slide

  136. Bitter Sweets
    Issue: OS bug regarding Parcelable that occurs only in Android Tiramisu
    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)
    }
    }
    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 {
    override fun createFromParcel(parcel: Parcel): Request =
    Request(checkNotNull(parcel.readString()), parcel.readInt())
    override fun newArray(size: Int): Array = arrayOfNulls(size)
    }

    View Slide

  137. Bitter Sweets
    Issue: OS bug regarding Parcelable that occurs only in Android Tiramisu
    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)
    }
    }
    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 {
    override fun createFromParcel(parcel: Parcel): Request =
    Request(checkNotNull(parcel.readString()), parcel.readInt())
    override fun newArray(size: Int): Array = arrayOfNulls(size)
    }

    View Slide

  138. Bitter Sweets
    Solution: Use IntentCompat (and kotlinx-parcelize)
    class MainViewModel(
    savedStateHandle: SavedStateHandle,
    private val repository: MainRepository
    ) {
    private val request: Request = checkNotNull(savedStateHandle[KEY_REQUEST])
    val viewDataFlow: Flow by lazy { repository.createDataFlow(request) }
    object Factory : ViewModelProvider.Factory {
    override fun create(modelClass: Class, extras: CreationExtras): T {
    return MainViewModel(extras.createSavedStateHandle(), MainRepository()) as T
    }
    }
    companion object {
    const val KEY_REQUEST = "request"
    }
    }
    @Parcelize
    @Keep
    data class Request(val word: String, val page: Int) : Parcelable

    View Slide

  139. Bitter Sweets
    Solution: Use IntentCompat (and kotlinx-parcelize)
    class MainViewModel(
    savedStateHandle: SavedStateHandle,
    private val repository: MainRepository
    ) {
    private val request: Request = checkNotNull(savedStateHandle[KEY_REQUEST])
    val viewDataFlow: Flow by lazy { repository.createDataFlow(request) }
    object Factory : ViewModelProvider.Factory {
    override fun create(modelClass: Class, extras: CreationExtras): T {
    return MainViewModel(extras.createSavedStateHandle(), MainRepository()) as T
    }
    }
    companion object {
    const val KEY_REQUEST = "request"
    }
    }
    @Parcelize
    @Keep
    data class Request(val word: String, val page: Int) : Parcelable

    View Slide

  140. Bitter Sweets
    Solution: Use IntentCompat (and kotlinx-parcelize)
    class MainViewModel(
    savedStateHandle: SavedStateHandle,
    private val repository: MainRepository
    ) {
    private val request: Request = checkNotNull(savedStateHandle[KEY_REQUEST])
    val viewDataFlow: Flow by lazy { repository.createDataFlow(request) }
    object Factory : ViewModelProvider.Factory {
    override fun create(modelClass: Class, extras: CreationExtras): T {
    return MainViewModel(extras.createSavedStateHandle(), MainRepository()) as T
    }
    }
    companion object {
    const val KEY_REQUEST = "request"
    }
    }
    @Parcelize
    @Keep
    data class Request(val word: String, val page: Int) : Parcelable

    View Slide

  141. Bitter Sweets
    Solution: Use IntentCompat (and kotlinx-parcelize)
    class MainViewModel(
    savedStateHandle: SavedStateHandle,
    private val repository: MainRepository
    ) {
    private val request: Request = checkNotNull(savedStateHandle[KEY_REQUEST])
    val viewDataFlow: Flow by lazy { repository.createDataFlow(request) }
    object Factory : ViewModelProvider.Factory {
    override fun create(modelClass: Class, extras: CreationExtras): T {
    return MainViewModel(extras.createSavedStateHandle(), MainRepository()) as T
    }
    }
    companion object {
    const val KEY_REQUEST = "request"
    }
    }
    @Parcelize
    @Keep
    data class Request(val word: String, val page: Int) : Parcelable
    IntentCompat.getParcelableExtra(intent, name, class)

    View Slide

  142. Bitter Sweets
    Other issues

    • Data is discarded when rotating the screen, etc.

    • Should use Parcelize library to reduce boilerplate codes

    • Should use SavedStateHandle when intent use on ViewModel

    • Activity may be displayed in a halfway initialized state

    • Should make constant value instead of direct value

    • Should make repository class

    • Flow on lifecycleScope might be run even if state is STOPPED

    • Unnecessary suspendable function to provide
    fl
    ow

    View Slide

  143. How did we create the question?

    View Slide

  144. We have the “Review committe”

    View Slide

  145. Review committee
    Team A
    Team B
    Committee members
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Ref: Confronting quality problems of long-lived codebase, Munetoshi Ishikawa, LINE Corporation https://www.youtube.com/watch?v=z8PwD68_zwg

    View Slide

  146. Review committee
    1. Committee members review “merged” pull-requests
    Team A
    Team B
    Committee members
    Review merged PRs
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Ref: Confronting quality problems of long-lived codebase, Munetoshi Ishikawa, LINE Corporation https://www.youtube.com/watch?v=z8PwD68_zwg

    View Slide

  147. Review committee
    1. Committee members review “merged” pull-requests

    2. Committee members then write and share a weekly report
    Team A
    Team B
    Committee members
    Review merged PRs
    write report
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Ref: Confronting quality problems of long-lived codebase, Munetoshi Ishikawa, LINE Corporation https://www.youtube.com/watch?v=z8PwD68_zwg

    View Slide

  148. Review committee
    1. Committee members review “merged” pull-requests

    2. Committee members then write and share a weekly report
    Team A
    Team B
    Committee members
    Review merged PRs
    write report
    share
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Ref: Confronting quality problems of long-lived codebase, Munetoshi Ishikawa, LINE Corporation https://www.youtube.com/watch?v=z8PwD68_zwg

    View Slide

  149. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports

    View Slide

  150. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports

    View Slide

  151. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports

    View Slide

  152. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports

    View Slide

  153. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports

    View Slide

  154. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports

    View Slide

  155. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports
    Make “Bad Code”
    Questions

    View Slide

  156. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports
    Make “Bad Code”
    class Ugly1 {
    fun dirtyFun1() { /* */ }
    }
    Questions

    View Slide

  157. class Ugly1 {
    fun dirtyFun1() { /* */ }
    }
    How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports
    Make “Bad Code”
    Questions
    class Ugly2 {
    fun dirtyFun2() { /* */ }
    }

    View Slide

  158. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports
    Make “Bad Code”
    Questions
    class Ugly1 {
    fun dirtyFun1() { /* */ }
    }
    class Ugly2 {
    fun dirtyFun2() { /* */ }
    } class Ugly3 {
    fun dirtyFun3() { /* */ }
    }

    View Slide

  159. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports
    Make “Bad Code”
    Questions
    class Ugly1 {
    fun dirtyFun1() { /* */ }
    }
    class Ugly2 {
    fun dirtyFun2() { /* */ }
    } class Ugly3 {
    fun dirtyFun3() { /* */ }
    } class Ugly4 {
    fun dirtyFun4() { /* */ }
    }

    View Slide

  160. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    Code maker
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    Weekly reports
    Make “Bad Code”
    Questions
    class Ugly1 {
    fun dirtyFun1() { /* */ }
    }
    class Ugly2 {
    fun dirtyFun2() { /* */ }
    } class Ugly3 {
    fun dirtyFun3() { /* */ }
    } class Ugly4 {
    fun dirtyFun4() { /* */ }
    } class Ugly5 {
    fun dirtyFun5() { /* */ }
    }

    View Slide

  161. How to make the question?
    Apply the weekly report in reverse to create a "Bad Code".
    “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
    White board
    class Ugly1 {
    fun dirtyFun1() { /* */ }
    }
    class Ugly2 {
    fun dirtyFun2() { /* */ }
    }
    class Ugly3 {
    fun dirtyFun3() { /* */ }
    }
    class Ugly4 {
    fun dirtyFun4() { /* */ }
    }
    class Ugly5 {
    fun dirtyFun5() { /* */ }
    }

    View Slide

  162. Want to learn more?

    View Slide

  163. Watch this video of
    DroidKaigi 2021
    Ref: Confronting quality problems of long-lived codebase, Munetoshi Ishikawa, LINE Corporation https://www.youtube.com/watch?v=z8PwD68_zwg

    View Slide

  164. Thank you for watching!

    View Slide