How do I convince my teammates that we should not ignore compiler warnings?

Question

I work on a huge project (more like a tangled up combination of dozens of mini projects that can't be easily separated due to poor dependency management, but that's a different discussion) in Java using eclipse. We've already turned off a number of warnings from compiler settings and the project still has over 10,000 warnings.

I'm a big proponent for trying to address all warnings, fix all of them if possible, and for those that are looked into and deemed safe, suppress them. (Same goes for my religious obsession with marking all implemented/overriden method as @Override). My biggest argument is that generally warnings help you find potential bugs during compile time. Maybe out 99 of 100 times, the warnings are insignificant, but I think the head scratching that it saves for the one time that it prevents a major bug, it's all worth it. (My other reason is my apparent OCD with code cleanness).

However, a lot of my teammates don't seem to care. I occasionally fix warnings when I stumble across them (but you know it's tricky when you touch code written by a co-worker). Now with literally more warnings than classes, the advantages of warnings are very much minimized, because when warnings are so common-place, nobody will bother looking into all of them.

How can I convince my teammates (or the powers that be) that warnings need to be addressed (or suppressed when fully investigated)? Or should I convince myself that I'm crazy?

Thanks

(P.S. I forgot to mention what finally prompted me to post this question is that I sadly noticed that I'm fixing warnings slower than they are produced)

Answer 1

You can do two things.

1. Point out that warnings are there for a reason. Compiler writers don't put them in because they are mean-spirited. People in our industry are generally helpful. Many warnings are helpful.

2. Gather up a history of spectacular failures arising from ignored warnings. A web search for "Pay attention to compiler warnings" returns some anecdotes.

Your obsession with @Override is not an "obsession." That is a Good Thing. Ever misspelled a method name?

Answer 2

Relevant reading here. C++, but still relevant. I especially like this example (code comment is mine):

int searchArray(int to_search[], int len, int to_find)
{
    int i;
    for( i = 0; i < len; ++i )
    {       
        if ( to_search[i] == to_find )
        {
            return i;
        }
    }
    // should be returning designated 'not found' value (0, -1, etc)
}

A lot of the time, a warning will mean the difference between the application crashing with an error which will actually help you track down a design flaw and fix it (such as safe typecasting) or the application making assumptions and then actually behaving in an incorrect or erroneous manner (which would be the case with unsafe typecasting). Similarly, they also point out cruft or dead code which can be removed (unreachable code blocks, unused variables), which can be considered code optimisation, or unaccounted-for cases which will show up in testing, like the above example for C++. Note that this example results in a compile-time error in Java.

So, fixing warnings has (at least) several advantages for management:

1. Less chance of the code behaving improperly with user-inputted data which, to the higher-ups concerned about the image of the company and how it handles its clients' data, should be a Big Deal(tm). Crashing to prevent a user from doing something silly is better than not crashing and letting them do it.
2. If they want to reshuffle personnel, people can come into a warning-free codebase (and not freak out or complain as much..). Maybe this will reduce the amount of grumbling that goes on, or opinions on the maintainability or quality of Team X's contribution to Project Y.
3. Optimising code by removing compiler warnings, while not giving any significant improvements to running time, is going to improve numerous scores when it comes to testing:
   • Any code coverage scores will likely increase.
   • Testing boundary and invalid test cases will likely succeed more often (due to the abovementioned safe typecasting, among other things).
   • When a test case does fail, you don't have to think about whether it's due to one of the compiler warnings in that source file.
   • It's easily arguable that zero compiler warnings is better than thousands. No warnings or errors speaks about the quality of code, so you can argue that code quality improves the less warnings there are.
4. If you have no compiler warnings, and one or more are introduced, it's a lot easier to know who caused those to appear in the codebase. If you have a "no warnings" policy, that would help them identify people who aren't doing their job properly, perhaps? Writing code isn't just about making something work, it's about making it work well.

Note that I'm still just a junior dev who knows little about project management, so if I've said something wrong please correct my thinking and give me a chance to edit before you downvote me out of existence :)

Answer 3

I've worked on a project with similar characteristics in the past. This one in particular was originally written in Java 1.4. Once Java 5 with generics came out, one can imagine the number of warnings thrown by the compiler for every usage of the Collections API.

