https://kotlinlang.org logo
#codereview
Title
# codereview
m

miqbaldc

02/03/2022, 1:55 PM
Any recommendation or preferences for following snippet?
preconditions
or
throw
for this case, maybe any references?
1️⃣
Copy code
suspend fun updatePushToken(newPushToken: String): Result<String> = invoke {
    require(newPushToken.isNotEmpty()) { "Push token is required to not empty" }

    kotlin.runCatching { repository.agentUpdatePushToken(newPushToken = newPushToken) }
}
or 2️⃣
Copy code
suspend fun updatePushToken(newPushToken: String): Result<String> = invoke {
        
        if (newPushToken.isEmpty()) {
            throw IllegalArgumentException("Push token is required to not empty")
        }
        
        kotlin.runCatching { repository.agentUpdatePushToken(newPushToken = newPushToken) }
    }
1️⃣ 2
j

Joffrey

02/03/2022, 1:57 PM
Option one personally, if your question is only about the first part. However, I'd rather not use
runCatching
in this context, and certainly not fully qualified like this. Why do you want to catch
OutOfMemoryError
,
NoClassDefFoundError
and other such throwables when calling
agentUpdatePushToken
, but you're fine throwing IAE 2 lines above this?
2
m

miqbaldc

02/03/2022, 2:33 PM
we’re currently use this in
UseCase
(class), here’s the full snippet:
Copy code
class AgentPushTokenNotificationUseCase @Inject constructor(
    private val repository: AccountRepository, // remote or local switch in the implementation detail
    dispatchers: CoroutineDispatchers,
) {

    suspend fun updatePushToken(newPushToken: String): Result<String> = withContext(dispatchers) {
        require(newPushToken.isNotEmpty()) { "Push token is required to not empty" }

        kotlin.runCatching { repository.agentUpdatePushToken(newPushToken = newPushToken) }
    }
}
j

Joffrey

02/03/2022, 2:36 PM
What's
invoke
here? Your class doesn't seem to extend any function type. Also, I don't believe this answers my points about
runCatching
🤔
m

miqbaldc

02/03/2022, 2:41 PM
The
runCatching
part intention was to consider as a tolerable failures (such any kind of throwable) in our case (and we transform it into a
Result.failure(…)
), and in a UI Layer, we can shows some sort of events/effect (e.g: showing a dialog in this case)
What’s 
invoke
 here? Your class doesn’t seem to extend any function type.
shot, lemme update it first, done updated
j

Joffrey

02/03/2022, 2:44 PM
I think you should catch a smaller subset of exceptions for this purpose, and convert them into a more suited result type. This
kotlin.Result
type was not meant to represent business errors. Please take a look at this: https://stackoverflow.com/questions/70847513/when-and-how-to-use-result-in-kotlin
💯 1
👀 1
today i learned 1
m

miqbaldc

02/03/2022, 3:32 PM
If it in domain/business-usecase (or model), we use a specific sealed class to determine business-error. Does it makes senses to use
runCatching
or an operator such as
Flow.catch
for error handling in the consumer side (e.g: UI Layer)?
e

ephemient

02/03/2022, 7:14 PM
runCatching will catch Error which is almost always a mistake. it will also catch CancellationException which will break coroutine cancellation and catch InterruptedException which will break thread interruption.
1
Result is meant for use in continuations/flows; the failure definitely gets unwrapped and thrown later
for your UI layer, you really should consider exactly which exceptions you can handle
✍️ 1
m

miqbaldc

02/05/2022, 7:21 AM
Any example/references for how to write error handling in a domain layer?
Here’s my one cents:
Copy code
sealed class AgentNotificationsResult {
    object EmptyNewPushToken : AgentNotificationsResult()
    data class FailUpdatePushToken(val e: Exception) : AgentNotificationsResult()
    object SuccessUpdatePushToken : AgentNotificationsResult()
}

class AgentNotificationsUseCase @Inject constructor(
    private val repository: AccountRepository,
    private val dispatchers: CoroutineDispatchers = Default,
) {

    suspend fun updatePushToken(newPushToken: String): AgentNotificationsResult = withContext(dispatchers) {
        if (newPushToken.isEmpty()) {
            AgentNotificationsResult.EmptyNewPushToken
        }

        try {
            repository.agentUpdatePushToken(newPushToken = newPushToken)
            AgentNotificationsResult.SuccessUpdatePushToken
        } catch (e: Exception) {
            AgentNotificationsResult.FailUpdatePushToken(e = e)
        }
    }
}
Not sure for the
try-catch
part that catching general
Exception
is considered as “business error”, as it’ll catch
retrofit2.HttpException
or
SocketTimeoutException
from
:data
layer.
There’s an article mentioning about transforming the
:data
exception to a `sealed class`/`inteface` as well in here:

https://miro.medium.com/max/1400/1*0HepdkVU-3zhoXBD2R0aLw.png

(the
:base
has a specific result type and exception in this case)
message has been deleted
m

Matteo Mirk

02/08/2022, 10:55 AM
require()
will throw an IAE so they’re equivalent. I would choose [1]
3 Views