views:

345

answers:

8

As a mostly self-taught programmer, I have never really had anyone explain why certain things should or should not be used. One example (which I picked up years ago and use quite often) is the alternative control structure syntax:

x = (y == true) ? "foo" : "bar";

I personally find this syntax easy to follow, especially for short, concise code, but I don't see it get a lot of use "in the wild" so I'd like to know if it's bad practice to use this over a more traditional if ... else structure?

Thanks in advance.

+1  A: 

The syntax you're referring to is often called the ternary conditional operator and it's semantically equal to an if/else clause and it performs just the same.

The main objection towards overusing theese constructs is that it can create somewhat messy source code that may not be as readable as using the if/else way. This is especially true for people who don't know about that particular syntax. It's far more likely 100% safe to assume that people understand if/else.

Markus Olsson
No it isn't - it is the "cnoditional" operator, that happens to be ternary (3 parts) rather than binary (2 parts)
Marc Gravell
typo: conditional
Marc Gravell
Sorry, of course you're right.. I was obviously rushing my response a bit
Markus Olsson
A: 

The conditional operator should be pretty familiar to most devs. Actually, I would use simply:

x = y ? "foo" : "bar";

which I find pretty easy to follow. The ==true / ==false seems (IMO) overkill for bools. A more common "olde world" usage is:

if(12345 == someVar) {...} // etc

This "const == variable" is to avoid the issue of "=" vs "==", but since C# doesn't treat numbers as booleans, it is very rare that this is an issue, and it is more common (in C#) to see the clearer:

if(someVar == 12345) {...} // etc

Since if you miss the extra = it usually won't compile.

Marc Gravell
+1  A: 

This is known as a ternary expression. If the the intent can be expressened clearly and concisely with a ternary operator then I do so. I usually draw the line and having compound expressions (more than 1 combined with && or ||)

Good:

var x = (someVar > 42 ) ? ThisFunction() : ThisOtherFunction();

Bad:

var x = (someVar > 42 && anotherVariable.IsSafeForConsumption() && IsNotProcessing ) ? ThisFunction() : ThisOTherFunction();
JaredPar
+13  A: 

As a fellow mostly self-taught programmer, I can only give you my opinion, without any special authority behind it.

I find that the ternary conditional operator (?:) is perfectly acceptable when you're using it for an expression with no side-effects. As soon as you start using it to represent a decision to do something vs. do something else, you're (in my opinion) abusing it.

In short, control constructs are for structuring the flow of the code and operators are for expressions. As long as one keeps things that way, the code remains readable.

Vojislav Stojkovic
Couldn't have been said better, when in doubt, don't use ternary, code needs to be readable first and foremost, functional ternary is anti-readability.
TravisO
+2  A: 

The conditional operator (sometimes referred to as the ternary operator, but that's not technically correct, see Marc's comments) isn't really an "alternative control structure", there's a few important differences:

  • The conditional operator is an operator and provides a value, it's used in an expression.
  • If/else is a control structure, it (of course) returns no value on its own.
  • The alternatives in a conditional operator must also be expressions.
  • The alternatives in an if-else structure can be an arbitrary number of statements.

Of course there are similarities as well:

  • Each evaluates a conditional expression.
  • Only the appropriate alternative (expression or block) is evaluated based on the boolean value of the conditional.
  • Both can be chained together with others of its kind to form longer "switch-like" statements.

In general, if you are evaluating alternative expressions for the side-effects (that is to do work instead of to return a value), then using a conditional operator can be more confusing to maintainers than using an if-else structure.

If all of the following are true, use a conditional:

  • Your conditional and alternative statements are easy to understand.
  • None of the expressions has any side-effects.
  • You won't need additional behavior (i.e. side-effects or additional statements) on the alternatives in the future.

Your example as given is a good use of a ternary conditional.

When in doubt, use an if-else. Beware of thinking of the conditional as another kind of if-else and trying to cram things that don't belong into it. That's the kind of thing that (especially when found in legacy code) turns people off to the conditional operator altogether.

Adam Bellaire
See my comments to Markus Olsson
Marc Gravell
@Marc: True, that's more correct. In Perl (the language I use most), the conditional operator is the only ternary operator as well, so using either to refer to it is unambiguous. I'll update my response.
Adam Bellaire
A: 

For me this issue is one of readability and maintainability in a multi-programmer environment.

If someone else has to read the code, or if you're going to come back to it later, you need to make it readable.

Unless you keep the cases simple, it's easy for code that uses the ternary operators to become obfuscated and unreadable.

  • Keep it simple
  • If it becomes even slightly hard to read, refactor into if { } else { }
A: 

Except for when it's used in the most basic scenarios, I'll say avoid it. If 2 constructs are compiled to the same exact MSIL, why use the one that's difficult to understand?

Tundey
A: 

In addition, since the tertiary operator is essentially a statement masquerading as an expression you can take this :

x = (y == true) ? "foo" : "bar";
string instructions = "Please can somebody go and get me a " + x;

and convert it to this :

string instructions = "Please can somebody go and get me a " + 
                      ((y == true) ? "foo" : "bar");

Please note that the additional parenthesis are required. Try to compile it yourself if you cant see why!

This is a very useful feature to have and I use it all the time. Just be cautious as others have mentioned not to write expressions with side effects, or overly complex expressions that reduce readability.

In a slightly unrelated note please for god's sake dont ever write code like this (I've seen it many times):

if (y == true) {
   weWantAFoo = true;
}
else {
   weWantAFoo = false;
}

Instead you should do this :

bool weWantAFoo = (y==true);

This is just another example of using a conditional statement somewhere other than in an if statement.

Simon_Weaver