It’s time for another post in the CodeIt.Right Rules Explained series.? I daresay it’s becoming a pretty predictable post series, with a pretty predictable cadence.? And that’s a good thing!? CodeIt.Right offers automated code review, and this series of blog posts gives you a detailed tour of the what and also the why of the code review rules.
As has become custom, I’ll start with two solid rules of thumb for static analysis.
- Never implement a suggested fix without knowing what makes it a fix.
- Never ignore a suggested fix without understanding what makes it a fix.
Every month, I explain that I don’t phrase it this way to play some sort of game.? After all, I could just declare that everyone should learn every suggested rule and be done with it.? But I phrase it this way to make it clear that you face a specific choice each and every time you encounter an analysis warning: fix it or ignore it.? And when you make that choice, you should make it for the right reasons.? “I don’t know what this means,” isn’t the right reason.
So let’s dive deep into understanding another three rules today.
Do not declare protected members in sealed types
The first rule instructs you not to declare anything protected in a type labeled sealed.? This CodeIt.Right rule reinforces the guidance offered by Microsoft’s code analysis, CA0147.
To understand this, consider the levels of visibility for declared elements.? These are called access modifiers, and they are as follows (in C#).
- public?— no restrictions on access whatsoever
- protected — only the containing class or types derived from the containing class can access
- internal — limits access to the assembly containing the element
- protected internal — since protected and internal are not mutually exclusive, this limits access to the current assembly and to current and derived types.
- private — limits access to containing type (no derived types).
Of interest to us here is, specifically, the protected keyword.? Let’s lay out a tangible example so that we can talk more concretely.? Consider the following simple Customer class.
public class Customer
{
public string LastName { get; set; }
public string SocialSecurityNumber { get; set; }
protected void ValidateSocialSecurity()
{
if (SocialSecurityNumber.Length != 9)
throw new InvalidOperationException("Customer has a bad SSN!");
}
}
The class itself and the last name and SSN properties have the public modifier.? Anyone can access them from anywhere.? The method ValidateSocialSecurity(), however, has the protected modifier.? That means only the Customer class and any methods deriving from Customer can invoke it.
But what happens when we add “sealed” to the declaration of Customer, making it “public sealed class Customer”?? Well, for one thing, CodeIt.Right flags it with the warning we’re talking about.? But think about what “sealed” means.? It means that the compiler won’t let you declare types that inherit from Customer.
So when we declare a protected member in a sealed type, we’re sending mixed messages.? We’re opening the member up for use by inheritors but also declaring the type in such a way as to make inheritance impossible.? This makes no sense and will confuse maintainers, so your tooling advises you to rethink.
In this case, CodeIt.Right offers you some easy solutions.? It will either remove the “sealed” designation for you or else change the visibility of the member in question for you to something appropriate.
Avoid long type arguments for VB6 clients
Let’s move now from the obvious to the somewhat arcane.? It wasn’t always this way.? But you don’t see a whole lot of VB6 COM code anymore.
This website explains COM, and you should take a cue from its call to install Silverlight and its ancient layout that we’re dealing with a relic.? You can also check out the wiki entry to see that it has been around since 1993.? There’s a rich history there, but for our purposes, you just need to know a couple of things.
- COM (like VB6) predates .NET as the way to allow disparate Windows libraries to work together.
- COM has a different set of rules than .NET, so you need to be mindful of how your .NET code interacts with COM, if applicable.
In the world of .NET, you’ll frequently have occasion to declare a long (or an Int64) — a 64-bit integer.? Generally, you do this when you’re confident that a “normal” 32-bit integer won’t get the job done in terms of scale.? By and large, that’s fine.
But if you specifically intend for your code to interact with COM that could include VB6 clients, it’s?not fine.? These COM clients will not know?how to read and access 64-bit integers, leading to runtime problems.
Most people will never see this?because you have to opt into COM visibility these days.? This warning crops up when you declare a type visible to COM objects and declare a long integer in that type.? To fix it, CodeIt.Right will offer to change the declaration to a regular int or a decimal, or else it will give you the option of adding a ComVisible attribute to the member in question and setting it to false.? This latter option tells the compiler to leave it out of the COM API.
Externally visible types and members should have XML comments
We’ve talked plenty on this blog about the subject of code comments.? Taking a stance on whether or not you should comment your code, and how much, seems to invite heated debates in the field of software.
So doesn’t it seem curious that a code review tool would wade into the thick of things?
Well, not if you take a closer look at what CodeIt.Right is suggesting, which is two specific things:
- XML comments…
- for externally visible code elements.
Why that?? Why not instead “You’re not commenting your code enough” or “Every method and type should have documentation?”? Well, method and type header comments in XML do more than just offer “comments” — they offer documentation.? In fact, bridging the gap between comments and documentation is the core value proposition of GhostDoc.
So what CodeIt.Right is really suggesting you do involves creating documentation.? When you format your comments this way, clients of your code receive various benefits.? Chief among these is information popping up in IntelliSense as you type and de facto documentation for API clients.
So CodeIt.Right isn’t reminding you to comment your code.? It’s reminding you to document your API as a courtesy to those using it.
Until Next Time
As always, I’ve tried to mix things up a bit.? First I talked about an issue listed under “design,” advising you to get on the same page between declaring members sealed and declaring things protected.? Then I took a look at the curious interoperability issue of mixing COM with long integers.? And finally, I described why CodeIt.Right gives you counsel on XML doc comments.
Join us next time for a look at three more rules for improving your .NET applications.
Learn more how CodeIt.Right can help you automate code reviews and improve the quality of your code.