It would have taken quite sometime to get rid of all the warnings, by beginning to use generics. That is a factor that must be accounted for, but when you have to convince someone (especially your manager) that it needs fixing, you will need hard data, like

the time spent on bugs in legacy or non-standard compliant code (the standard is your project's coding standard) that could have been avoided.

You could keep presenting a bill that demonstrates how you are loosing time and money by ignoring "certain" warnings, and someone will get the idea. The key part is that not all warnings are worth looking at, at least not right away; in simpler words, you will need to establish the priority of the warnings that need addressing immediately. To begin with, consider the ones pertinent to bugs that you are seeing in your project.

You could also make notes in the bug tracker, when you fix bugs, that the said bug could have been avoided by not ignoring a warning (or perhaps by running PMD or FindBugs if you have a CI or build system). As long as there are a sufficient number of bugs that can be fixed by heeding compiler warnings, your point about looking at compiler warnings would be valid. Otherwise, it is a business decision, and usually it is not worth the time spending to fight this battle.

Answer 4

If you succeed and your team decides to observe the warnings, you should take a step-wise approach. No one can and will 10000 warnings in one time. So you can select the most important ones (those that are bugs with high probability) and disable the less significant ones. If they are fixed, increase the warning level again. Additionally, you can start with FindBugs, which warns on code that is almost always a bug.

Answer 5

I totally agree with you, it's a best practice to clean up the compile warnings as more as possible. You mentioned your team is using Eclipse as development tool. Eclipse is a very good tool to help you clean up the code and make the code style consistence.

1. define 'Save action' for your each Java project, such as formatting code, unused local variables, organizing the imports(I think nobody has objections on such kind of cleaning up).
2. define project specific compile options for each project. For example, compile level(if your code need be compatible with 1.4 to clean the warnings related to generic), assignment has no effect(e.g. 'x=x') and so on. You and your teammate can make an agreement for those items, and Eclipse will report them as 'Error' in your development phase after you have agreement for the compile error.

Eclipse will create some properties files for those preference under .settings folder, you can copy them to other Java projects and check them in to SCM as part of source code.

You can check out the code of Eclipse to see how Eclipse developers do it.

Answer 6

You have two ways of getting rid of all the warnings (and I agree it can hide a subtle bug):

1. Convince the whole team that this is the best thing to do, and have them fix it.
2. Convince your boss that this is the best thing to do, and make it mandatory. Then they need to fix it.

I believe that 1 is not feasible in your case any more. Also this will probably take so long due to the number of warnings that this will show in the productivity output so the management will need to know anyway.

So, my suggestion is: Take it up with the boss, convince him this is a ticking bomb, and have it made official policy.

Answer 7

I would just say them that most of these are insignificant warnings and it is essential that we remove them from the list. So that we will not miss the real significant warning in the crowd, as it happens!

Answer 8

You will need lots of patience in trying to convince them, removing warnings when doing pair-programming will help others pick up the habit. Suggest them to read Effective Java 'Item 24 : Eliminate unchecked warnings' (for generic collection). And probably ignore many instances when people do not follow your advice ;)

Answer 9

An effective approach here would be to setup a Continuous Integration server that automatically builds the project and runs the tests whenever anyone checks in code. If you're not already using this, you should be, and it's not very hard to convince others of the benefits of doing so.

1. Convince management / team leads of the benefits of doing this (preventing bad builds from being deployed, finding bugs early, especially those that accidentally affect other parts of the software, maintain regression testing, testing early and often, etc.)

2. Install CI server with all tests passing (if you don't have tests, write a quick one that passes, so that everyone sees that it's green).

3. Setup emails or other notifications about the status of each build. It's important here to include who checked in the code, what the change was (or a link to the commit), and the status+output of the build. This is an important step, as it causes team-wide visibility both for successes and for failures.

4. Update the CI server to make tests fail if there are any warnings. This will be seen by management and team leads as increasing the discipline of the team in a systematic way, and no one wants to be responsible for all the failure emails going out.

5. (optional) get nice and geeky with this, by adding some visible dashboards, lava lamps, flashing lights, etc. to indicate the status of the build and who broke it / fixed it.