Legible or not: C# multiple ternary operators + Th

Question

Do you find the following C# code legible?

private bool CanExecuteAdd(string parameter) {
    return
        this.Script == null ? false
        : parameter == "Step" ? true
        : parameter == "Element" ? this.ElementSelectedInLibrary != null && this.SelectedStep != null
        : parameter == "Choice" ? this.SelectedElement != null
        : parameter == "Jump" ? this.SelectedStep != null
        : parameter == "Conditional jump" ? false
        : false.Throw("Unknown Add parameter {0} in XAML.".F(parameter));
}

where Throw is defined as:

public static T Throw<T>(this T ignored, string message) {
    throw new Exception(message);
}

I know that's not idiomatic C#. However, would you be able at understand it at first or second glance? Or did I stray too far?

Answer 1

I like the conditional operator, and use it a lot.

This example is a little confusing, because the nature of the operator is not clear from the layout and usage.

At the very least I like to make the choice and alternatives clear by using this formatting:

choice
  ? true-case
  : false-case

But if we apply that to your code it reveals the lack of clarity in using the construct this way:

return
    this.Script == null 
                ? false 
                : parameter == "Step" 
                    ? true
                    : parameter == "Element" 
                        ? this.ElementSelectedInLibrary != null && this.SelectedStep != null
                        : parameter == "Choice" 
                            ? this.SelectedElement != null
                            : parameter == "Jump" 
                                ? this.SelectedStep != null
                                : parameter == "Conditional jump" 
                                        ? false
                                        : false.Throw("Unknown Add parameter {0} in XAML.".F(parameter));

This feels to me like we're trying to use the conditional operator like a switch statement, where a switch statement, or better yet a design pattern like the Command Pattern would be much clearer.

Answer 2

Why not use a nullable type bool?

private bool? CanExecuteAdd(string parameter) {
return
    this.Script == null ? false
    : parameter == "Step" ? true
    : parameter == "Element" ? this.ElementSelectedInLibrary != null && this.SelectedStep != null
    : parameter == "Choice" ? this.SelectedElement != null
    : parameter == "Jump" ? this.SelectedStep != null
    : parameter == "Conditional jump" ? false
    : null;

}

Answer 3

Being non-idiomatic means that you're forcing the reader to spend time thinking about whether or not what they're reading means what they think it means.

So being legible doesn't buy the sophisticated (namely, suspicious) reader very much. This strikes me as a case of being clever for the sake of being clever.

Is there any reason not to use a switch or an else if construct here?

Answer 4

I've actually never seen the ternary operator pushed out so far. However, I understood where you were going. Also, I agree with Jon, I don't like the false.Throw part.

Answer 5

Convert your nested ternary into a switch. Never coerce one control structure into doing poorly or illegibly what a built-in structure will do perfectly, especially if there's no obvious benefit.

Answer 6

I vote for non-legible.

Although the syntax is correct, it's somewhat convoluted and since it's not, dare I say, "traditional", many developers will have to waste time trying to ensure they understand what they're reading. Not an ideal situation.

Readability is definitely one key ingredient to good coding, and I would say your sample isn't immediately readable for most devs.