Kotlin / kotlin-style-guide

Work-in-progress notes for the Kotlin style guide
288 stars 14 forks source link

Lambda formatting #10

Open yole opened 8 years ago

yole commented 8 years ago

When possible, put lambdas on a single line, using it for single-parameter lambdas:

elements.dropWhile { it !is KDoc }

Don't use it in nested lambdas; always declare parameter names explicitly.

When declaring parameter names in a multiline lambda, put the names on the first line, followed by the arrow and the newline:

appendCommaSeparated(properties) { prop ->
    val propertyValue = prop.get(obj)  // ...
}
voddan commented 8 years ago

Simple collection operations inside nested lambdas are ok to use it in:

appendCommaSeparated(properties) { props ->
    props.map {it.name}
}
damianw commented 8 years ago

To improve readability, multiline single-argument lambdas should also always name the argument instead of using it, since it can become unclear in longer lambdas.

// Good
appendCommaSeparated(properties) { prop ->
  val propertyValue = prop.get(obj)  // ...
}

// Bad
appendCommaSeparated(properties) {
  val propertyValue = it.get(obj)  // ...
}
trevjonez commented 7 years ago

what about the case where the method has multiple function args like with RxJava's subscribe method?

someObservable.subscribe { next -> doThing(next) }
someObservable.subscribe({ next -> 
    doThing(next) 
}) { error -> 
    error.printStackTrace() 
}
someObservable.subscribe({ next -> 
    doThing(next) 
}, { error -> 
    error.printStackTrace() 
}) { 
    logger.logStreamCompletion() 
}

Seems to me that once there are multiple lambda arguments, forcing the last one outside of the parenthesis may not be desirable?

someObservable.subscribe({ next ->
    doThing(next)
}, { error ->
    error.printStackTrace()
}, { 
    logger.logStreamCompletion() 
})
Zhuinden commented 5 years ago

@trevjonez I think at that point it is a better option to use named arguments without trailing lambdas, that way you don't confuse lambda order and keep it readable

trevjonez commented 5 years ago

I do think we both agree that we want to optimize for reading clarity.

Anymore I find I always put it inside parenthesis when a follow up call might look ambiguous as to what it operates on to those with less context of the code.

again with an rxjava usecase as an example.

someSingle().subscribe({ doThings(it) }) { error ->
  logger.log(error)
  throw error
}.trackInMap(someMap, someKey)

is trackInMap operating on the lambda or on the result of subscribe?

Lets adjust the parens and see if it is more clear:

someSingle().subscribe({ doThings(it) }, { error ->
  logger.log(error)
  throw error
}).trackInMap(someMap, someKey)

the compiler can solve it fine either way, but for us humans that parenthesis disambiguates with minimal mental effort.

Zhuinden commented 5 years ago

Hrmm to be a bit more specific, I think the best practice in this case is to wrap it with an extension function that allows you to use named arguments.

Just like RxKotlin does with subscribeBy.

https://github.com/ReactiveX/RxKotlin/blob/d20fa56bbe72c369b53f553a9710841eca1b63de/src/main/kotlin/io/reactivex/rxkotlin/subscribers.kt#L34-L38

Because now you can do

someSingle().subscribeBy(
    onSuccess = { result -> doThings(result) },
    onError = { throwable -> 
        logger.log(throwable)
        throw throwable
    })

Now you definitely won't get confused about which lambda is for what.