I might have mentioned before that I really like ReSharper. In my opinion if you’re writing C# code and not using ReSharper, you’re handicapping yourself. And probably committing professional malpractice. Next to Visual Studio, it’s the most useful development tool I have.
Today’s discovery is a case in point.
I was reviewing some code today when I ran across a latent bug and a definite bug, both because ReSharper had flagged the lines involved.
string paramName;
var paramNameMatch = Regex.Match(message, @"Parameter name\:\s+(?.+)");
if (paramNameMatch != null) { paramName = paramNameMatch.Groups["ParamName"].Value; }
else { paramName = string.Empty; }
string actualValue;
var actualValueMatch = Regex.Match(message, @"Actual value was (?.+)\.");
if (paramNameMatch != null) { actualValue = paramNameMatch.Groups["ActualValue"].Value; }
else { actualValue = string.Empty; }
Don’t worry too much about what the regular expressions are doing or why.
The code looks reasonable, right? Try to match a regular expression and assign a value based on whether the match was successful. Except that’s not what the code does. Let’s take a look at the first part. I’ve reformatted it for readability.
string paramName;
var paramNameMatch = Regex.Match(message, @"Parameter name\:\s+(?.+)");
if (paramNameMatch != null)
{
paramName = paramNameMatch.Groups["ParamName"].Value;
}
else
{
paramName = string.Empty;
}
The primary problem here is that Regex.Match
never returns null!. Documentation says that if no match was found, the return value is a Match
object that is equal to Match.Empty
. In any case, the else clause will never be executed.
The code passed the programmer’s test only because trying to access a group that doesn’t exist returns an empty string. As documentation says:
If groupname is not the name of a capturing group in the collection, or if groupname is the name of a capturing group that has not been matched in the input string, the method returns a Group object whose Group.Success property is false and whose Group.Value property is String.Empty.
The code works by accident, which to me means that it’s a latent bug. Had we wanted to return something other than string.Empty in the else clause, this code would have been in error.
In this case, ReSharper told me that in this line:
if (paramNameMatch != null)
The expression is always true. In other words, paramNameMatch will never be null.
The proper way to determine if a regular expression match was successful is to check the value of the Match.Success property. The above code is more correctly written as:
string paramName;
var paramNameMatch = Regex.Match(message, @"Parameter name\:\s+(?.+)");
if (paramNameMatch.Success) // will be false if no match was made.
{
paramName = paramNameMatch.Groups["ParamName"].Value;
}
else
{
paramName = string.Empty;
}
I find that overly verbose. Situations like this just beg for the ternary operator:
var paramNameMatch = Regex.Match(message, @"Parameter name\:\s+(?.+)");
string paramName = (paramNameMatch.Success) ? paramNameMatch.Groups["ParamName"].Value : string.Empty;
ReSharper also notified me of a real bug in this code:
string actualValue;
var actualValueMatch = Regex.Match(message, @"Actual value was (?.+)\.");
if (paramNameMatch != null) { actualValue = paramNameMatch.Groups["ActualValue"].Value; }
else { actualValue = string.Empty; }
ReSharper’s warning was that the variable actualValueMatch is assigned a value that is never used. Look closely: the second line assigns a value to actualValueMatch, but the next line checks the value of paramNameMatch. This code could not have worked. The programmer obviously didn’t test it.
Had the programmer who wrote this been using ReSharper and paying attention to what it was telling him, neither of these bugs would have been checked into source control. Most likely, the programmer would have seen ReSharper’s warnings immediately after he wrote the bugs, and would have fixed them before he even tried to compile the program.
Some people complain that ReSharper is expensive. I find that a ridiculous argument. A ReSharper subscription costs $239 per year. Or, if you don’t want upgrades you can buy a license for $300 and keep it for a few years before upgrading. If you’re writing code for pay, $300 is something like four or six hours of work. ReSharper saves me that much time every month! It helps me avoid mistakes when I’m writing code, and identifies trouble spots when I’m reviewing code. It also helps me with refactoring when I’m working on older code, and helps me navigate this very large solution I’ve inherited. I could do my work without it, but I’d rather not. It’d be like riding a horse to work: I’d get there eventually, but it’d be uncomfortable and would take too long. Buy the best tools you can afford. And if you’re making a living writing C# code, you can afford ReSharper.