Today, I’ll do another installment of the CodeIt.Right Rules Explained series.? This is post number eight in this series.? As always, I’ll start off by citing my two personal rules about static analysis, along with an explanation.
- Never implement a suggested fix without knowing what makes it a fix.
- Never ignore a suggested fix without understanding what makes it a fix.
This might seem glib.? After all, I could just as easily say, “Learn the reasoning behind all suggested fixes.”? But I say it the way I do to highlight the decision you face when confronted with static analysis warnings.? In all cases, you must actively choose to ignore the feedback or to address it.? And for both options, you need to understand the logic behind the suggestion from the warning.
In that spirit, I’m going to offer up explanations for three more CodeIt.Right rules today.
Avoid single line if statements
First up, let’s look at something conceptually easy to understand. ?This rule falls under the category of “coding style” within the CodeIt.Right rules categories. ?I talk about this as easy to understand because I can show you the issue with a single line of code:
if (x < y) x = y;
This draws from the Microsoft .NET Framework guidelines, recommending that you not put both parts of the “if” condition on a single line. ?But in CodeIt.Right’s explanation and options for correction, you can get some additional context.
Packing your code onto a single line this way creates two problems, practically speaking. ?First, it makes things difficult for you when setting breakpoints in the debugger. ?The debugger paradigm assumes a statement per line of code. ?Secondly, it makes life complex should you want to attach a trailing comment to the line of code. ?Neither of these things represent the end of the world, but then again, neither does adding a newline. ?And when you add newline, you’ll find yourself writing code that looks a lot more the way all of your fellow developers expect it to look. ?This, in turn, helps you not run afoul of the principle of least astonishment with maintenance programmers.
Before moving on, I’ll mention one last thing. ?You will see some debate on this matter. ?And while CodeIt.Right does take a position on putting two statements on a line, it does not take a position on adding or omitting curly braces for single line “if” statements. ?That one you’ll have to duke out with your teammates in your coding standards documents.
Enum underlying type should be System.Int32 or one of the CLS-compliant integral types
Now that we’ve had a chance to warm up a bit, let’s move on to something a little more involved. ?This particular rule might strike you as a little arcane, and you might not have realized that violating it was even possible. ?But violate it you can. ?Let’s take a look.
public enum Suit : sbyte
{
Clubs = 1,
Diamonds = 2,
Hearts = 3,
Spades = 4
}
A lot of you reading probably just declare enums without specifying the data type for the constants. ?In other words, you’d write this enum, omitting the colon and the data type for the enum’s constant values. ?But, you?can specify the data type. ?And you can specify a non-CLS compliant integral type.
CodeIt.Right draws from FX Cop’s CA1028?to make this recommendation. ?The idea here speaks to CLS-compliance in general. ?If you write code specific to your language (in this case, C#), consumers of your code using other .NET framework languages will hit incompatibilities when you don’t make your code CLS-compliant.
The fix here is a simple one. ?Just use a CLS-compliant type, guaranteed to work across all framework languages. ?I would also add that you should just use the default underlying type, unless you have a pretty compelling reason to need a different one.
Do not re-throw exceptions explicitly
For this one, I’ll initially let the code do the talking instead of trying to explain it abstractly.
public double Divide(int x, int y)
{
try
{
return x/y;
}
catch(DivideByZeroException dbze)
{
throw dbze;
}
}
You’ll see this CodeIt.Right violation when you write the above code. ?It’s objecting to you catching the exception and then using the “throw” keyword to throw it — an explicit re-throwing. ?And like the last rule, this one draws from something you’ll find in FxCop. ?This is a warning you almost certainly don’t want to ignore.
Thrown exceptions carry information about the call stack, starting from where they’re thrown and up to where they’re handled. ?You can probably relate to this from your debugging travels. ?Think about debugging a thrown exception and looking at the window that comes up, offering you exception details. ?One of those details lets you see the stack trace, examining how you came to the code generating the exception. ?This stack trace is so important that you’ll see it in ASP applications inside the “yellow screen of death,” in error messages that pop up from running applications, and in its own docked Visual Studio window. ?To say this matters to troubleshooting your code is an understatement.
And when you re-throw exceptions explicitly, you severely undermine this capability. ?When you do this, you turn the throw statement into the new beginning of the stack trace for a new exception. ?In the code above, the new origin of the exception will not be the division by zero itself but rather my “throw dbze” line. ?You’ll no longer be able to see the actual origin of the problem when troubleshooting.
While that’s no big deal here, it matters elsewhere. ?Imagine that you’re trapping an exception in third party code or that the exception you’ve caught came from way down in the call stack. ?Your exception handling will go from valuable troubleshooting tool to active hindrance. ?You can easily fix this by using the “throw” keyword by itself, without specifying the exception.
Until Next Time
For this post in the series, I tried to offer an eclectic mix, as always. ?We looked at a relatively cosmetic “coding style” concern. ?Then we looked at a problem that will make it difficult for other framework languages to consume your code. ?And finally, we took a look at an exception handling issue that threatens to make your life miserable while troubleshooting.
Hopefully, with each of these posts you get some framework/language tidbits, and you see the value in leveraging an automated review tool to supplement your own checking.
Learn more how CodeIt.Right can help you automate code reviews and improve the quality of your code.
