How to Rewrite Complicated Lines of C++ Code (Nested Ternary Operator)

How to rewrite complicated lines of C++ code (nested ternary operator)

The statement as written could be improved if rewritten as follows....

good = m_seedsfilter==0 ? true :
       m_seedsfilter==1 ? newClusters(Sp) :
                          newSeed(Sp);

...but in general you should just become familar with the ternary statement. There is nothing inherently evil about either the code as originally posted, or xanatos' version, or mine. Ternary statements are not evil, they're a basic feature of the language, and once you become familiar with them, you'll note that code like this (as I've posted, not as written in your original post) is actually easier to read than a chain of if-else statements. For example, in this code, you can simply read this statement as follows: "Variable good equals... if m_seedsfilter==0, then true, otherwise, if m_seedsfilter==1, then newClusters(Sp), otherwise, newSeed(Sp)."

Note that my version above avoids three separate assignments to the variable good, and makes it clear that the goal of the statement is to assign a value to good. Also, written this way, it makes it clear that essentially this is a "switch-case" construct, with the default case being newSeed(Sp).

It should probably be noted that my rewrite above is good as long as operator!() for the type of m_seedsfilter is not overridden. If it is, then you'd have to use this to preserve the behavior of your original version...

good = !m_seedsfilter   ? true :
       m_seedsfilter==1 ? newClusters(Sp) :
                          newSeed(Sp);

