Suppressing Static Code Analysis Warning CA1806 for TryParse calls

Question

I was wondering what people's thoughts were on the CA1806 (DoNotIgnoreMethodResults) Static Code Analysis warning when using FxCop.

I have several cases where I use Int32.TryParse to pull in internal configuration information that was saved in a file. I end up with a lot of code that looks like:

Int32.TryParse(someString, NumberStyles.Integer, CultureInfo.InvariantCulture, out intResult);

MSDN says the default result of intResult is zero if something fails, which is exactly what I want.

Unfortunately, this code will trigger CA1806 when performing static code analysis. It seems like a lot of redundant/useless code to fix the errors with something like the following:

bool success = Int32.TryParse(someString, NumberStyles.Integer, CultureInfo.InvariantCulture, out intResult);
if (!success)
{
 intResult= 0;
}

Should I suppress this message or bite the bullet and add all this redundant error checking? Or maybe someone has a better idea for handling a case like this?

Thanks!

Answer 1

Why not refactor your TryParses into a single function with the behavior you're after?:

static int ParseOrDefault(string someStr)
{
    int result = 0;
    if(int.TryParse(someStr, out result))
    {
        return result;
    }
    return 0;
}

That way you avoid the annoying warning and ditch the redundant code. A separate function makes your expectations explicit and leaves no room for confusion.

Answer 2

Bite the bullet, or at least add comments. How does Johnny the Homicidal coder know that Int32.TryParse will provide a 0 output without looking at external documentation? Similarly, how does he know that 0 is what you WANT the default to be? The later implementation is specific in its purpose, so I have to agree with FxCop here.

Remember, Johnny knows where you live.

With that being said, I OFTEN suppress style cop, (largely because I use the async CTP which doesn't play nice together yet). Style guidelines are just that, guidelines. Use your head, but always err on the side of clarity if performance is a non-issue.

Keep in mind that behavior like this could be changed in the future, do you always want to have 0 to be the default? Could this change? Even if you refactor into a ParseOrDefault helper method, you should probably account for reasonable change of defaults during product lifetime.

Answer 3

Suppress away!

FxCop/Code Analysis is only really guidelines. It can help improve parts of your code, especially if it's being distributed to other developers for use, but at the end of the day, it's only a guideline and you can code however you like.