Clarify using braces and return instead of equal sign

Language Design
1

Could someone explain to me using a “=” sign in a function instead of curly braces and a “return” statement?

For example, I have a function:

private fun updateStatus(id: String, status: Status) =
getById(id)
.takeIf { it.status !in prohibitedStatuses }
?.apply { this.status = status }
?.let { repository.save(it) }
?.also {
logger.info { “New status. id ${it.id}: ${it.status}” }
}
?: throw ProhibitStatusChangingException(status, prohibitedStatuses)

Some of my teammates say that so long function has to be in curly braces with a “return” statement, but I think if a function is a single expression, also with multiple chain calls, it could be defined with “=” sign, like in the example above.

This situation isn’t described in “Kotlin Coding Conventions”, it would be very nice if someone could clarify this situation. Thanks!

2

There are no clear rules about it. Personally I like to use = when I have either one-liner or a simple block like fun foo() = when{...}.

In your particular case code looks unreadable, but it is not because of =, but because you clearly abused chaining operations. There are completely no reasons not to use straight forward imperative logic here with return in the end.

2 Likes
3

Actually, I think I wouldn’t have quite so much of a problem with this particular example — as long as it were properly indented, of course! (I think the lack of indentation really isn’t helping OP here…)

But I agree: the variety of different operations here does make it quite hard to follow. (The let() half-way down is a particular offender there, I think.) It would almost certainly be easier to read and maintain if it were a traditional function body.

4

I love oneliners.
But, only when it’s combined with lots of functions.

norma functions

When you use normal functions, you can get away with having bigger functions.

private fun updateStatus(id: String, status: Status) : StateHaver{
   val oldStateHaver = getById(id) 
   if(oldStateHaver.status in prohibitedStatuses) 
       throw ProhibitStatusChangingException(status, prohibitedStatuses)
   
    oldStateHaver.status = status
    val newStateHaver = repository.save(oldStateHaver)
    logger.info { 
        newStateHaver.run{ “New status. id $id: $status” }
    }
    return newStateHaver
}

inline functions

But, when you’re writing with oneliners, this doesn’t look well.
This makes that you need to do more work to make it look acceptable.
The most important reason is that you’re not chaining two actions, but two implementations.
The actions are updating and checking if it’s valid.

for checking it’s valid, you could use classes:

fun StateHaver.asValidOrNull() = ValidStateHaver.orNull(this)
class ValidStateHaver private constructor (val stateHaver : StateHaver){
    companion object{
        fun orNull(
                prohibitedStates : List<States>
        ) = when(status in prohibitedStateHaver){
            true -> null
            false -> ValidStateHaver(this)
       }
}

You can create a function to the valid state do do the real updating:

private fun ValidStateHaver.updateStatusTo(status : Status) : StateHaver{
    this.status = status
    return repository.save(this).apply{
        logger.info {"New status. id $id : $status" }
    }
}

Now your function becomes:

private fun updateStatus(id: String, status: Status) = getById(id)
     .asAllowedOrNull(prohibitedStatuses = prohibitedStatuses)
     ?.updateStatusTo(status)
     ?: throw ProhibitStatusChangingException(status, prohibitedStatuses)

closing remarks

Now, you can’t forget to check as it simply won’t compile.
I choose for safety, but you could choose for memory/speed with inline classes.
Overkill? just make your code look a bit more similar to the last. Or ignore everything I typed :wink:

2 Likes
5

The downside is that it may hide the type. So if the function is public or protected I add a return type regardless of using = or {}
The decision on using an = or not depends on readability.

1 Like
6

I’m not a fan of arbitrary rules like “no long = functions”, BUT it looks to me like you compromised the functionality just so you didn’t have to use braces and return, and that’s a problem.

You are throwing ProhibitStatusChangingException if repository.save returns null, even if a prohibited status is not used. Is that really appropriate?

7

No, getById() returns only not-null value, also it throws exception on unexisted entity