Is it bad to coalesce the evaluator in a ternary expression? (C#)

Question

I've looked around a little and haven't found an equivalent question.
Is this bad coding practice? I can read it easily, but is it too cryptic for someone reading the code?

bool? testBool = null;  
string result;  
result = (testBool ?? false ? "Yes" : "No");

Edit: My apologies to everyone for some horrible code! Here is a working example..
I'm a little overwhelmed by the speed of the replies. I was going to delete this and do it right, but already had 4 replies!

Answer 1

EDIT: The original question used an int?. It's now been fixed.

That code won't even compile, so yes, I'd say it's too cryptic.

Typos aside, the fact that you posted it without easily spotting that there's a problem (you're trying to use a bool on the RHS of the ??, when the LHS is an int?) suggests it's not a good idea even when you get it right.

I'd need to see a real-world example, but I think I'd usually split this into one statement using the null coalescing operator, and then another using the conditional operator. Another option is to use the behaviour of nullable types in relation to operators... but again, that's reasonably obscure. (I've just had to remind myself exactly what that behaviour is!)

I like the null coalescing operator in general, but I think combining it with the conditional operator just makes it a little bit to obscure. I think I'd probably accept it in a case where there was a significant benefit to it being a single expression (e.g. for initialization where the alternative is introducing an extra method) but in general I'd prefer to split it into two statements.

EDIT: One alternative in this particular case is to just compare against "true" - which looks redundant, but isn't in the case of bool?:

result = (testBool == true) ? "Yes" : "No";

The brackets aren't necessary, of course, but add clarity IMO.

I think this is simpler: the result is only "Yes" if testBool is actually true; otherwise it's "No". If you wanted to "default" to Yes, you'd write:

result = (testBool == false) ? "No" : "Yes";

Answer 2

I'd add parens so clarify what's going on -- ie.

bool? testbool = null;
string result;
result = (testbool ?? false) ? "Yes" : "No";

With that, I'd be fine with it. Without it, I had to study it for a bit to figure out what was going on (if it even compiles -- I don't know the order of operations off the top of my head for ?: vs. ??)

Answer 3

Wrap it and I think it's fine.

string result = (testbool ?? false) ? "Yes" : "No";

Otherwise the intended order of operations, even if it works, is not obvious.

(Edit: Jonathan beat me by a pinch.)

Answer 4

Shouldn't it be

bool? testbool = null;

?

I find no need to use a nullable here. This makes me think twice, so I'd really code it as

bool testbool = false;
string result;
result = testbool ? "Yes" : "No";

I don't know if your example was incomplete, but I think initializing the variable to the default value is saner than using a nullable and then the ?? operator to set the value after the declaration.

Answer 5

It's a bit on the cryptic side, it's not even obvious if the expression is evaluated as:

string result = (testbool ?? false) ? "Yes" : "No";

or:

string result = testbool ?? (false ? "Yes" : "No");

You can use the GetValueOrDefault method of the nullable type instead of the ?? operator to make it more readable:

bool? testbool = null;
string result = (testbool.GetValueOrDefault(false) ? "Yes" : "No");