There have been times when, working on other people’s code, one of the first things that pops up is that their code generates thousands of warnings. I’m not exaggerating the number; there have been at least two times I can recall where someone’s code set generated that many warnings. And both times, the code set wasn’t all that large (only tens of thousands of lines).

Which leads us to Rule #3: Don’t ignore warnings!

There are actually two parts to this rule:

1. Turn on all warnings your compiler can recognize.
2. Write code that does not generate warnings.

Now that’s an absolutist stance, and in the real world nothing is absolute (except perhaps the fact that nothing is absolute). Ideally, you want to recognize all warnings, and, ideally, your code should generate no warnings.

That’s not always possible, and there are times when writing code to avoid a warning actually generates less clear — sometimes even convoluted — code. As always, nothing should interfere with Rule #1: Clarity Trumps Everything!

One example might be the Java warning for assignment to a  function parameter:

public void  foobar (int a, int b) {
    if (a < 0) {a = 0;}
    if (b < 0) {b = 0;}
    /* ... stuff ... */
}

The idea is that the function wants to ensure that “a” and “b” aren’t less than zero. This will generate the warning, “The parameter type should not be assigned” (assuming you’ve enabled that particular warning). The way around the warning might look like this:

public void  foobar (int _a, int _b) {
    int  a = _a < 0? 0: _a;
    int  b = _b < 0? 0: _b;
    /* ... stuff ... */
}

Or perhaps even worse:

public void  foobar (int _a, int _b) {
    int  a = _a;
    int  b = _b;
    if (a < 0) {a = 0;}
    if (b < 0) {b = 0;}
    /* ... stuff ... */
}

I consider both of those uglier than the “correct” way to do it.  The thing is, given that Java uses a pass by value protocol (yes, it does, trust me), there’s nothing at all wrong (in my eyes) with the original code fragment other than it perhaps violates some absolute sense of formal correctness.

Another Java example might be the “unqualified access to field” warning you would get from code like this (the assumption is that “host“, “port“, “timeout” and “sock” are instance variables):

public void  openSocket () {
    sock = new Socket(host, port);
    sock.setSoTimeout(timeout);
}

Obviously, to be most correct (and avoid the warnings), the code should be:

public void  openSocket () {
    this.sock = new Socket(this.host, this.port);
    this.sock.setSoTimeout(this.timeout);
}

To be honest, I go back and forth on this one. Not using the this keyword makes the code a bit cleaner and easier to read, but using it provides an important clue about those variables (i.e. that they’re instance variables).

For what it’s worth, I lean every so slightly towards using this exactly because of the extra information it carries.  This is one place where I’ll accept a bit of visual clutter for the sake of that extra info. (But it’s a close call and one I sometimes change my mind regarding.)

In the C-family of languages, a common warning comes from code like this:

void  foobar (int a, int b) {
    if (a = b) {
        /* ... stuff ... */
    }
}

Using an assignment as a gate expression may have indeed been the coder’s intent, but the more expected code would probably be:

void  foobar (int a, int b) {
    if (a == b) {
        /* ... stuff ... */
    }
}

That the C-family languages treat an assignment as an expression rather than a statement allows the first syntax to be valid code (if that’s what the coder really intended), but the more likely case of the single equals sign being a typo is why most compilers will issue a warning.

This is one case where a re-write is called for. I consider using an assignment as an expression to be bad coding. Plus, the example shown also violates the idea of not assigning to function parameters (although as I’ve mentioned, that’s one rule I’ll break rather casually).

The code should be written:

void  foobar (int a, int b) {
    int  c = b;
    if (c) {
        /* ... stuff ... */
    }
}

The example is so artificial that the code fragment seems pretty silly (why not just use “b” as the gate expression?), but it illustrates the point. The usual place you see assignment used as an expression is code like this:

void  readInput () {
    while (int ch = readchar()) {
        /* ... do stuff with ch ... */
    }
}

The assumption is that readchar() returns zero on end of input. The code above does have less visual clutter than one way of re-writing it to avoid the assignment as expression:

void  readInput () {
    int  ch = readchar();
    while (ch) {
        /* ... do stuff with ch ... */
        ch = readchar();
    }
}

But one can argue the re-write makes it slightly easier to see what’s going on. Another way to re-write it and avoid using readchar() twice might be:

void  readInput () {
    while ((int ch = readchar()) != 0) {
        /* ... do stuff with ch ... */
    }
}

And while the test against zero is unnecessary (in the C-family), it does satisfy the compiler and avoid the warning.

While we’re on the topic of assignment-as-expression, there is a trick many programmers use to help avoid typographic errors where equality was intended but they forgot to type the second equals sign. This trick is useful if your compiler isn’t capable of issuing warnings when it sees assignments used as expressions.

Re-write the code thus:

void  readInput () {
    while (0 != (int ch = readchar())) {
        /* ... do stuff with ch ... */
    }
}

By putting the constant first, any attempt to make an assignment of the expression will fail. (Because you can’t assign values to constants!) Programmers who use this trick use it universally:

void  foobar (int a) {
    if (5 == a) {
        /* ... stuff ... */
    }
}

Which generates an error on the accidental:

void  foobar (int a) {
    if (5 = a) {
        /* ... stuff ... */
    }
}

Now, there may be some warning types you won’t want to enable. For example, in modern Java, if you’re working with older code, you might need to turn off the warnings involving use of raw types (like Map and Set).

There was a time when I was writing Java for a J2EE environment than ran on Java 1.4, but was developing on a machine that had Java 1.5. Because the target didn’t support generic types, I had to use raw types which generated bucu warnings. I had no choice but to disable that warning for that code.

The bottom line is simply this: enable all warnings (or as many as possible) and then treat any warnings generated like errors.

You can make case-by-case exceptions to turning off warnings if the code issue cannot be avoided and there are many such instances. If there are only a few warnings generated, it may be better to leave the warning enabled and live with the warning.

But the unbreakable requirement should be this: shoot for minimum warnings (ideally none), and fully understand each and every warning that is generated.

I’ll close with this: if your code generates more than a dozen warnings, you’re not the coder you think you are. Get better! And if your code generates hundreds of warnings (or, god forbid, thousands) then, quite simply: You. Are. A. Hack. You either need to get a lot better or consider finding another profession.