Nullable boolean value with property

Question

Our Sonar code analysis server tells me

Boolean literals should not be redundant. Redundant Boolean literals should be removed from expressions to improve readability. Tag: Clumsy

and categorizes it as a minor bug.

Is this an error in the associated Sonar/FXCop C# rule (maybe it cannot handle nullable types?!) or is this really clumsy?

Note: createMissing is a value which comes from a datastore (Edit: in this implementation it is parsed from a XML document) where null is a valid value (because the attribute is not mandatory) and in my business logic null means equals false).

[XmlIgnore]
private bool? createMissing = null;
[XmlAttribute("createMissing")]
public bool CreateMissing
{
 get
 {
 return createMissing.HasValue ? createMissing.Value : false;
 }
 set
 {
 createMissing = value;
 }
}
public bool ShouldSerializeCreateMissing()
{
 return createMissing.HasValue;
}

• RobH: Nullable<T> has a method called GetValueOrDefault which is the equivalent of your code as false is default(bool).
• 200_success: This question lacks context. Could you show us where you load the data from the datastore, so that we can better advise you?
• seveves: I've added the missing context (meta: but was this really worth a downvote?! :D)

Answer 1

I don't know about redundant, but this:

return createMissing.HasValue ? createMissing.Value : false;

is equivalent to this:

return createMissing ?? false;

This doesn't look like there's any redundancy to me.

• seveves: Oh yes thats better. ?? operator is new in C# 6 right?
• RobH: @PhilipC - no it's not. The Null Coalescing Operator is ancient. The Null Propagation Operator is the new one.
• Philip C: @RobH You're quite right - I was getting it mixed up with the Null Propagation Operator. The Null Coalescing Operator was introduced alongside nullable types in C# 2. I'll get rid of my earlier comment to remove the incorrect information.

Answer 2

Although Philip C has provided one option, I'd just like to show a couple of others for completeness.

My suggestion from the comments:

public bool CreateMissing
{
 get
 {
 return createMissing.GetValueOrDefault();
 }
 set
 {
 createMissing = value;
 }
}

I personally think this makes the most sense.

Another option would simply be:

return createMissing.HasValue && createMissing.Value;

Answer 3

Boolean literals should not be redundant

a redundant statement is something that doesn't add anything. Something like:

if(booleanVariable == true)

or

numericValue + 0

or

numericValue * 1

you get the idea.

In your case, you use the ternary operator:

condition ? value : false;

Notice that condition is evaluated to a boolean value already.

The problem is that the usual solution of simply omitting the redundant stuff doesn't work in your situation. In the above examples, simply remove ==true, + 0 and * 1. In your case there's no such obvious thing to remove.

One could be tempted to simply return the condition part like so:

get
{
 return createMissing;
}

But that doesn't work because bool? cannot be converted to bool implicitly. So what now?

You have to make a decision on what should happen when the value is null. In a way, the ternary operator does this, but (and this is why Sonar tags this as clumsy) in a really clumsy way.

Try something like:

get
{
 return createMissing == true;
}

Wait a second, that's one of the examples of redundant code this answer started with! Wtf?

No, it's not. The difference is subtle, but important: this is not a boolean variable. It's a bool?. It has 3 states, hence checking for equality against true is like checking numericValue == 5, which is perfectly valid.

bool? is like a numeric value of a 2 bit system, except that one of the 4 possible values is not available whatsoever.

Not applicable to your situation, but if you wanted to evaluate both null and true to true, try this code:

get
{
 return createMissing != false;
}