It’s time for a deep dive into yet another three CodeIt.Right rules.? In case you’ve missed it, this continues a post series in which we, well, do a deep dive into the CodeIt.Right Rules.
CodeIt.Right?is an automated code review tool, making it fall under the broad heading of static analyzers.? It checks your source code against a set of rules, both ones that you customize?and a set that comes out of the box.? In this series, we’re focusing on the latter, since those are the ones that will obviously apply to everyone.
Followers of this series will know that I always kick the posts off by listing two static analysis rules of thumb that I advertise as good ideas.
- Never implement a suggested fix without knowing what makes it a fix.
- Never ignore a suggested fix without understanding what makes it a fix.
If this seems a little obtuse, it’s probably because you could condense these two rules into a single one by logical implication: “always understand a suggested fix.”
But I deliberately choose not to phrase it this way, opting for the two separate rules.? I do this to emphasize your choice: ignore the warning or implement a fix.? If you decide to ignore the warning, you should do so only when you’re fully aware of the risks of doing so.? If, on the other hand, you decide to fix the warning, you should always understand what you’re doing when you change your own code.? Otherwise, you’re programming by coincidence.
Don’t program by coincidence.? Instead, take a look at today’s three rules.
Avoid ‘Out’ and ‘Ref’ Parameters in Async Method
CodeIt.Right has an entire suite of rules around the async programming construct.? And this makes sense since it is both an important concept and a potentially confusing one.? It can be hard to reason about the intersection between immediate and long-running code.
To understand this rule, let’s take a look at a coding example.? It’s admittedly a little goofy, but hopefully, it illustrates the point.
public void ShowTemperatureOnJupiter()
{
int temperatureOnJupiter = GetTemperatureOnPlanet(Planet.Jupiter);
Console.WriteLine($"The temperature on Jupiter is {temperatureOnJupiter}");
}
private int GetTemperatureOnPlanet(Planet planet)
{
int temperature;
GetTheTemperatureOnPlanetAsync(planet, out temperature);
return temperature;
}
private async void GetTheTemperatureOnPlanetAsync(Planet planet, out int temperature)
{
temperature = await _spacePhone.CallPlanetForTemperatureAsync(planet);
}
Space is pretty big.? So it takes a while to place a phone call to Jupiter and ask them what the temperature is.? And that’s what this code does.? The GetTemperatureOnPlanet method declares an integer, passes it to the async method as an out parameter and depends on the async method to update it.
At first blush, this might seem fine.? But there’s a problem here.? Specifically, async methods return pretty much right away — before the processing is done.? So this code will actually dispatch a call to Jupiter and then return twice, back to the public method.? And because of this, by the time the actual temperature comes back from Jupiter, the temperature variable will no longer be in scope.
CodeIt.Right is warning you about this situation.? It turns out the compiler will also offer this feedback — this code won’t compile.? But the CodeIt.Right warning is helpful in that it gives you more information about the reasoning (including code examples) compared to the compiler’s terse error message.
Do Not Place Assembly Attributes Outside of AssemblyInfo File
Next up, we have a rule that falls into the “design” category.? Here, CodeIt.Right admonishes you not to put assembly attributes in places other than the AssemblyInfo.cs file.? I sincerely hope your reaction to this is, “why would anyone do that?”? But in case it’s something else, let me elaborate.
To understand what assembly attributes are, don’t overthink things.? They are attributes that you can define for the assemblies that you’re building as part of your codebase.? Some examples include:
- Version
- A GUID
- Your company name
- Copyright information
Hopefully you get the idea.? When you’re shipping your DLLs and executables places, you can control this meta information via assembly attributes.
To do this, Visual Studio provides you with a handy construct whenever you create a new project.? This is the AssemblyInfo file, pictured here:
As you can see, Visual Studio creates the file and pre-populates it with a bunch of information, for your convenience.? You can update it as you see fit.
The warning comes in when you start defining assembly attributes in other files.? You can, technically, define those attributes in any code file you please, provided they’re assembly level scope (outside of a class).? But the question is, “why would you?”
In this case, CodeIt.Right is just helping you conform to the principle of least astonishment.? If you put this in other places, it’s going to be extremely counterintuitive to everyone else in your group and hard to track down.? And, for your convenience, CodeIt.Right offers the automated fix of moving it into the right place for you.
Acronyms Should be Cased Correctly
For the last one, we’ll dive into the “naming” category.? This section is usually pretty straightforward, in that it will call out items that have to do with what you call code elements.? Let’s dive right in with an offending example.
public string GetSportsMVP(Sport sport)
{
switch(sport)
{
case Sport.Baseball: return "Mike Trout"; break;
case Sport.Basketball: return "James Harden"; break;
case Sport.Football: return "Tom Brady"; break;
default: return string.Empty;
}
}
There’s a decent amount of code here, but the entirety of the problem lies in the name of the method: “GetSportsMVP.”? CodeIt.Right wants long acronyms (three or more letters) to be Pascal Cased.? So, if you employ its auto-correct option, you will find your method (and those referring to it) renamed “GetSportsMvp.”
Now, let’s get something out of the way here.? This is arbitrary.? Full stop.
But, it also hews toward the principle of least astonishment that we discussed earlier.? Pascal casing is the way people do things in .NET.? So, if you go creating your own conventions on the fly, they’re going to surprise your fellow developers, even when self-consistent.? And this is actually pretty important when it comes to things like acronyms, where you’re never quite sure how to case them exactly.? Having a tool steer everyone in the same direction with automated feedback is a big help.
That said, if your entire group prefers acronyms always to be all uppercase, you can, of course, turn the rule off.? Or, you can even create a new rule enforcing the standard that you prefer.? Naming rules make for an excellent potential example of “ignore a warning when you understand the reasoning and don’t agree.”
Until Next Time
As always, I tried to provide a bit of variety here.? First up, we looked at a rule from the relatively complex world of async programming.? Then we saw a design rule that had to do with conventions about where to define assembly meta information.? And, finally, we headed into the always-contentious subject of naming.
Tune in next time, and we’ll take a look at another three rules, including one about globalization, another one about naming, and one about managing configurations in web assemblies.
Learn more how CodeIt.Right can help you automate code reviews and improve the quality of your code.
1 Comment. Leave new
[…] And, finally, here’s another in the CodeIt.Right rules explained series. […]