Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I often like to do things like this:

return (aCondition == aThing)
        ? someLongExpression.getAThing()
        : somethingElse;

Is this practice considered more or less readable than using a simple if/else block, like this:

if (aCondition == aThing)
    return someLongExpression.getAThing();
else
    return somethingElse;
share

locked by Jamal Dec 20 '13 at 6:42

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

    
The second example has a hidden [harmful] goto. –  richard Mar 30 '12 at 11:54
    
@richard What's the difference? –  Quentin Pradet Mar 30 '12 at 12:05
    
The first example does not have a hidden [harmful] goto. The harm is not in what the code does but in analyzing the code. See my answer below. –  richard Mar 30 '12 at 12:10
comments disabled on deleted / locked posts

7 Answers 7

up vote 39 down vote accepted

I find that the ternary operator (or conditional operator, as it's officially known in C#) can be used for improvement or abused horribly. So, employing a stock answer to many programming questions like this, it depends.

I generally think that fewer lines of code, if done right, minimizes the points of failure that could occur. One return statement in one line of code versus two returns embedded in an if-else-control block.

Now, without more context around your example, it's hard to say whether you're using it in its intended spirit or not. I'm inclined to say "yes" it's good and is more readable over the alternative given what I see.

share
add comment

If the content of the ternary operator doesn't get too long, I find it often more readable than if else. And in your example you have one return instead of two, which enhances readability.

There is an important difference between if else and the ternary operatator: Only the latter returns a value. This allows to use it on places where you can't use if else, and to reduce code repetition. In some cases, e.g. in the super-call of a constructor or an assignment to a static class variable, the alternatives are awkward.

One idiom I use sometimes, but which I never saw in other code is something like

(user.wantsEmail() 
  ? getEmailProvider() 
  : getMailProvider()
).send(message);  
share
    
I really like this answer, since I completely agree with it. However, I'm accepting the other one, since it's more general and answers the question more directly. –  Jake King Mar 29 '12 at 19:39
    
I'm glad to hear this, thanks for the feedback! –  Landei Mar 30 '12 at 6:51
1  
The idiom you present looks a bit messy. I think a more readable approach is to store the mail provider in separate variable: Provider provider = (... ? ... : ...); provider.send(message). Using the same formatting as you do, it wouldn't even increase line count. –  Xion May 7 '12 at 14:57
    
@Xion: That depends, e.g. the variable might be generic, so you would be forced to type all the nasty <> stuff. –  Landei May 8 '12 at 10:09
add comment

You've defeated the purpose of that operator - to increase readability! You've taken a syntax designed to reduce code to one line and used it in place of a construct designed for multiple lines.

The only time you should use the ternary operator is when there is zero chance of any new options being added later. What if you later change it to return something different for greater than, less than, and equals? Or what if you want to add another operation after you do somethingElse?

Here's a case where ternary is perfect:

String msg = "You won " + results.size() + item + (results.size() > 1? "s": "") + ".";

Inline, this replaces a good 5-6 lines of code.

Side note: don't ever do something like this. Your successors will curse you.

return ((a==b)?(g==c?true:false):something();
share
6  
I somewhat disagree. I think that using the operator in a multiline structure does increase readability, since you aren't repeating return each time, instead only defining a value. Additionally, this is a case where only two options are present anyway, and it will never expand further. In theory, I could put it on one line, but I think spacing it out increases readability. Just my opinion, hence why I'm asking it here. –  Jake King Mar 29 '12 at 18:14
4  
return ((a==b)?(g==c?true:false):something();can be written as return a==b ? g==c : something(); –  Landei Mar 30 '12 at 6:52
1  
I'd say your perfect case is pretty ugly. In that case I'd, at least without some string formatting. String msg = String.format("You won %s item%s.", results.size(), results.size() > 1 ? "s" : ""); (that might be a mix of php and c#, and also I'd put the two last parameters on separate lines) –  Svish Mar 30 '12 at 10:18
5  
Nonsense. The conditional operator isn’t about lines of code, it’s about number of statements. It reduces a conditional statement with two blocks down to one single expression. Whether that’s expressed on one or multiple lines is irrelevant. I happen to think that OP’s decision to split up the lines is perfect. –  Konrad Rudolph Mar 30 '12 at 10:31
add comment

I think the ternary operator is fine to use in that case. It's an if-else simplified by using a single return keyword. If the condition is long you can use a boolean variable to store it, and I prefer to format ternary statements like this:

return isConditionTrue ? someLongExpression.getAThing()
                       : somethingElse;

This also works well for when you have multiple conditions.

share
add comment

No, it is not bad practice.

Addendum:

[opinion]

Not everyone is onboard with this, but since the ternary operator is chainable and allows you to put condition and result side-by-side in a ceremony-free manner, I really like using it as follows.

return (aCondition == aThing)    ? someLongExpression.getAThing() :
       (aCondtion == otherThing) ? somethingElse.DoThis() :
       (oneMore == comparison)   ? somethingElse.DoThisThen() :
                                 : somethingElse;

It's not quite as clean as maps in functional languages, but it's similarish looking.

Though I think everyone can agree that at some point you get enough conditions that you should use an appropriate design pattern.

Edit: As pointed out in the comments, the above code is actually a really poor example of a time when this pattern makes sense. A better example is

return (customer.Country == "USA")   ? usa.ShippingFor(customer) :
       (customer.Region == "Europe") ? germany.ShippingFor(customer) :
                                     : NotAvailableShipping.Instance;

As always, this is not a replacement for appropriate design patterns. For example, if the above code had any need to get any more complex I would consider refactoring to a strategy pattern.

[/opinion]

share
1  
This is effectively like using a switch-case block, but I find this less readable than looking at a switch-case block. The advantage to this is that the comparisons being made don't always have to be made on the same value. –  leonardo Apr 5 '12 at 2:42
    
This is like an else-if chain except it is an expression not procedural. Except those words do (unless the return type is a procedure, but not all return values are a procedure). –  richard May 9 '12 at 9:38
    
@richard yes, its almost the same thing as an if-else chain, it just brings all the conditions up to the same level and forces you to call method for anything that is more than one line. Plus it looks like what code looks like in other, nicer, languages. But you're right, it absolutely has its limitations - I just like to bring it up as a style that I believe is underused. –  George Mauer May 9 '12 at 13:52
    
I believe I am miss-understood. I love your (@George Mauer) code, except I would never put a procedure in a ternary, only functions. And mixing procedures and functions is way out. –  richard May 11 '12 at 14:48
    
Ah, good point, yes, I did not mean to imply by my example that you should do this. This works best with functions and I wouldn't recommend it with anything else. –  George Mauer May 11 '12 at 18:52
show 2 more comments

It is not just about size.

The ternary operator is an expression (the 3 parts should be expressions, Condition a Boolean expression, the other two should be of the same type as each other). The type of the ternary expression is the same type as other two expressions.

S = AValue if Condition else AnotherValue

More generally, Condition, AValue, AnotherValue, S are expressions, Condition, AValue, AnotherValue are r-values, S is an l-value, AValue must conform to S, AnotherValue must conform to S, Condition must be Boolean.

If Then Else is procedural, it should do something. (often referred to as side effects, in the C tradition, where everything is a Function).

if Condition then do Something else do SomethingElse end  

If you understand this then the version using ternary is much easier to reason.

Examples shown in generic, human readable, pseudo code (any resemblance to other languages is purely coincidental).

share
    
This question is about Java, someLongExpression.getAThing() returns the same type as somethingElse is. –  Izkata Mar 30 '12 at 16:13
    
What keeps the ternary operator from having side effects, e.g. condition ? function_with_side_effects() : other_function_with_side_effects(); ? –  Wayne Conrad May 7 '12 at 3:57
    
You are correct the compiler does not stop it in most languages, but in some there is forced command / query separation. You can enforce this you self, and thus benefit from it and the ternary operator. Some lint tools can also be used. –  richard May 9 '12 at 9:25
add comment

Lets break this down into simple questions...

Q: Why was the ternary operator introduced?
A: To provide developers with an easy way to express simple if/else statements on a single line.

Q: When should I use the ternary operator?
A: It should be used when you have a simple if/else statement that makes more sense expressed in a single line.

Q: When should the ternary operator not be used?
A: If you plan on writing the statement over multiple lines just use an if/else statement since you are in essence defeating the whole point of the ternary operator. It should also be avoided if you plan to nest multiple ternary operators together since it was never intended to replace if/elseif statements. (FYI: to me this is a cardinal sin!)

I know a lot of the posts in this thread disagree with my statements but I fell that following my advice will produce the most readable and easy to follow code.

share
1  
–1. The talk about “single line“ is a red herring. Like I commented on Michael’s answer, the conditional operator is about expressions, not lines. A conditional expression is just that: an expression, whereas an if is a statement and should be used only as such, not when an expression is more appropriate. Same for nesting. Look at George Mauer’s answer and tell me with a straight face that this is not more readable than a chained if … else with lots of assignments rather than just a single one. –  Konrad Rudolph May 7 '12 at 16:32
    
I still disagree. The if/else statement is considered a BLOCK statement. The ternary operator is, well... an operator. It wasn't intended to create or manage a block of code. –  Chris Frazier May 14 '12 at 19:32
    
Right you are. A block of code isn’t involved. –  Konrad Rudolph May 14 '12 at 19:49
add comment

protected by Jamal Dec 20 '13 at 6:41

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

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