Does single-line if-statement without curly braces violate Kotlin code style?

1

Given:

  1. I started working in a new team on a greenfield project which has 100% Kotlin code base
  2. Team members mostly have Java experience
  3. By single-line if-statement I mean the following:
if (list.isEmpty) throw IllegalArgumentExpression()

The problem:

I am arguing with my team mate if the above one-liner should be considered as a style violation in our Kotlin code or not. He insists that any if-block requires curly braces, while I do not think so.

For me, having recent Ruby experience, where I’ve seen a lot of:

raise 'Invalid input' if list.empty?

the above Kotlin one-liner looks so natural, that I was very surprised it can be considered as a style violation.

I decided to check what are the Java rules for most widely known Checkstyle configurations (Google and Oracle Java code styles). I was surprised, that both Checkstlyle Java configs have a NeedBraces rule enabled for if-statements.

But while this is true for old Java code styles, I still do not think we should bring it to Kotlin. In Kotlin world there are ktlint and detekt tools for checking formatting and none of them with default config gets triggered on a single-line if-statement. By default, detekt only complains on multi-line if-statements without curly braces:

if (list.isEmpty) 
    throw IllegalArgumentExpression()

and I agree with it. This is a very old rule for most of the languages and the motivation for it is well-known. Adding the 2nd statement with the same level of indentation as the 1st one won’t work as you might expect.

So, in my opinion one-line if-statement without curly-braces is fine, while multi-line if-statement without braces is error-prone and should be avoided.

There is another argument my team mate uses to convince me that single-line if-statement without braces is bad. When I type the following code:

fun someFunction(input: List<String>) {
    if (input.isEmpty) throw IllegalArgumentException("Input can't be empty")

    if (someFlagWithQuiteLongName) {
        return delegate.computeResultWithTheFunctionHavingVeryLongNameWhichWillExceed120CharsWhenUsedOnOneLineWithIfStatement(input)
    }
}

he says that I am inconsistent with myself, because for 1st condition I do not use curly braces, and right after that I do. The only reason for the 2nd multi-line if-statement above is obvious: it does not fit the default 120 chars line length, so I split my if-statement to multiple lines, where I do use curly braces for the reasons already described above.

If the goal is to write the modern Kotlin code (let’s forget about past Java experience) following the official Kotlin code style (or if it does not answer the question, using the most widely adopted style), should we enforce curly braces for single-line if-statements (like old Java code styles do) or allow single-line if-statements without {}?

3 Likes
2

Maybe tell your team mate that even the official Kotlin documentation uses this syntax a lot? Conditions and loops | Kotlin

Kotlin authors intentionally didn’t include the ternary operator in the language, because it duplicates if expressions.

You can also ask your team mate if we should always use braces in cases of when. Because braces are optional there as well and we sometimes put a simple single-line expression, but sometimes we create a section of code.

edit:

Having said that, I’m not sure about your specific case. Single-line brace-less if looks good if it is used as an expression. But you use it as a regular control-flow statement and I would probably add braces there. But this is just me, I’m interested in others opinion.

Also, this specific case could be replaced with simple:

require(list.isNotEmpty)
5 Likes
3

I suspect personal taste is a big factor in this. People seem to have very different opinions about it (in Kotlin, and also in other C-like languages).

For what it’s worth, I’m perfectly happy with brace-less if statements — but only if the body is on a separate line; if they’re all on one line, I find them far too easy to miss when scanning code. (I’ve had colleagues who like to write them like that, so I speak from experience…)

4

From my experience, I much prefer this:

if (condition)
    code();  // Next line, indented

Having everything on the same line makes code harder to read (it’s easier to miss the contents of the if). It’s also very rarely needed: the only reason I can think of is for early returns/exceptions in case some data is incorrect, in which case Kotlin has better syntax already:

// IllegalArgumentException
require(list.isNotEmpty) { "The parameter 'list' should not be empty." }

// Early return when a value is nullable
val param = someParam ?: return
5

Your example (no curly braces on a single line) is used by JetBrains in their docs and code. We use it and I see it in plenty of open source projects. As you said, it reads very naturally. I don’t think it can reasonably be considered a style violation.

Suggestion: Consider putting together a simple Kotlin style guide for your company on your wiki or as a Google doc. In the guide explain your criteria for style rules (e.g. used by JetBrains, etc.)

1 Like
6

I wouldn’t go as far as to call it “a rule”. Even the page you link to doesn’t provide any definitive answers. The reasoning that it’s bad because it can lead to bugs is worth considering, but too weak to make it a solid rule. Pretty much anything can lead to bugs, depending on how you (ab)use it.

Personally, I’m totally fine with braceless ifs, as long as they are simple error checks or early returns, like if .. throw or if .. return null. Anything more serious than that, I add braces, even if they are still single line. If I want to comment out such a statement for debugging, I’ll comment out both lines, and adding ugly code in a hurry… I just don’t do it. I may add ugly logic, but not ugly formatted code.

But I also understand that some people want to always have braces for additional security, trading vertical space for it. It’s a fair trade-off.

Back on topic, I really, and I mean really, hate one-line ifs. Their obvious weak point is that they are much less noticeable than muti-liners. A few days ago my boss called me to help in debugging some really non-obvious piece of code. It had a few debugging outputs, and he showed a log with outputs. He had a hard time figuring out how a value can be first output as 30, then incremented by one, and then printed out as 0. We spent a few minutes searching for any assignments to that variable, and failed to find any. Then, after a while, I realized that there was an if ... return between the two outputs, so I figured out the outputs must be from different invocations, so the value was 30 at first, then the code hit a return, and then, on the next invocation it was 0. It was far from obvious from what was going on, but it turned out to be the case, and helped to figure out the reason. Now, that if was a two-liner. If it was a single-liner, it would probably take me much longer to even notice it.

But this isn’t the main reason I hate one-line ifs. After all, if we avoid them in complicated code, and only use for early returns and exceptional conditions, they would be quite fine in this regard. But there’s more to them.

The second reason is stack traces. When an if ... throw throws, it can be hard to figure out whether it’s the throw statement or the condition itself that caused the exception. In most cases it can be figured by looking at the exception message, though, so that’s not the main reason either. Still, sometimes it can be rather annoying. It’s always better to know the exact statement where the exception happened.

But the main reason is debugging. When I hit a special condition, that threw an exception, or returned null where I think it shouldn’t, all I have to do is to put a break point on the very line that contains that throw or return. If it’s a single-liner, I’ll have to edit the code first. That is a hassle by itself, since I have to remember to revert it back to avoid committing unnecessary pointless changes, and it can also be not an easy thing to do—for example, when debugging some pre-compiled code running outside the IDE, perhaps even remotely. Of course, I can sometimes put a conditional break point, but it’s not always easy, and sometimes painfully slow.

So, no, no one-liners for me, thank you very much. Whether two-line single-statement ifs should use braces or not is debatable, but one-liners? No, there’s so little to gain and so much to lose, that I’m even thinking of making all checks and requires multiline as well.