Best and Worst Coding Standards?

An anonymous reader writes "If you've been hired by a serious software development house, chances are one of your early familiarization tasks was to read company guidelines on coding standards and practices. You've probably been given some basic guidelines, such as gotos being off limits except in specific circumstances, or that code should be indented with tabs rather than spaces, or vice versa. Perhaps you've had some more exotic or less intuitive practices as well; maybe continue or multiple return statements were off-limits. What standards have you found worked well in practice, increasing code readability and maintainability? Which only looked good on paper?"

Comments

Re:braces

[__aapbzv4610]

yes, putting the else on a new line makes it a bit more readable; you know that line marks the beginning of an else:

if( condition ) {
    statement1;
}
else {
    statement2;
}

reading this kind of code tells you that there is an else condition there. having a leading closing brace makes you have to read into the line to see what's going on. I know it's 2 characters, but when scanning code for structure, it helps to have it on a bew line.

[Java] Use Checkstyle

[Lucas.Langa]

If you are using your computer right, it does not only enable you to do things, it does the boring things for you, automatically.

Checkstyle is one of the tools in a company toolkit that is often overlooked but in fact VERY handy. It enables you to define a ruleset for your source code, finding stuff which is incompatible with the coding practice in your company/team/project/whatever. Moreover, you can stick it into Eclipse using the free Eclipse-CS plugin, so it will automagically mark the places which need to be change. Last but not least, you can put Checkstyle as an Ant task in your building environment (and in your continous integration toolkit) so commited code that does not conform certain standards does not build.

As for the rules themselves, we've found these to be the most successful:

• limit the length of the method to be visible on one 1280x1024 screen; if it's longer it's probably handy to extract smaller methods from it - which will be far more easy to read

• similarly: set a file length limit (e.g. 1000 lines should be enough for everybody)

• an upper limit on the allowed number of method parameters seems to be a good idea as well

• ensure that new code is commented by marking structures which could be javadoced but aren't

• any naming and formatting convention is better than no convention; Checkstyle can use regular expressions to validate type, variable and other names. It can also check for whitespace constraints, new lines, etc.

• avoid star, redundant and unused imports

• enforce or forbid the usage of the tabulator character to have all code clean no matter where you open it

• warn on redundant modifiers

• enforce the usage of braces everywhere (e.g. disallow if (something) action; ): no misformatting will hide a trivial but dangerous bug

• a major one: Checkstyle can warn if it discovers a typical coding problem (of course this has some false positives but better to be on the safe side): double checked locking, lack of hashcode when overriding equals, switch fallthroughs, illegal catches and throws, lack of super.clone() or super.finalize(), etc.

