tags:

views:

41

answers:

1

I get the following error when running FXCop:

CA1800 : Microsoft.Performance : 'obj', a variable, is cast to type 'Job' multiple times in method 'ProductsController.Details(int, int)'. Cache the result of the 'as' operator or direct cast in order to eliminate the redundant castclass instruction

Code:

        object obj = repository.GetJobOrPlace(jobId);//Returns  (object) place or (object) product

        if (obj != null)
        {
            if (obj is Job)
            {
                Job j = (Job) obj;
                Debug.WriteLine(j.Title);
            }
            else if (obj is Place)
            {
                Place p = (Place) obj;
                Debug.WriteLine(p.Title);
            }
        }

What's wrong with this? I can only see one cast: Job j = (Job) obj.

+5  A: 

There's only one cast but there's also a test. So you can replace the first block with:

Job j = obj as Job;
if (j != null)
{
    Debug.WriteLine(j.Title);
}

That means the execution time test only needs to be performed once, instead of twice. It's a bit of a micro-optimisation - and in your case it would make the code a bit messier, as you'd need:

Job j = obj as Job;
if (j != null)
{
    Debug.WriteLine(j.Title);
}
else
{
    Place p = obj as Place;
    if (p != null)
    {
        Debug.WriteLine(p.Title);
    }
}

(Or declare and initialize p earlier, which wastes a test if obj is actually a Job...)

Jon Skeet
A minor detail: the first line should be "Job j = obj as Job;" With capitalized J in Job.
Anders Abel
Minor nitpick: "as job" should be "as Job" ;-)
Andy Shellam
Darn it, Anders beat me to it :P
Andy Shellam
That works. It's indeed uglier though.
Carra
@Andy/Anders: Fixed, thanks.
Jon Skeet