Search code examples
c#code-analysis

Explanation of 'CA1800: Do not cast unnecessarily' needed


I'm a little bit confused about the Code Analysis warning in Visual Studio. The following code raises it:

static void Main(string[] args)
{
    var something = new Something();
    object expression = "Any value";
    for (int i = 0; i < 2; i++)
    {
        switch (i)
        {
            case 0:
                something.Help = expression as string;
                break;

            case 1:
                something.Description = expression as string;
                break;
        }
    }
}

class Something
{
    public string Help;
    public string Description;
}

The documentation of CA1800 says:

... Duplicate casts decrease performance, especially when the casts are performed in compact iteration statements. For explicit duplicate cast operations, store the result of the cast in a local variable and use the local variable instead of the duplicate cast operations. ...

I can use and assign a local variable, yes. But the casts shown above won't be executed in one iteration. So, in my opinion, this can't be more expensive than using a local variable.

In my real method the variable expression can have many other types and the class Something properties of the other types. There's an other parameter, which replaces i and there's no for-loop.

Assume I create one local variable for any possible type of expression. Then many cast operations will be executed, whereas only one will be needed.

for (int i = 0; i < 2; i++)
{
    var type1 = expression as Type1;
    var type2 = expression as Type2;
    var type3 = expression as Type3;
    var type4 = expression as Type4;
    switch (i)
    {
        case 0:
            something.*** = type1;
            break;

        case 1:
            something.*** = type2;
            break;

        ...
    }
}

THAT is expensive...

Can somebody tell me the background of this warning? That does not make any sense to me.


Solution

  • The warning is indicating that you're casting multiple times because of the loop. This should resolve it:

    var something = new Something();
    object expression = "Any value";
    string expressionAsString = expression as string;
    for (int i = 0; i < 2; i++)
    {
        switch (i)
        {
            case 0:
                something.Help = expressionAsString;
                break;
    
            case 1:
                something.Description = expressionAsString;
                break;
        }
    }
    

    That said, as Servy mentions warnings are warnings - if your code is easier to understand with multiple casts and it doesn't significantly affect the performance (which can only be determined by trying it both ways and measuring the difference) then either ignore or suppress the warning. I've seen too much code that's ugly and/or difficult to understand just to avoid a warning that does not understand the context of the code.