Slashdot | Best and Worst Coding Standards?

[VLSmooth]

Slashdot: Best and Worst Coding Standards?

Interesting comments, although this might (should) be preaching to the choir.

Make it "cut and paste" friendly, and as small as possible.

That's a really bad idea. Cut and paste causes code cloning, which is among the most difficult maintenance problems.

Code should be designed, when possible, in small chunks (methods, functions, etc.). This keeps the need to think about refactoring to a minimum, since the code is already factored. Well factored code has many other benefits, including easier-to-write unit tests and better understandability.

I maintain software that was originally written by someone as a prototype and eventually given production status. 4 years later, I am still pulling bugs out that relate to code cloning. Think of the guys who will maintain your software, please.

the most egregious bug I think I ever introduced was due to code cloning. It was awful. I did not bother to properly refactor (hey it was 12 years ago) and as a result we ended up with diverging clones that needed to be separately maintained.

I found the following tools post very interesting, but don't have any experience with them.

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

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

And probably others

Of course, there are also the indent wars:

I really don't get this obsession people have with putting braces on separate lines.

Block scoping is perfectly well indicated by indentation and blank lines are valuable for dividing functional blocks of code.

When you go and put all the braces on separate lines it totally kills the visual effect of the actually empty lines. Then you'll have to go and start using multiple blank lines for separating things and before long half the screen will be empty and mostly empty lines.

Hmmm perhaps that's it, maybe this is a scheme for people who don't like looking at the code.

It doesn't really matter what you do, so long as everyone on the team does the same thing.

For Java, there's Checkstyle:

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.

A big picture comment follows, emphasis mine.

Coding guidelines are typically justified because, as it goes, most of the time is spent fixing bugs in existing code than writing new code. The guidelines are needed because it helps others to come up to speed quickly while they try to figure out the code in which they have to fix the bug(s).

I think that is the wrong focus, as it tends to reinforce incorrect behavior, i.e., the writing of buggy code.

Coding guidelines should focus instead on the techniques that help reduce the number of bugs in code. How is that done? It takes someone (typically a senior person) looking at the the bugs that have been found in the code, categorizing their cause, devising a way to prevent those bugs from occurring, then putting that into the guidelines.

Keep the focus of the guidelines where it should be: to increase the quality of the software.

[Jonathan]

http://lxr.linux.no/linux/Documentation/CodingStyle

Some interesting thoughts.

A big deal for me is choosing

Code: Select all

 class Car: public vehicle {
       public:
          go_fast();
}; 

class Bus: public vehicle {
       public:
          go_fast();
}; 

void Car::go_fast() {
    //implementation A
}


void Bus::go_fast() {
    //implementation B
}

over the all-too-common but inherently unmaintainable:

Code: Select all

void go_fast() 
{
    if (Car) {
        //implementation A
    } else if (Bus) {
        //implementation B
    }
}

[quantus]

ummm, in java you could just put go_fast() in an interface that vehicle implements. You cannot put code inside an interface. Go Java.

Or you could just slap the authors and send them back to a course on OOD/OOP

[quantus]

http://lxr.linux.no/linux/Documentation/CodingStyle

My thoughts on this:
It's meant for C. These ideas aren't necessarily appropriate for other languages. Perl for example is not strongly typed and it can therefore sometimes be useful to code the type into the var name.

Personal note: Tabs being 8 chars just looks bad to me and I prefer 4.

[Jonathan]

The point of the second is that it is not, in fact, OOP.

What's wrong with C? 8 seems like a lot, I agree, but he has an interesting take on that.