CODEWITHSUNDEEP
c#equals
deafsheep
deafsheep

Reputation: 789

Using .Equals on a constant to avoid (save) null check

So here is standard code for IValueConverter

{
 if (value == null)
  return null;

 if (value.Equals(true))
  return Colors.Red;

 return null;
}

And another way:

{
 if (true.Equals(value))
  return Colors.Red;

 return null;
}

So, by using true.Equals() we are saving a null check. What are general approach and best practices in regards to using true.Equals() or "Hello".Equals() type checks?

P. S. My question is: what is your/general opinion on this: bad/hacky or ok/nice?

Upvotes: 1

Views: 189

Answers (4)

Martin Liversage
Martin Liversage

Reputation: 106856

You can avoid NullReferenceException and "funny" calls like true.Equals(value) by using the static Object.Equals method. Because the method is implement by Object it is available in all methods and it will perform the necessary null checks for you:

return Equals(value, true) ? (Object) Color.Red : null;

or if you dislike the ternary operator:

if (Equals(value, true))
  return Color.Red;
return null;

Upvotes: 0

Valentin Kuzub
Valentin Kuzub

Reputation: 12093

First of all you should be probably using base generic class for converters, which has a virtual strongly typed method, in which case you aren't dealing with object, but instead with a bool inside your converter code.

If we are stuck with object value I am advocate of readable and maintanable code and short functions which replace ifs which can become obscure in future.

So id be using smth like

{
 if(IsValueBoolAndTrue(value){
    return ErrorColor;
 }
 else {
    return DefaultColor;
 }
}

Color.Red and null are constants which make their intent unclear too. Also having those 2 constants defined can help with converting back in second method of IValueConverter

Although method IsValueBoolAndTrue technically does same thing as true.Equals(value) does (inside itself it can call this actually), having a special method will help person, who looks at this in future, not to make a "refactoring" which will break code, because he would look at true.Equals(value) with no comments and think that its actually not smth that is suitable or will think that its inelegant code and refactor it, without leaving its funcionality the same.

Upvotes: 0

Stjepan Rajko
Stjepan Rajko

Reputation: 886

I have a strong preference for shorter code (and by that I mean less tokens, not using undecipherable short identifiers) - it is often more readable and understandable, less bug prone, and gives you less code to maintain.

So leaving the null check to .Equals by using true.Equals is a clear win in my book.

In this particular example, you could even remove the duplicate return and do

return true.Equals(value) ? Colors.RedColors.Red : null;

but some might find that less readable than the if statement.

Upvotes: 0

Kyle W
Kyle W

Reputation: 3752

The .Equals method generally checks for null anyway, but if you're trying to optimize your code by removing a single null check, you're really missing the mark. This is not going to make a difference in virtually any application.

Upvotes: 1

Related Questions