When should I use let {} and when just plain != null

Question

How would you write the following code in a more kotlinic way ?

var returnValue = ...
val s3data = presignedUrl.body()
if (s3data != null) {
    val uploadImage = api.uploadImage(s3data.bucketUrl, s3data.awsAccessKeyId, s3data.policy, s3data.key, s3data.signature, body).execute()
    if (!uploadImage.isSuccessful) {
        crashReporterService.sendIssue("Failed uploading file", "Failed uploading file ${uploadImage.raw()}")
        returnValue = Result.FAILURE
    } else {
        returnValue = Result.SUCCESS
    }
} else {
    crashReporterService.sendIssue("Failed uploading image", "Error - ${presignedUrl.raw()}")
    returnValue = Result.FAILURE
}

return returnValue

I can use let, but I feel it makes the code more complicated to understand

a note on your code: returning asap is a good practice afaik, reduces scope and simplifies the logic — DPM, Dec 02 '17 at 20:27

F. George · Accepted Answer · 2017-12-03T07:50:22.873

Common shared code - in this case, error reporting and return with failure result - can be consolidated into a local function.
Nullability causing a return (in this case, s3data being nullable) can usually be replaced with a returning ?: elvis operator.
When typing the same variable over and over again (in this case, accessing s3data), a run block is appropriate. If confused, see What is a "receiver" in Kotlin?
As already stated in another answer, if/else blocks are an expression in Kotlin.

I would therefore find the following implementation most ideomatic, subject to proper naming of parameters for the local function:

fun foo() {
    fun failure(p0: String, p1: String) = crashReporterService.sendIssue(p0, p1).let { Result.FAILURE }
    val s3data = presignedUrl.body() ?: return failure("Failed uploading image", "Error - ${presignedUrl.raw()}")
    val uploadImage = s3data.run { api.uploadImage(bucketUrl, awsAccessKeyId, policy, key, signature, body).execute() }

    return if (uploadImage.isSuccessful) {
        Result.SUCCESS
    } else {
        failure("Failed uploading file", "Failed uploading file ${uploadImage.raw()}")
    }
}

Your question is bordering on code review, so you'll probably also be happy to know there's a dedicated Stack Exchange network just for that. Do read A guide to Code Review for Stack Overflow users before, however.

Oliver Charlesworth · Answer 2 · 2017-12-02T20:30:47.670

if/else is an expression in Kotlin, so the following is certainly more Kotlinesque:

val s3data = presignedUrl.body()
return if (s3data != null) {
    val uploadImage = api.uploadImage(s3data.bucketUrl, s3data.awsAccessKeyId, s3data.policy, s3data.key, s3data.signature, body).execute()
    if (!uploadImage.isSuccessful) {
        crashReporterService.sendIssue("Failed uploading file", "Failed uploading file ${uploadImage.raw()}")
        Result.FAILURE
    } else {
        Result.SUCCESS
    }
} else {
    crashReporterService.sendIssue("Failed uploading image", "Error - ${presignedUrl.raw()}")
    Result.FAILURE
}

When should I use let {} and when just plain != null

2 Answers2