Code formatting: is it worth to make bad code to look bad?

Question

In Code Complete 2nd Edition, Steve McConnell wrote the following (page 732):

Techniques that make good code look good and bad code look bad are more useful than techniques that make all code look good.

I have an argue about that and Eclipse code format rules with a colleague. He said that badly looking code makes refactoring and cleaning up bad code harder, since he needs more effort to understand the code. On the other hand I've found that code that looks bad is similar to a "be careful" warning. I reminds me that I have to think it through twice before modifying anything to make sure I won't broke anything. Furthermore, it probably forces developers to restructure their code to be a little bit more beautiful and better structured.

Another thing is @SuppressFBWarnings annotations for false positive FindBugs warnings which also makes the code harder to maintain since these annotations look bad, break the structure of code and they're hard to read. On the other hand I think these also save time. When a developer finds some code that smells and FindBugs has already found it the developer probably has a reason why it's OK (or why somebody thought it's OK) in the justification element of the annotation.

Are there other reasons why is it worth or not to make bad code to look bad or it is rather a psychology question and both formatting are acceptable?

Answer 1

The quote from the same page, same paragraph, a few sentences before:

Making the code look pretty is worth something, but it's worth less than showing the code's structure. If one technique shows the structure better and another looks better, use the one that shows the structure better. [...] In practice, prioritizing logical representation usually doesn't create ugly code—unless the logic of the code is ugly.

The point is not that you should format bad code badly intentionally. The point is that in good code, you can easily see the structure, while in bad code, finding any structure (if there is any) is painful.

In this context, formatting is completely superfluous. You may spend hours improving naming conventions or adding line breaks, but bad code will still remain bad, because—once again—of the lack of structure.

In essence, this is the difference between pretty and readable. Style makes code pretty, but this doesn't mean that it becomes immediately readable. Only (relatively important and painful) refactoring can cope with that. On the other hand, readable code can be perceived as ugly because of the lack of style, but a basic tool can walk through it and reformat it in a matter of milliseconds.

Answer 2

Not only bad formatting makes it harder to improve the code, but also, bad (or, more likely, unfamiliar) formatting may make code look as if it is logically worse than it really is.

I once caught myself troubleshooting something which I was almost sure it was a bug, only to find out that there was no bug after all, and I realized that I got the impression that there was a bug because the bad formatting of the code tempted me to believe that it was doing something wrong.

So, for the most part, no, I do not think it is a good idea to intentionally go out of your way to either make bad code look bad, or leave bad looking code looking bad.

I would be willing to make an exception in certain cases where ugly identifiers are chosen to correctly represent the ugliness of what the code does. For example, time and again I see programmers write void-returning functions called 'checkSomething()' as if they were the most natural innocent little things in the world. Of course, these functions work entirely by side effect, so they are quite evil. I would rename this function to 'checkSomethingAndDoThisIfSuchOrThatOtherwise()' in order to clearly indicate that this is an evil function which possibly does evil things.

Answer 3

I will refer you to this question, for which properly indenting the code instantly revealed the source of the bug. I will also refer you to the Linux kernel coding style, which explicitly uses 8-space tabs as a way of highlighting excessive nesting.

These sorts of coding standards aren't intended to place a permanent caution tape around bad areas of your code. They are intended to give you a visual kick in the head so that badly-structured code never gets submitted in the first place. They make certain kinds of code intentionally more difficult to read, not as some sort of punishment or permanent warning, but so you will fix it. If you have to temporarily turn off the formatting in order to fix it, so be it, but that's no excuse to leave it disabled forever.

There are two ways to write code: write code so simple there are obviously no bugs in it, or write code so complex that there are no obvious bugs in it.
—Tony Hoare

Formatting code is one easy way of making certain kinds of bugs obvious, so they can be removed. It doesn't help with all kinds of bugs, but any easy step you can take in that direction is a good one.

Answer 4

If you are beginning a refactoring project, and the code is badly formatted, you should first mechanically reformat the entire codebase to something sensible. You should make no other changes in that VCS commit.

You can make manual adjustments for local readability (insertion of blank lines, alignment of consecutive equals signs, removal of meaningless comments, etc.) later, as you refactor each logical unit, but the initial reformatting should be 100% mechanical and applied to 100% of the code.

There are three reasons for this:

• You should not reformat bad code piece-by-piece at the same time as you refactor it semantically, because that makes it harder for code reviewers to understand what the semantic changes were.

• It will be easier for you to understand the code well enough to refactor it, if it is well-formatted when you begin.

• A mechanical reformatting patch is certain to be large and boring. If it is 100% mechanical, code reviewers do not have to read through it looking for semantic changes. It is also certain to touch nearly every file, so you may as well make it all the files, minimizing the number of times anyone has to rebuild the world.

It does not matter what mechanically enforceable code-formatting style you pick (except that you should never put spaces on the inside of parentheses, because that makes the baby Jesus cry). Consistency of formatting across an entire project is far more important than any particular style. (Local readability is even more important, but you don't worry about that at this stage.)

Finally, if the code is so messy that mechanically reformatting the code breaks it, you should apply corrections before the reformatting patch. That is, your VCS history should wind up looking like

... --- [corrections so reformatting doesn't break the code] --- [reformatting] --- ...

time advancing to the right. This is so every revision of the code in the VCS is non-broken, but the reformatting patch remains 100% mechanical.

If you do not have a VCS, a code review process, a test suite, or a continuous integration infrastructure, get yourself those things before you attempt to refactor the program. In that order.