3
\$\begingroup\$

The variable result is an integer representing the exit code of a command line process that was run via Groovy code. An exit code of 0 means the process was successful and an exit code from 1-255 means a failure. Then this line is executed:

println result ? "The command failed." : "The command succeeded."

What's happening here is that result is being coerced to a boolean (nonzero is true, zero is false) and then the ternary operator is used to determine which message to print, telling the user if the command succeeded or failed.

Is this fine in the context of the Groovy language, or is this going a bit too far with implicit magic?

\$\endgroup\$
2
  • 6
    \$\begingroup\$ Nothing about this seems like magic. It is very straightforward especially in light of the Elvis Operator \$\endgroup\$
    – Daniel
    Nov 7 '13 at 20:40
  • \$\begingroup\$ anyone who can read code should understand Ternary operations. \$\endgroup\$
    – dbarnes
    Nov 8 '13 at 5:32
5
\$\begingroup\$

In your original post, you wrote

The variable result is an integer representing the exit code of a command line process that was run via Groovy code. An exit code of 0 means the process was successful and an exit code from 1-255 means a failure. Then this line is executed:

println result ? "The command failed." : "The command succeeded."

I find it interesting you had to explain it so much. I imagine if this were real code you may have to attach a similar code comment.

Personally I prefer to write code that requires no comments, if possible, and no explanation, by using well-crafted symbols. So for example if you wrote it as

boolean success = (exitCode == 0);
println success ? "The command succeeded." : "The command failed.";

...you wouldn't have to explain that a exit code of 0 indicates success, and it wouldn't require a second glance to figure out the reversed ternary expression.

Any compiler worth its salt will optimize out the working variable and emit exactly the same executable code, so the increased readability costs you nothing.

\$\endgroup\$
1
  • \$\begingroup\$ that's exactly why I made the question. the code isn't complicated, but the fact that it required an explanation was a bad smell. \$\endgroup\$
    – smcg
    Nov 8 '13 at 14:59
2
\$\begingroup\$

I don't do JS or Groovy stuff, so I'm a bit biased (and possibly completely wrong).

What I would normally expect in such an expression is

{bool expression} ? {true part} : {false part}

... and your groovy code has that reversed, which is confusing, if not a bug (I don't know how Groovy implicitly converts integers into Booleans).

If the language allows it, I would cast/convert result into a Boolean to make it more clear, because implicit conversions can cause not only surprises, but also nasty unexpected bugs.

Bottom line, write code as if the next person reading it was a dangerous psychopath that knows where you live - and that person might be your future self!

\$\endgroup\$
5
  • \$\begingroup\$ the standard for exit codes is successful=0 (which would coerce to false) and failure >0 (which would coerce to true) \$\endgroup\$
    – smcg
    Nov 7 '13 at 20:29
  • \$\begingroup\$ If my future self can't understand a ternary expression there's little hope for him. \$\endgroup\$
    – Daniel
    Nov 7 '13 at 20:42
  • \$\begingroup\$ This isn't about the ternary expression (I use them all the time in C#). If result were a Boolean, result ? "command failed" : "command succeeded" would look wrong. What I'm saying is that using implicit conversion in this case forces the casual reader to look twice before saying "ah, ok I get it". OTOH, by making the conversion explicit, you instantly eliminate any ambiguity and find yourself having to reverse the two string values to keep the resulting string correct, hence the readability issue with implicit conversion in this case. \$\endgroup\$ Nov 7 '13 at 20:43
  • 3
    \$\begingroup\$ I Agree with @retailcoder - but would go further and add a function to do the checking for me: println successful(result) ? "the command succeeded." : "The command failed.". This has served me well in the past when dealing with program exit codes, etc. where the 'external' logic is not the same as the natural logic of the language. In C language for example, the 0 == true is 'normal' and the code above would need to be adapted too. \$\endgroup\$
    – rolfl
    Nov 7 '13 at 20:50
  • \$\begingroup\$ May I ask why this post was downvoted?! Has the downvoter also downvoted the accepted answer, which says the same thing worded differently/better? I'll gladly edit this post if I have to. \$\endgroup\$ Nov 8 '13 at 15:48
1
\$\begingroup\$

I think it's common knowledge in groovy, and it's idiomatic, to rely on the "truthiness" of non-boolean values. The only change that I'd suggest to what you have is to put the explicit parens in for the println, so:

println(result ? "The command failed." : "The command succeeded.")

At my first glance at what you wrote, I was misunderstanding the precedence and thinking that you were evaluating the truthiness of println result (which is a void method so would always be "falsy"). With the parens in place, I think it's obvious that you're coercing the result into it's truthiness value and the rest is self documenting.

\$\endgroup\$
3
  • \$\begingroup\$ This still leaves the ternary expression reversed, which still requires a double-read. \$\endgroup\$ Nov 8 '13 at 11:25
  • 1
    \$\begingroup\$ ah, I see what you're saying now about that. yeah, it is a little confusing that the true value is a failure. I think that could be fixed by changing the variable name to something like failedExitCode. \$\endgroup\$
    – Ted Naleid
    Nov 8 '13 at 20:23
  • \$\begingroup\$ Indeed - but John Wu's successful function solution is even better :) \$\endgroup\$ Nov 8 '13 at 20:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.