I’ve been using FxCop to help find problems in the code I work on, and it sent me this gem:
“‘value’, a parameter, is cast to type ‘string’ multiple times in method ‘RegexAttributeFilter.Accepts(object)’. Cache the result of the ‘as’ operator or direct cast in order to eliminate the redundant castclass instruction.”
Looking at the code, I found this:
//get the string to compare against
if ( value == null )
strValue = string.Empty;
else if ( value is string )
strValue = (string) value;
strValue = value.ToString();
return this.regex.IsMatch( strValue );
The double cast happens because the is operator is used (no pun intended) to see if a value is of a certain type. To do that, it actually has to do an internal cast to that type (and see if it works). If it works, the boolean logic results in ‘true’ and the if loop is executed… Where we do *another* cast.
Another option is to use the as operator:
if(item != null)
Yet another way to fix the issue is to get rid of all that code altogether. Reading through the code; I didn’t immediately see why we were doing all this, so I opened up trusty LinqPad to see if maybe I was missing something with C# casts. Sure enough, a simple ToString() would work for most code, and when the passed object is null, we have our friend, the null–coalescing operator (also known as the operator I can never spell correctly, or ‘??’ for short).
With the ?? operator, the above code becomes:
return this.regex.IsMatch( (value ?? String.Empty).ToString() );
thereby reducing 15 lines of code to 1 compact and readable line of code, and getting rid of the needless double cast at the same time!
Bonus: The ?? operator was introduced in C# 2.0. In C# 1.0, no such being existed, which is why (hopefully only in legacy code) you’ll see something like above.
Caveat: It would rarely be necessary to change working code unless there’s a clear performance gain; in this case I was already in the code to make a change, so it made sense. Don’t go around mucking with working code and if you do, don’t blame me.