I have a method coded as following:
public int
ConvertDirect(string input)
{
int result;int.TryParse(input, out result);
return result;
}
And when I ran the code analysis I got the following warning:
CA1806 Do not ignore method results
'Class1.ConvertDirect(string)' calls 'int.TryParse(string, out int)' but does not explicitly check whether the conversion succeeded. Either use the return value in a conditional statement or verify that the call site expects that the out argument will be set to the default value when the conversion fails.
I then did a google search on int.TryParse and found that when the conversion failed, zero is output to the out parameter. And this is what I want it to do. So technically speaking, the code block at the question is doing what I want it to do. There is nothing wrong about it.
So I went to suppress it. By doing so I added an attribute to the method. The method become the following.
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1806:DoNotIgnoreMethodResults",
MessageId = "System.Int32.TryParse(System.String,System.Int32@)")]
public int ConvertDirect(string
input)
{int result;
int.TryParse(input, out result);
return result;
}
I felt uncomfortable about the suppression I just did. The fact is that by doing what I am doing here, if I make another call to another function and ignore the result. Even if it could produce unintended result, it would be also suppressed as the suppression is done in the member level. But I also do not want to write more code. With that I was settled with the following code block
public int
Convert(string input)
{int result;
return (int.TryParse(input, out result) ? result : 0);
}
With I ran the code metrics, I get 3 lines for the original version and I got 2 lines for the modified version of the method. On the other side, I got 2 Cyclomatric Complexity for the modified version and only got 1 Cyclomatric Complexity for the original version of the method. But on the maintainability aspect, the modified version offers 84% in maintainability index while the original version of the method offers 81% in in maintainability index. Considering all aspects mentioned above, I am in favor of the modified version. How about you?
public int
Convert(string input)
{
int result;return (int.TryParse(input, out result) ? result : 0);
}
Another option is to provide a extension method just do the conversation. but as the input parameter is of string type, the extension method must be made on string type. as such, the name of the method need to indicate the target type it try to parse. (TryParseToInt) this raise another naming warning. "CA1720: Identifiers should not contain type names"
IMHO, the best solution would be Microsoft add a static method in int class, TryParseWithDefault or it changes the implementation of Parse so that it does not throw exception when parsing failed, instead it provide default value as output.
