Slide 1
Slide 1 text
Yuki Ando, LINE Corporation
Code Review Challenge
An example of a solution
DroidKaigi 2023: ίʔυϨϏϡʔνϟϨϯδͷղઆ
Slide 2
Slide 2 text
Self-introduction
Name: Yuki Ando / ҆౻ ༞و
Twitter: @m1zyuk1
Hobby: Car, Game
Main activity:
Apply new Android feature to LINE Android
Slide 3
Slide 3 text
Did you participate in
DroidKaigi 2023 offline?
Slide 4
Slide 4 text
Yes I am
“Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
Slide 5
Slide 5 text
Have you participated in the
Code Review Challenge?
Slide 6
Slide 6 text
No content
Slide 7
Slide 7 text
No content
Slide 8
Slide 8 text
What’s Code Review Challenge?
Slide 9
Slide 9 text
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/
Slide 10
Slide 10 text
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/
Slide 11
Slide 11 text
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/
Slide 12
Slide 12 text
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/
Slide 13
Slide 13 text
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/
Slide 14
Slide 14 text
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/
Slide 15
Slide 15 text
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/
Slide 16
Slide 16 text
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/
Slide 17
Slide 17 text
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/
Slide 18
Slide 18 text
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/
Slide 19
Slide 19 text
Explain the assumed solutions
Slide 20
Slide 20 text
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/
Slide 21
Slide 21 text
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
Slide 22
Slide 22 text
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
}
Slide 23
Slide 23 text
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)
}
}
Slide 24
Slide 24 text
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()
}
Slide 25
Slide 25 text
Major issues
1. OutputStream is not handled properly
2. Signatures that can be used incorrectly
3. Structure of functions with poor readability
Slide 26
Slide 26 text
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()
}
Slide 27
Slide 27 text
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()
}
Slide 28
Slide 28 text
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()
}
Slide 29
Slide 29 text
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())
}
}
Slide 30
Slide 30 text
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())
}
}
Slide 31
Slide 31 text
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
}
}
Slide 32
Slide 32 text
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
}
}
Slide 33
Slide 33 text
Major issues
1. Might not be called OutputStream.close()
2. Signatures that can be used incorrectly
3. Structure of functions with poor readability
Slide 34
Slide 34 text
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
}
Slide 35
Slide 35 text
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 }
Slide 36
Slide 36 text
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 }
Slide 37
Slide 37 text
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 }
Slide 38
Slide 38 text
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 }
Slide 39
Slide 39 text
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()
}
Slide 40
Slide 40 text
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()
}
Slide 41
Slide 41 text
Major issues
1. Might not be called OutputStream.close()
2. Signatures that can be used incorrectly
3. Structure of functions with poor readability
Slide 42
Slide 42 text
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
}
Slide 43
Slide 43 text
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
}
Slide 44
Slide 44 text
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)
}
}
Slide 45
Slide 45 text
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)
}
}
Slide 46
Slide 46 text
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
Slide 47
Slide 47 text
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/
Slide 48
Slide 48 text
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
)
}
}
Slide 49
Slide 49 text
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
}
Slide 50
Slide 50 text
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
}
}
Slide 51
Slide 51 text
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
Slide 52
Slide 52 text
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
)
}
}
Slide 53
Slide 53 text
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
)
}
}
Slide 54
Slide 54 text
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
}
Slide 55
Slide 55 text
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
}
Slide 56
Slide 56 text
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
)
}
Slide 57
Slide 57 text
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
)
}
Slide 58
Slide 58 text
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
Slide 59
Slide 59 text
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"
)
}
}
Slide 60
Slide 60 text
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"
)
}
}
Slide 61
Slide 61 text
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"
)
}
}
Slide 62
Slide 62 text
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")
}
}
Slide 63
Slide 63 text
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")
}
}
Slide 64
Slide 64 text
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
Slide 65
Slide 65 text
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"
)
}
}
Slide 66
Slide 66 text
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"
)
}
}
Slide 67
Slide 67 text
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")
}
}
Slide 68
Slide 68 text
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")
}
}
Slide 69
Slide 69 text
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
}
Slide 70
Slide 70 text
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
Slide 71
Slide 71 text
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/
Slide 72
Slide 72 text
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 */ }
}
Slide 73
Slide 73 text
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 */ }
}
Slide 74
Slide 74 text
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
}
}
Slide 75
Slide 75 text
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
}
}
Slide 76
Slide 76 text
Major issues
1. Cause the app to crash
2. Hard to understand what this function does
3. Race condition occurs
Slide 77
Slide 77 text
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
}
}
Slide 78
Slide 78 text
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
}
}
Slide 79
Slide 79 text
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()
}
Slide 80
Slide 80 text
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()
}
Slide 81
Slide 81 text
Major issues
1. Cause the app to crash
2. Hard to understand what this function does
3. Race condition occurs
Slide 82
Slide 82 text
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
}
}
Slide 83
Slide 83 text
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
}
}
Slide 84
Slide 84 text
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
}
}
Slide 85
Slide 85 text
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()
}
Slide 86
Slide 86 text
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()
}
Slide 87
Slide 87 text
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()
}
Slide 88
Slide 88 text
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
Slide 89
Slide 89 text
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/
Slide 90
Slide 90 text
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) }
}
Slide 91
Slide 91 text
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) }
}
Slide 92
Slide 92 text
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" }
}
Slide 93
Slide 93 text
Major issues
1. Do not use `runCatching`
2. Do not overload functions
3. Data class has value that doesn’t have implementation of equals
Slide 94
Slide 94 text
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) }
}
Slide 95
Slide 95 text
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) }
}
Slide 96
Slide 96 text
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)
}
Slide 97
Slide 97 text
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
Slide 98
Slide 98 text
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
Slide 99
Slide 99 text
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
Slide 100
Slide 100 text
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
Slide 101
Slide 101 text
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
Slide 102
Slide 102 text
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) }
}
Slide 103
Slide 103 text
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
}
}
Slide 104
Slide 104 text
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
}
}
Slide 105
Slide 105 text
Major issues
1. Do not use `runCatching`
2. Do not overload functions
3. Data class has value that doesn’t have implementation of equals
Slide 106
Slide 106 text
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
}
}
Slide 107
Slide 107 text
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
}
}
Slide 108
Slide 108 text
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?
}
Slide 109
Slide 109 text
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?
}
Slide 110
Slide 110 text
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?
}
Slide 111
Slide 111 text
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
}
});
Slide 112
Slide 112 text
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
Slide 113
Slide 113 text
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?
}
Slide 114
Slide 114 text
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?
}
Slide 115
Slide 115 text
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?
}
Slide 116
Slide 116 text
Major issues
1. Do not use `runCatching`
2. Do not overload functions
3. Data class has value that doesn’t have implementation of equals
Slide 117
Slide 117 text
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
)
Slide 118
Slide 118 text
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 { }
}
Slide 119
Slide 119 text
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 { }
}
Slide 120
Slide 120 text
Caught on Running
Solution: Use ordinal class
class UserData(
val id: UserId,
val userName: UserName,
val userType: UserType,
val profileImage: Bitmap
)
Slide 121
Slide 121 text
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
)
Slide 122
Slide 122 text
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
Slide 123
Slide 123 text
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
Slide 124
Slide 124 text
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)
}
}
Slide 125
Slide 125 text
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'"
}
}
Slide 126
Slide 126 text
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)
}
Slide 127
Slide 127 text
Major issues
1. Unintentional synchronous execution of async functions
2. OS bug regarding Parcelable that occurs only in Android Tiramisu
Slide 128
Slide 128 text
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'"
}
}
Slide 129
Slide 129 text
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'"
}
}
Slide 130
Slide 130 text
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'"
}
}
Slide 131
Slide 131 text
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'"
}
}
Slide 132
Slide 132 text
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'"
}
}
Slide 133
Slide 133 text
Major issues
1. Unintentional synchronous execution of async functions
2. OS bug regarding Parcelable that occurs only in Android Tiramisu
Slide 134
Slide 134 text
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)
}
}
Slide 135
Slide 135 text
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)
}
}
Slide 136
Slide 136 text
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)
}
Slide 137
Slide 137 text
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)
}
Slide 138
Slide 138 text
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
Slide 139
Slide 139 text
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
Slide 140
Slide 140 text
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
Slide 141
Slide 141 text
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)
Slide 142
Slide 142 text
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
Slide 143
Slide 143 text
How did we create the question?
Slide 144
Slide 144 text
We have the “Review committe”
Slide 145
Slide 145 text
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
Slide 146
Slide 146 text
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
Slide 147
Slide 147 text
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
Slide 148
Slide 148 text
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
Slide 149
Slide 149 text
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
Slide 150
Slide 150 text
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
Slide 151
Slide 151 text
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
Slide 152
Slide 152 text
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
Slide 153
Slide 153 text
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
Slide 154
Slide 154 text
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
Slide 155
Slide 155 text
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
Slide 156
Slide 156 text
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
Slide 157
Slide 157 text
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() { /* */ }
}
Slide 158
Slide 158 text
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() { /* */ }
}
Slide 159
Slide 159 text
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() { /* */ }
}
Slide 160
Slide 160 text
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() { /* */ }
}
Slide 161
Slide 161 text
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() { /* */ }
}
Slide 162
Slide 162 text
Want to learn more?
Slide 163
Slide 163 text
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
Slide 164
Slide 164 text
Thank you for watching!