...and as xanatos' comment below proves, if your newClusters() and newSeed() methods return different types than each other, and if those types are written with carefully-crafted meaningless conversion operators, then you'll have to revert to the original code itself (though hopefully formatted better, as in xanatos' own post) in order to faithfully duplicate the exact same behavior as your original post. But in the real world, nobody's going to do that, so my first version above should be fine.

UPDATE, two and a half years after the original post/answer:
It's interesting that @TimothyShields and I keep getting upvotes on this from time to time, and Tim's answer seems to consistently track at about 50% of this answer's upvotes, more or less (43 vs 22 as of this update).

I thought I'd add another example of the clarity that the ternary statement can add when used judiciously. The examples below are short snippets from code I was writing for a callstack usage analyzer (a tool that analyzes compiled C code, but the tool itself is written in C#). All three variants accomplish exactly the same objective, at least as far as externally-visible effects go.

1. WITHOUT the ternary operator:

Console.Write(new string(' ', backtraceIndentLevel) + fcnName);
if (fcnInfo.callDepth == 0)
{
   Console.Write(" (leaf function");
}
else if (fcnInfo.callDepth == 1)
{
   Console.Write(" (calls 1 level deeper");
}
else
{
   Console.Write(" (calls " + fcnInfo.callDepth + " levels deeper");
}
Console.WriteLine(", max " + (newStackDepth + fcnInfo.callStackUsage) + " bytes)");

2. WITH the ternary operator, separate calls to Console.Write():

Console.Write(new string(' ', backtraceIndentLevel) + fcnName);
Console.Write((fcnInfo.callDepth == 0) ? (" (leaf function") :
              (fcnInfo.callDepth == 1) ? (" (calls 1 level deeper") :
                                         (" (calls " + fcnInfo.callDepth + " levels deeper"));
Console.WriteLine(", max " + (newStackDepth + fcnInfo.callStackUsage) + " bytes)");

3. WITH the ternary operator, collapsed to a single call to Console.Write():

Console.WriteLine(
   new string(' ', backtraceIndentLevel) + fcnName +
   ((fcnInfo.callDepth == 0) ? (" (leaf function") :
    (fcnInfo.callDepth == 1) ? (" (calls 1 level deeper") :
                               (" (calls " + fcnInfo.callDepth + " levels deeper")) +
   ", max " + (newStackDepth + fcnInfo.callStackUsage) + " bytes)");

One might argue that the difference between the three examples above is trivial, and since it's trivial, why not prefer the simpler (first) variant? It's all about being concise; expressing an idea in "as few words as possible" so that the listener/reader can still remember the beginning of the idea by the time I get to the end of the idea. When I speak to small children, I use simple, short sentences, and as a result it takes more sentences to express an idea. When I speak with adults fluent in my language, I use longer, more complex sentences that express ideas more concisely.

These examples print a single line of text to the standard output. While the operation they perform is simple, it should be easy to imagine them as a subset of a larger sequence. The more concisely I can clearly express subsets of that sequence, the more of that sequence can fit on my editor's screen. Of course I can easily take that effort too far, making it more difficult to comprehend; the goal is to find the "sweet spot" between being comprehensible and concise. I argue that once a programmer becomes familiar with the ternary statement, comprehending code that uses them becomes easier than comprehending code that does not (e.g. 2 and 3 above, vs. 1 above).

The final reason experienced programmers should feel comfortable using ternary statements is to avoid creating unnecessary temporary variables when making method calls. As an example of that, I present a fourth variant of the above examples, with the logic condensed to a single call to Console.WriteLine(); the result is both less comprehensible and less concise:

4. WITHOUT the ternary operator, collapsed to a single call to Console.Write():

string tempStr;
if (fcnInfo.callDepth == 0)
{
   tempStr = " (leaf function";
}
else if (fcnInfo.callDepth == 1)
{
   tempStr = " (calls 1 level deeper";
}
else
{
   tempStr = " (calls " + fcnInfo.callDepth + " levels deeper";
}
Console.WriteLine(new string(' ', backtraceIndentLevel) + fcnName + tempStr +
                  ", max " + (newStackDepth + fcnInfo.callStackUsage) + " bytes)");

Before arguing that "condensing the logic to a single call to Console.WriteLine() is unnecessary," consider that this is merely an example: Imagine calls to some other method, one which takes multiple parameters, all of which require temporaries based on the state of other variables. You could create your own temporaries and make the method call with those temporaries, or you could use the ternary operator and let the compiler create its own (unnamed) temporaries. Again I argue that the ternary operator enables far more concise and comprehensible code than without. But for it to be comprehensible you'll have to drop any preconceived notions you have that the ternary operator is evil.

Alternative to nested ternary operator in JS

Your alternatives here are basically:

That if/else you don't want to do
A switch combined with if/else

I tried to come up with a reasonable lookup map option, but it got unreasonable fairly quickly.

I'd go for #1, it's not that big:

if (res.distance == 0) {
    word = 'a';
} else if (res.distance == 1 && res.difference > 3) {
    word = 'b';
} else if (res.distance == 2 && res.difference > 5 && String(res.key).length > 5) {
    word = 'c';
} else {
    word = 'd';
}

If all the braces and vertical size bother you, without them it's almost as concise as the conditional operator version:

if (res.distance == 0) word = 'a';
else if (res.distance == 1 && res.difference > 3) word = 'b';
else if (res.distance == 2 && res.difference > 5 && String(res.key).length > 5) word = 'c';
else word = 'd';

(I'm not advocating that, I never advocate leaving off braces or putting the statement following an if on the same line, but others have different style perspectives.)

#2 is, to my mind, more clunky but that's probably more a style comment than anything else:

word = 'd';
switch (res.distance) {
    case 0:
        word = 'a';
        break;
    case 1:
        if (res.difference > 3) {
            word = 'b';
        }
        break;
    case 2:
        if (res.difference > 5 && String(res.key).length > 5) {
            word = 'c';
        }
        break;
}

And finally, and I am not advocating this, you can take advantage of the fact that JavaScript's switch is unusual in the B-syntax language family: The case statements can be expressions, and are matched against the switch value in source code order:

switch (true) {
    case res.distance == 0:
        word = 'a';
        break;
    case res.distance == 1 && res.difference > 3:
        word = 'b';
        break;
    case res.distance == 2 && res.difference > 5 && String(res.key).length > 5:
        word = 'c';
        break;
    default:
        word = 'd';
        break;
}

How ugly is that? :-)

Optimize ternary operator

That's just horrible code.

It's badly formatted. I don't see the hierarchy of the expression.
Even if it had good formatting, the expression would be way too complex to quickly parse with the human eye.
The intention is unclear. What's the purpose of those conditions?

So what can you do?

Use conditional statements (if).
Extract the sub-expressions, and store them in variables. Check this nice example from the refactoring catalog.
Use helper functions. If the logic is complex, use early returns. Nobody likes deep indentation.
Most importantly, give everything a meaningful name. The intention should be clear why something has to be calculated.

And just to be clear: There's nothing wrong with the ternary operator. If used judiously, it often produces code that's easier to digest. Avoid nesting them though. I occasionally use a second level if the code is crystal clear, and even then I use parentheses so my poor brain doesn't have to do extra cycles decyphering the operator precedence.

Care about the readers of your code.

Using nested ternary operators

Wrap it in parentheses:

$selectedTemplate = isset($_POST['selectedTemplate'])
                  ? $_POST['selectedTemplate']
                  : (
                       isset($_GET['selectedTemplate'])
                       ? $_GET['selectedTemplate']
                       : 0
                  );

Or even better, use a proper if/else statement (for maintainability):

$selectTemplate = 0;

if (isset($_POST['selectedTemplate'])) {
    $selectTemplate = $_POST['selectedTemplate'];
} elseif (isset($_GET['selectedTemplate'])) {
    $selectTemplate = $_GET['selectedTemplate'];
}

However, as others have pointed out: it would simply be easier for you to use $_REQUEST:

$selectedTemplate = isset($_REQUEST['selectedTemplate'])
                  ? $_REQUEST['selectedTemplate']
                  : 0;

A somewhat painful triple-nested ternary operator

I think you can have this to avoid the deep nesting:

var H

if(C == 0){
    H = null;
}
else if(V == r){
    H = (g - b) / C;
}
else if (V == g){
    H = (b - r) / C + 2;
}
else {
    H = (r - g) / C + 4;
}

To ternary or not to ternary?

Use it for simple expressions only:

int a = (b > 10) ? c : d;

Don't chain or nest ternary operators as it hard to read and confusing:

int a = b > 10 ? c < 20 ? 50 : 80 : e == 2 ? 4 : 8;

Moreover, when using ternary operator, consider formatting the code in a way that improves readability:

int a = (b > 10) ? some_value                 
                 : another_value;

Change from if else to ternary operator

It's not generally a good idea to use nested conditional operators, but it can be done:

printf("%s", a==b ? "equal" : (a > b ? "bigger" : "smaller"));

How to Rewrite Complicated Lines of C++ Code (Nested Ternary Operator)