Thursday, May 21, 2009

Indentation Wars

Ever suffer from tab rage? You know, you're fixing or refactoring someone else's code, and the logic is solid but the indentation is all over the place. So much so, that your appreciation of what the code does is over-shadowed by how bad its looks.

At the recent project kick-off, we had occasion to debate what coding conventions should be put in place. A few general principles occured to me, crystalized in the following rules:

  1. No endless debate.
    The first rule of code style is: you do not talk about code style.
    The second rule of code style is: you DO NOT talk about code style!
    Endless amounts of time can be burned splitting hairs over obscure rules like the requirement to signal abstractness with Abstract or Base in the class name. Believe me, I've been there. The trick is just to agree that once the rules are in place, they're treated as being eteched in stone. You don't have to like them, just follow them. Yep, I do have a slightly dictatorial streek.

  2. Enforce the rules early and often.
    Do not rely on folks to follow the conventions out of community spirit. Neither is it a good idea to wait for code reviews to smoke out style issues. The rules must instead be enforced from the get-go. With an iron fist, I tells ya!
    Where IDEs such as Eclipse are used, ensure continuous builds flag style violations as you type them. Enable checking via checkstyle and/or PMD as part of all routine automated builds. Fail the build if any sytle violation is found, no ifs or buts. Make it so that resistance to the style police is futile.

  3. Rules are made to bent.
    What, doesn't that contravene the preceding principles? Well yes it kinda does, but sometimes needs must. When a boatload of pre-existing code is being imported into a project for instance. Or when the style checker doesn't understand some new language feature, such as PMD's tendancy to trip over variadic arguments. In these cases, use //CHECKSTYLE:OFF comments or @SupressWarnings("PMD.theRuleName)" annotations to selectively exempt the least amout of code. Selectivity is the key here.

  4. Skip checks altogether when appropriate.
    A tad contradictory again, but style checking does not come for free, and there is no point wasting time auditing code you already know to be clean. So when you build a fresh checkout for the first time, or when running the same test from maven multiple times without any code changes, the checkstyle.skip and pmd.skip properties are your friend.

No comments:

Post a Comment