34
Warnings Are Really Errors
“Compiler warnings are just for the overly cautious and pedantic. They’re just warnings after all. If they were serious, they’d be errors, and you couldn’t compile. So just ignore them, and let ’er rip.” |
|
When your program has a compilation error, the compiler or build tool refuses to produce an executable. You don’t have a choice—you have to fix the error before moving on.
Warnings, unfortunately, are not like that. You can run the program that generates compiler warnings if you want. What happens if you ignore warnings and continue to develop your code? You’re sitting on a ticking time bomb, one that will probably go off at the worst possible moment.
Some warnings are benign by-products of a fussy compiler (or interpreter), but others are not. For instance, a warning about a variable not being used in the code is probably benign but may also allude to the use of some other incorrect variable.
At a recent client site, Venkat found more than 300 warnings in an application in production. One of the warnings that was being ignored by the developers said this:
| | Assignment in conditional expression is always constant; |
| | did you mean to use == instead of = ? |
The offending code was something like this:
| | if (theTextBox.Visible = true) |
| | ... |
In other words, that if will always evaluate as true, regardless of the hapless theTextBox variable. It’s scary to see genuine errors such as this slip through as warnings and be ignored.
Consider the following C# code:
| | public class Base |
| | { |
| | public virtual void foo() |
| | { |
| | Console.WriteLine("Base.foo"); |
| | } |
| | } |
| | |
| | public class Derived : Base |
| | { |
| | public virtual void foo() |
| | { |
| | Console.WriteLine("Derived.foo"); |
| | } |
| | } |
| | |
| | class Test |
| | { |
| | static void Main(string[] args) |
| | { |
| | Derived d = new Derived(); |
| | Base b = d; |
| | d.foo(); |
| | b.foo(); |
| | } |
| | } |
When you compile this code using the default Visual Studio 2003 project settings, you’ll see the message “Build: 1 succeeded, 0 failed, 0 skipped” at the bottom of the Output window. When you run the program, you’ll get this output:
| | Derived.foo |
| | Base.foo |
But this isn’t what you’d expect. You should see both the calls to foo end up in the Derived class. What went wrong? If you examine the Output window closely, you’ll find a warning message:
| | Warning. Derived.foo hides inherited member Base.foo |
| | To make the current member override that implementation, |
| | add the override keyword. Otherwise, you'd add the new keyword. |
This was clearly an error—the code should use override instead of virtual in the Derived class’s foo method.[34] Imagine systematically ignoring warnings like this in your code. The behavior of your code becomes unpredictable, and its quality plummets.
You might argue that good unit tests will find these problems. Yes, they will help (and you should certainly use good unit tests). But if the compiler can detect this kind of problem, why not let it? It’ll save you both some time and some headaches.
Find a way to tell your compiler to treat warnings as errors. If your compiler allows you to fine-tune warning reporting levels, turn that knob all the way up so no warnings are ignored. GCC compilers support the -Werror flag, for example, and in Visual Studio, you can change the project settings to treat warnings as errors.
That is the least you should do on a project. Unfortunately, if you go that route, you will have to do it on each project you create. It’d be nice to enable that more or less globally.
In Visual Studio, for instance, you can modify the project templates (see .NET Gotchas [Sub05] for details) so any project you create on your machine will have the option set, and in the current version of Eclipse, you can change these settings under Window Preferences Java Compiler Errors/Warnings. If you’re using other languages or IDEs, take time to find how you can treat warnings as errors in them.
While you’re modifying settings, set those same flags in the continuous integration tool that you use on your build machine. (For details on continuous integration, see Different Makes a Difference.) This small change can have a huge impact on the quality of the code that your team is checking into the source control system.
You want to get all of this set up right as you start the project; suddenly turning warnings on partway through a project may be too overwhelming to handle.
Just because your compiler treats warnings lightly doesn’t mean you should.
|
Treat warnings as errors. Checking in code with warnings is just as bad as checking in code with errors or code that fails its tests. No checked-in code should produce any warnings from the build tools. |
What It Feels Like
Warnings feel like…well, warnings. They are warning you about something, and that gets your attention.
Keeping Your Balance
-
Although we’ve been talking about compiled languages here, interpreted languages usually have a flag that enables run-time warnings. Use that flag, and capture the output so you can identify—and eliminate—the warnings.
-
Some warnings can’t be stopped because of compiler bugs or problems with third-party tools or code. If it can’t be helped, don’t waste further time on it. But this shouldn’t happen very often.
-
You can usually instruct the compiler to specifically suppress unavoidable warnings so you don’t have to wade through them to find genuine warnings and errors.
-
Deprecated methods have been deprecated for a reason. Stop using them. At a minimum, schedule an iteration where they (and their attendant warning messages) can be removed.
-
If you mark methods you’ve written as deprecated, document what current users should do instead and when the deprecated methods will be removed altogether.