Time for yet another deep dive into three more CodeIt.Right Rules. For those of you who don’t know about our CodeIt.Right Rules series, here’s a brief explanation.
CodeIt.Right is one of the tools offered by SubMain. It’s a static code analyzer tool or, more specifically, an automated code review tool. CodeIt.Right works by checking your code against a set of rules and giving you instant feedback on how to solve potential problems. It even offers automatic fixes when applicable.
In each installment on this ongoing series, we explain three CodeIt.Right rules, usually picked from as diverse a category as possible, to showcase the flexibility and breadth of the ruleset.
Readers of this series know that we like to start these posts by stating two things:
- Never implement a suggested fix without knowing what makes it a fix.
- Never ignore a suggested fix without understanding what makes it a fix.
Yeah, you could simplify this down to “always understand a suggested fix.” But I choose to not put it that way since I want you to really think and consider your options. If you decide to ignore a fix, do so understanding the consequences. On the other hand, if you accept a suggested fix, do it with a full understanding of how that’s going to benefit your application.
With all that in mind, let’s get to today’s rules.
Do Not Catch General Exception Types
This post’s first rule falls into the “exception handling” category. That’s not surprising since it’s called “Do not catch general exception types.” You violate this rule when you catch System.Exception or System.SystemException. You can also violate this rule by using a general catch clause—in other words, a catch that doesn’t specify an exception type. The following listing features a code excerpt that will trigger a violation of this rule:
public class ProductRepository : IProductRepository
{
private readonly string FILE_PATH;
public IEnumerable<Product> All()
{
string content;
try
{
content = System.IO.File.ReadAllText(FILE_PATH);
}
catch (Exception)
{
content = string.Empty;
}
return string.IsNullOrWhiteSpace(content)
? Enumerable.Empty<Product>()
: JsonConvert.DeserializeObject<IEnumerable<Product>>(content);
}
}
As you can see, the code above features a try-catch block that catches the most generic exception one can handle: System.Exception. This is going to trigger a violation of this rule. We’ll soon show you a fixed version of the same code. But first we need to understand why this is even an issue to begin with. Even the .NET Framework design guidelines recommend not to catch generic exception types, stating that you should catch more specific types. But why?
The answer is simple: when you catch a generic exception type, you blur the distinction between expected and unexpected errors.
It’s not that hard to understand the intent of the code above. The method tries to access a file, which has its location denoted by a constant. The file’s content is completely read and assigned to a variable. After that, the variable’s content is checked. If it’s null or contains only whitespace or the empty string, the method returns an empty sequence of the Product class. If, on the other hand, the string is valid, we deserialize its content (which we now know is supposed to be a JSON string) into a sequence of the same object and return that sequence.
The file might not exist, and that’s fine. The code expects the scenario when the file is not there. However, in its current form, the code has a serious flaw. To understand that, let’s do a quick thought experiment. Imagine that the path stored in the FILE_PATH constant refers to some network drive (e.g., “G:\submain-demo-app\products.json”). Imagine also that, for some reason, the drive got unmapped. What’s going to happen? Well, the “ReadAllText” method is going to throw a System.IO.DirectoryNotFound exception, which will then be “silenced” by the catch block. But it shouldn’t be silenced. The problem with the network drive definitely belongs to the realm of unexpected exceptions. Instead of being quietly dismissed, it should be escalated in order to be noticed as quickly as possible.
What’s the correct approach? Simple: you catch exceptions as specific as possible. The fixed version of the code above looks like this:
public class ProductRepository : IProductRepository
{
private readonly string FILE_PATH;
public IEnumerable<Product> All()
{
string content;
try
{
content = System.IO.File.ReadAllText(FILE_PATH);
}
catch (System.IO.FileNotFoundException)
{
content = string.Empty;
}
return string.IsNullOrWhiteSpace(content)
? Enumerable.Empty<Product>()
: JsonConvert.DeserializeObject<IEnumerable<Product>>(content);
}
}
Now things are way better. We’re now catching only the specific exception type we want to acknowledge (and in this case, ignore): System.IO.FileNotFoundException. If the ReadAllText method throws any other type of exception, the catch block won’t handle it and it will bubble up until someone decides to deal with it.
COM Visible Types Should Be Creatable
The second rule in today’s post is also the most obscure rule of the ones we’re showing you today. And by that, I mean that it’s the one most likely to make you raise your eyebrows and scratch your head. Well, let’s get to it.
This rule falls into the “Interoperability” category. It states that “COM visible types should be creatable.” If you’re not familiar with COM, then there’s a good chance you’re one of the ones scratching your head right now. COM stands for “Component Object Model.”
COM is a mechanism that allows developers to distribute binaries that can be used even if the caller was using a different programming language. Some people refer to it as “the .NET from old times,” and that’s not that far from the truth. Even though COM is getting obsolete, it’s still very much alive.
You violate this rule when you have a class that’s visible for COM—i.e., marked with the “System.Runtime.InteropServices.ComVisibleAttribute” attribute—but doesn’t have a public, parameterless constructor. That means the type isn’t creatable by COM clients. COM clients can still access it, though, provided there’s another way of creating the objects and passing them to the clients.
To fix the violation, just add a parameterless constructor to the class and you’re done.
Source File Name Should Be Pascal Cased
Today’s last rule falls into the general category. It states the file names should follow the Pascal case convention. And what would that be?
Pascal case is a specific variation of the camel case naming convention. Camel case, on the other hand, is the practice of writing compound words or phrases such that each word or abbreviation in the middle of the phrase begins with a capital letter, with no intervening spaces or punctuation. For instance, one could write the phrase “Hello, world!” as both “HelloWorld” and “helloWorld.” How could we distinguish between the two forms?
To solve that issue, people usually call the version that capitalizes the first letter “upper camel case” and the version that doesn’t capitalize the initial letter “lower camel case.” “Pascal case” is just another name for the upper camel case variation.
The generally accepted practice in the C#/.NET world is to use Pascal case to name types (classes, interfaces, structs, etc.), methods, and properties. You should use camel case to name fields, local variables, and parameters.
Consider the code below to see some examples:
public class ProductLoggingRepository : IProductRepository
{
private readonly IProductRepository repository;
private readonly ILogger logger;
public ProductLoggingRepository(IProductRepository repository, ILogger logger)
{
this.repository = repository ?? throw new ArgumentNullException(nameof(repository));
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
}
public IEnumerable<Product> All()
{
logger.Info("Retrieving all stored products.");
return repository.All();
}
public void Save(Product product)
{
logger.Info("Saving product: " + product.ToString());
repository.Save(product);
}
}
Awesome, but what does all of this have to do with the file name? It’s pretty much a matter of convention. Here, CodeIt.Right is helping you conform to the principle of least astonishment. If you name your files in odd and inconsistent ways, it’s going to be counterintuitive to the people in your team (and even to you). If you’re lazy like me, CodeIt.Right’s got your back: it offers the automated fix of renaming the file so it becomes properly cased.
That’s It For Today
CodeIt.Right rules cover a lot of ground and can fall into diverse categories. In today’s post, we covered rules falling into the exception handling, interoperability, and general categories, in that order. If you aren’t a user of CodeIt.Right yet, what are you waiting for? Download it and install it today, and start taking full advantage of the power of its rules.
