CodeIt.Right Rules Explained, Part 15

It’s time again for another post in the CodeIt.Right Rules Explained series.? We’re fifteen posts deep into the series now, as you can see from the title.? But if you haven’t seen these posts before, I’ll recap the same information I cover in each intro.

CodeIt.Right?is an automated .NET code review tool (both C# and VB), and it has a lot of rules to help you.? In the posts in this series, I explain those rules in detail.

In all posts in this series, I start out by listing these rules of thumb for static analysis in the hopes that repetition hammers them home.

• Never implement a suggested fix without knowing what makes it a fix.
• Never ignore a suggested fix without understanding what makes it a fix.

And as always, I’ll explain that I don’t use this phrasing to be cute or because I can’t figure out the logical implication of these statements: “always understand why the tool is suggesting a fix.”

I do this because whenever a static analysis tool confronts you with a warning, you face a choice: address the warning or else ignore it.? And, either way, you should act intentionally and for the right reasons.? If you’re ignoring it, do so because you think the pros of your warning-triggering code outweigh the cons implied by the warning.? Don’t ignore it because you don’t understand and don’t feel like doing it.? And on the other side of the coin, don’t change your code simply because some tool tells you to.? Always understand why you’re changing your code, or else you’re programming by coincidence.

All that said, let’s look at today’s three rules.

Async Method Should Have Await Statement

It’s been a while since I last talked about the async-related rules in one of these posts.? Today I’ll remedy that by discussing one of the most fundamental warnings CodeIt.Right has to offer as far as async is concerned.? It’s an admonition that an async method should have an await statement within it.

Asynchronous programming is a broad and important concept in .NET.? Getting deep into the theory of it would go well beyond the scope of this post, but you can check this page out if you want a primer on asynchronous programming.

But here’s the quick version.? The code you write is, by default,?synchronous, meaning that it executes one statement after the next.? But if you add the async keyword?to a method declaration, you’re telling the compiler that this method will behave in asynchronous fashion.? Your method will execute sequentially until it encounters an await keyword, which, in essence, says, “go off and do this thing that will take a while and pick back up when it finishes.”

var order = _orderQueue.GetLastOrder();
await _rocketShip.SendOrderToTheMoonAndBack(order);
Console.WriteLine("Order is now covered in moon dust.");

The main purpose of async is to inform the compiler to enable the await keyword in a given method.? So declaring an async method but not using await violates the principle of least astonishment (and is most likely an oversight, hence the warning).? CodeIt.Right gives you a warning about this and so does the compiler.? To fix the issue, you should probably either add await inside the method somewhere or else remove the async keyword from the method declaration.

Do Not Hide Base Class Methods

Let’s switch gears a little and talk some classic OOP.? This is a warning that you’ll see when you’re creating inheritance hierarchies in the .NET world.? It instructs you not to hide base class methods.? Consider the following classes:

public class Vehicle
{
    protected bool IsOn { get; set; }
    public void Start()
    {
        IsOn = true;
    }
}
public class Boat : Vehicle
{
    public void Start()
    {
        Console.WriteLine("Spraying water everywhere!");
    }
}

This code generates the warning in question (as well as a compiler warning, like the last CodeIt.Right rule).? And that’s because of a subtle, but important, oversight.

Generally, when you write code like this, the intent is for the derived class, Boat, to override the behavior defined in the base class, Vehicle.? But remember that classes need to signal that they’re willing to participate in method overriding by using the virtual keyword.? And derived classes then override by using the override keyword.

CodeIt.Right warns you when you’re “hiding” a base class method instead of overriding it.? The reason this is a dangerous situation is that it will generally seem to work in most cases.? If you instantiate a Boat and invoke Start(), you’ll see the behavior you want. ?The same thing will happen with instantiating and invoking it on Vehicle.? But things can get weird.

var boat = new Boat();
boat.Start();
(boat as Vehicle).Start();

The second line of code there invokes Boat.Start() as you might expect.? But the second one invokes Vehicle.Start().? If this were a proper inheritance relationship, rather than method hiding, both would invoke the Boat.Start() method.

Hence the warning.? When people hide methods, they often don’t mean to do it.? So either give the derived class method a different name or else make it a proper inheritance relationship.

Compilation Debug Should Be False in Web.config

The last couple of explanations have been relatively involved.? So let’s take a look at one that’s actually pretty straightforward.? CodeIt.Right will tell you that “compilation debug” should be set to false in the Web.config file, which is your configuration mainstay for any ASP web applications.

Here’s what the offending bit of XML looks like in that file when you see this warning:

<configuration>
 <system.web>
  <compilation debug="true" />
 </system.web>
</configuration>

The warning is simply telling you that you probably don’t want this flag set to true.? Why not?? Well, you set this to true specifically when you want to seamlessly debug your ASP application.? And it’s very helpful for that.? But like hyper-verbose logging or costly application instrumentation, you don’t want to leave this on all the time.? It has significant performance implications, security implications, and more.? If you’d like to read in detail, Scott Gu wrote about it years ago.

CodeIt.Right flags this more or less as a reminder.? You may toggle this value here and there to help with your debugging efforts.? But you don’t want it to stay true past debugging.? You don’t want to commit it as true, and you certainly don’t want to deploy it this way.? So remember to set it back to false.

Until Next Time

As always, I’ve tried to include some variety in this post.? We took a look at something from the world of asynchronous programming, reminding you that the purpose of async is to await stuff.? Then we made a foray into the world of classic object-oriented programming to look at the finer details of inheritance in C#.? And finally, we looked at some web-specific settings with security and performance implementations.

Stay tuned for next time when we have a VB-specific option, some old-school COM stuff, and another web-specific item.

Learn more how CodeIt.Right can help you automate code reviews and improve the quality of your code.

About the author

Erik Dietrich

Website

I am Erik Dietrich, founder of DaedTech LLC, programmer, architect, IT management consultant, author, and technologist.

This author does not have any more posts.