CodeIt.Right Rules Explained, Part 7

Today, I’ll do another installment of the CodeIt.Right Rules, Explained series.? This is post number seven in the 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 like rhetorical cuteness.? After all, I could simply say, “Learn the reasoning behind all suggested fixes.”? But I want to highlight the decision you face when confronted with static analysis feedback.? In all cases, you must actively choose to ignore the feedback or address it.? And for both options, you need to understand the logic behind the suggestion.

In that spirit, I’m going to offer up explanations for three more CodeIt.Right rules today.

Disable anonymous access in web.config

In the world of .NET web development, the web.config file occupies a central role. ?As you can infer from its name, it stores configuration information for your application. ?Some common bits of configuration include the following:

• Connection strings
• Security and authentication information
• Session state handling
• Role and profile information
• Custom application settings

It has plenty more besides, but that should give you a taste. ?Of particular interest today we have web.config’s role in authenticating and authorizing users. ?Authentication involves establishing their identity, whereas authorization involves determining what resources they can access based on that identity.

You can use the web.config file to configure rules for this. ?And then you can make those rules fairly elaborate. ?Want to define groups and roles? ?Fine. ?Want to control access to pages and directories in granular fashion? ?No problem. ?It gives you a lot of power, but you can tie yourself in knots. ?And you can also undercut your own best interests with an errant setting.

.NET web applications really shine?as applications. ?In other words, they have a common use case of authenticated interaction, particularly compared to something like a content or marketing site. ?People writing .NET web applications usually want to secure them.

This CodeIt.Right setting brings your attention to a situation where you may be undermining that desire. ?It’s saying, “Hey, you’re allowing anyone to access your resources.” ?If this is your intention, you can disregard the warning. ?Just make really sure that it is, in fact, your intention.

Do not include multiple statements on a single line

Let’s now go from a highly functional concern around security to a more cosmetic one in your source code. ?This rule admonishes you to avoid having more than one statement on a single line. ?So, for example, you should avoid something like this:

public void DoSomething()
{
    CallAMethod(); CallAnotherMethod();
}

Yes, you can do that as perfectly legal C#. ?And if you do it, CodeIt.Right will flag it. ?This goes against any common language convention you’ll see in C#, but you may encounter cases that tempt you to do this. ?For instance, up until C# 7, the noisiness of out parameters might make you want to squish code onto a single line.

public int Convert(string conversionTarget)
{
    int ret; int.TryParse(conversionTarget, out ret);
    return ret;
}

I can’t speak for anyone else, but I’ve found that sort of thing tempting. ?All of the magic happens in the parse call, and declaring an int just to make TryParse() happy seems laborious.

But CodeIt.Right has its reasons for discouraging this. ?First, as I’ve already mentioned, it goes against expected convention and runs afoul of the principle of least astonishment. ?You will confuse other people by doing this. ?Second, and perhaps more concretely, this plays havoc with your debugging workflow. ?Put these statements on a single line, and you lose easy control over setting a breakpoint on one of them. ?Finally, it torpedoes your ability to add a comment specifically describing the first statement.

If you want to make your code more vertically compact, I’d advise the clean code approach of writing small, focused methods.

Abstract class should have at least one derived class

Last up, let’s take a look at something that’s perhaps a bit philosophical. ?This rule, under the heading of “design,” suggests that you not have abstract classes without at least one class that inherits from them. ?To address this, I’m going to divide potential codebases into two camps. ?First, there’s codebases not intended as libraries, and second, there’s those intended as libraries.

If nobody outside your team will ever use your code, then a non-inherited abstract class counts,?ipso facto, as dead code. ?At best, it’s speculative and runs afoul of YAGNI. ?After all, abstract classes cannot be directly instantiated. ?You use them through concrete inheritors. ?Without those concrete inheritors, nobody uses that code. ?So it just sits in your codebase, taking up space (and maintenance mindshare/effort). ?Don’t leave that there. ?Honestly, I’d say just delete it and re-implement it later when you have concrete need for it.

Now, consider the case where you distribute this code as a library or a framework of some sort. ?In this case, you might define an abstract class and task your users with defining inheritors. ?In this case, you?might disregard this warning. ?But I actually wouldn’t advise this. ?Forcing your clients to use a framework via inheritance is a high-friction, low-discoverability way to interact with them. ?Modern design wisdom suggests featuring composition over inheritance. ?So when you see this warning, ask yourself if foisting inheritance on your clients really represents an ideal design. ?Could you achieve the same thing with an interface or a composable model? ?If so, maybe listen to the static analyzer and go with that approach instead. ?If not, well, you can ignore the warning in this narrow situation.

Until Next Time

In this series, I’ve increasingly tried to cover a variety of concerns in each post. ?Here, we’ve looked at a web-development-specific concern. ?Next up came something cosmetic but pragmatic. ?And finally, we took a look at more of the design/architectural side of things. ?So, variety mission accomplished.

When you use a tool to get automated review feedback, you get more than just the feedback. ?You get insight into design principles that can help you learn and make better decisions going forward.

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.