• you can also constraint the number of returns from methods (we found it to be very useful to set this to 1, using a result variable everywhere else - it's far easier to get hold of the code flow then)

• we also constraint the if depth and boolean expression complexity to manage the cyclomatic complexity - also for easier reading

• it's useful to make Eclipse clean up your code on every save: to add "final" where it can, to fix imports, format the code to the specified form, etc.

Of course, we let developers to add suppresions for the 1% of false positives. In fact, there are very few suppresion rules set.

Re:Keep it simple!

[StrawberryFrog]

There are several tools that can detect cut and paste code:

Simian: http://www.redhillconsulting.com.au/products/simian/
PMD: http://pmd.sourceforge.net/
DuplicateFinder: http://www.codeplex.com/DuplicateFinder

And probably others

Standards are important for shared projects

[yelvington]

Some programmers think they should be able to do anything they want.

That might be OK if you live in your parents' basement and code for yourself, but in the real world it's a bad (and selfish) idea.

Strict adherence to a standard is helpful in code review and in cases where a component is taken over by a new maintainer.

This is always important, but it's particularly important in a genuinely open, community-driven project.

The Drupal project is an example. It has a coding standard derived from the PEAR project that applies to any code submitted for inclusion in the core.

Contrib authors are encouraged, but not required, to follow it. The good ones do.

The Drupal Coder module does a very good job of nagging at you until you get the formatting right, and also helps with code migration and updating when the API changes. And it finds some common bonehead mistakes that can create security issues.

Adhering to a standard doesn't have to be painful. Using a properly configured text editor helps. There is good support for Drupal standards and conventions in OpenKomodo and the commercial Komodo IDE, as well as some other editors.

Re:braces

[Haeleth]

Like many Python evangelists, you seem to have a remarkably limited experience of computer languages.

Here's a language with no braces:

IF condition THEN
    statement1
ELSE
    statement2
ENDIF

Here's another:

(cond (condition statement1)
      (t statement2))

Here's another:

if condition then begin
    statement1;
end else begin
    statement2;
end;

Here's another:

if condition then
  value1
else
  value2

or, more extensibly,

match condition with
  | true -> value1
  | false -> value2

And the list goes on. Maybe you should try learning some other languages. Broaden your mind a bit. There's a lot out there that isn't Python or C/C++/Java. Some of it is quite interesting.

no multiple return statements?

[dysfunct]

maybe continue or multiple return statements were off-limits.

Now why would anybody do this? I've always assumed code like this was basically what inexperienced people would use:

if(condition) {
// 100s of lines of code
return true;
}
else
return false;

Why not just return immediately if any basic conditions or assumptions are not met and prevent that completely unnecessary indentation? The only advantage I can see is that you could miss the return statement when reading the code.

Re:Space Usage

[LordOfLead]

The Linus says:

"If you need more than 3 levels of indentation, you're screwed anyway, and should fix your program."

from http://en.wikiquote.org/wiki/Linus_Torvalds

Re:Some of those examples

[Zwicky]

From a personal perspective that happens to tie in with the coding practices at my last company:

The second example (GNU style) I have found to be quite cumbersome in writing, unless tabs are set to 2 with braces indented once and content twice (company mandated four with one indent for content in the block), in which case I would be frustrated with the extra keypresses involved.

The first example (Allman style) I used to use until I moved over to Kernighan-Ritchie style (opening brace on same line as control statement, with functions (and classes in OOP languages) braces the exception; these are written in Allman style). This allows me to scrunch more onto the screen vertically.

FWIW I never liked the '} else {' style of elses but at the same time, I never found it difficult to read so it was never a real issue. It makes sense to me to have the else begin at the same column as the if to which it belongs.

This may be of interest to you.

Re:Well hungarian notation...

[hedronist]

Strangely enough, Hungarian worked quite well for the problem it was originally intended to solve.

I worked at Xerox in the late 70's and my manager was Charles Simonyi, inventor of this notation. The project was BravoX (grandparent of MS Word) and was written in BCPL. BCPL basically has one type: integer. How that integer is treated is purely a function of how you reference it. E.g. fooFirst>>fooNext means "use the variable 'fooFirst' as a pointer to a structure of type FOO, one of whose elements is (from the naming convention) a pointer to some other FOO." Whereas fooFirst+1 adds one to an integer and (almost certainly) yields an invalid point that bill blow up when you try to use it. (It's been 30 years since I wrote anything in it, so I probably screwed up the example.)

Since there was only one type, the compiler didn't/couldn't perform type checking. Hungarian was a way of putting the type into the name of the variable so that the programmer could perform visual type checking. There were 9 of us on the project and the consistency/readability across the code base was impressive. Any of us could go into anyone else's code and almost immediately see what was going on.

I still use a light variant of it in my own code, but when in someone else's code I try to stick to their naming/formatting convention.

Like so many good ideas, it worked well in its original context but became twisted out of shape when used for something never intended/envisioned by the original developers (even though the person doing the twisting was, in fact, the original developer!). Another example of this is the Third Eye Software symbol table format I created for my debugger, CDB, but which was then used and abused by Mips to create a complete piece of crap. What they did still has people swearing at me 20+ years after the fact. (More on this at Third Eye Software and the MIPS symbol table)

Re:Keep it simple!

[BlaisePascal]

Even in languages that recurse properly that'll overflow on big numbers. To not overflow in properly recursing languages, you need:

int multiply(int a, int b, int c = 0)
{
    if (b == 0) return c;
    if (b < 0) return multiply(-a, -b, c);
    return multiply(a, b-1, c+a);
}