Avoid type casts - Use the "as" operator and check for null

Note: I don't care about measuring performance on these 2 operations because we don't use them in a tight loop.
For me its all about readability and robustness of my code.

Look at these code samples doing some type casts

        public List GetJobRulesFromPhysicalDB(DataTable renewedRules)
        {
            if (renewedRules.Rows.Count > 0 && CurrentJob.JobRules.Count > 0)
            {
                foreach (DataRow row in renewedRules.Rows)
                {
                    foreach (JobRule item in CurrentJob.JobRules)
                    {
                        if ((Guid)row["RuleID"] == item.RuleId)
                        {
                            item.UpdateRule(CurrentJob.RuleRepository.GetNewRuleByRuleId(item.RuleId));
                            break;
                        }
                    }
                }
            }

            return CurrentJob.JobRules;
        }
Figure: 1 code sample with untyped datatables

 

  
        private void AMControlMouseLeftButtonUp(object sender, MouseButtonEventArgs e)
        {
            var auc = (AMUserControl)sender; 
            var aucSessionId = auc.myUserControl.Tag;
            // snip snip snip
   
        }
Figure: Event handler in Silverlight

I see this VERY often, so I had to blog about it.

The problem above is the high possibility to get an InvalidCastException. That happens when the object cannot be casted to the type defined in the brackets. We try to make our code 100% bullet proof so we should avoid this problem.

In Silverlight, if that InvalidCastException happens, you get a nice yellow Warning icon in the browser.
image

In a a WCF service your code continues until a maximum of failures in that application pool (check IIS Manager)
image
Figure: Maximum failures configuration in IIS Manager for an application pool

 

Use casts only if

  1. You know 100% that you get that type back (you never know.. the next dev comes in and uses your method, API differently..)
  2. You want to fail early, when something is wrong
  3. You want to perform user-defined conversions
    1. Might be handy for numbers, or if you have your own defined conversions

Otherwise just use the "as" operator and check for null
This will bite you, trust me :-)

Note:
In order to use the "as operator" your type has to be a reference type or a nullable type..

 

  
        private void AMControlMouseLeftButtonUp(object sender, MouseButtonEventArgs e)
        {
            var auc = sender as AMUserControl; 
            if (auc != null)
            {
               var aucSessionId = auc.myUserControl.Tag;
               // snip snip snip
            }   
        }

Figure: Event handler in Silverlight using the as operator

 

More details on "cast vs. as operator" on Eric's blog

2 comments:

Anonymous said...

"as" should be used only when the object might be a different type. Direct casting is good as you avoid 5 lines of code if you know the type should always be the expected done.

var x = y as RandomType;
if(x != null)
{
// do random stuff
}
else
{
throw new RandomException ("Who managed to squeeze this square object in this round hole?");
}

vs:
var x = (RandomType)y; // boom is y is not RandomType.

Peter Gfader said...

Hi Corneliu

#1
>> "as" should be used only when the object might be a different type.
How can you be sure that you get always the same type?
The next dev comes along and uses your class/method differently.

If you provide an API then an exception might make sense, but for an internal class maybe not.
Why not using Code Contracts?

#2
Your code sample is a very bad sample for using the "as" operator...

----
I guess you agree with me on that one, because we said the same... ;-)

From my post
"Use casts only if you know 100% that you get that type."

Post a Comment

Latest Posts

Popular Posts