Warnings Need to be Cleaned Up Too

Quick show of hands - how many of you developers reading this clean up compiler warnings in your projects? Every one of you that raised your hand, good for you for following directions, but bad for you since you literally followed a command from a blog post as you were reading it. Now those of you that didn't raise your hands because you don't get rid of warnings in your code, shame on you. Those warnings exist for a reason; to tell you that something may actually be wrong with your code, even if it technically isn't an error. They should be treated just like errors, you should not consider your code ready until every warning has been addressed. Of course, addressing some warnings may result in you doing nothing with the warning, and letting it live on in your code, but you still need to review the warning and make sure that you are doing the right thing with it.

Why Should Warnings be Fixed?

Warnings should be fixed for a very simple reason – they can actually cause problems in your code. Here is a great example. We had a developer that did an automated conversion of some VB.NET code to C#. When he did this, he didn't bother checking to make sure his warnings were cleared up. He ended up with some code that essentially boiled down to this:

bool a = false;
if (a = true)
{
    // Do something
}

This is clearly a problem and definitely not what the intention was – the intention was to compare ‘a’ to true, not assign it the value true (which means that this if statement was always firing, even when it shouldn't). Had he checked out his compiler warnings, he would have found something like this:

Assignment in conditional expression is always constant; 
did you mean to use == instead of = ?

Makes it pretty clear as to what the issue is and how to fix it. Since this company had no formal code review process (or any review process really – but that’s a topic for another day), the code was released to production this way and stayed that way until I happened to open up the project in question to work on something else and got very irritated by all the warnings this project had when compiled. I decided to clear them out since most of them were deprecation warnings about objects that had been replaced as of .NET 2.0. In the process of doing this, I found the warning above, and realized that this bad code had been running on this site for several months. Checking out the results of this bad code, I found that we had lost a fairly significant chunk of potential revenue, all because the warning was not heeded.

This brings up a good reason why all warnings should be fixed, not just those that you might think cause an error. All of those warnings about deprecated classes were forcing the problem warning into obscurity and actually invisibility – the warning was at the bottom of a list of about 50 warnings. For the average (okay, maybe below average) programmer, seeing a bunch of the same warning in a row may lead that programmer into thinking that all of the warnings are like that, and he doesn’t need to do anything with them. Of course, the bad programmer is just going to ignore warnings entirely, even hiding them from view in the IDE). However, a good developer going in and replacing the deprecated classes with the new classes (along with cleaning up any other warnings he finds) is going to end up seeing that bad warning at one point and make sure it gets fixed properly. To me, this is one of the absolutely necessary steps that needs to be taken by anyone that wants to consider himself a good developer. And everyone reading this is a good developer, right? So that means everyone that didn't raise their hands before is going to start addressing warnings in their code properly, right? Okay, sounds good. My work here is done.