Slashdot Mirror
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?"
Sound an awful lot like coding in C... no bad coding practice needed...
Seven Days with Ubuntu Unity
I've worked where we were supplied a full IDE and a 17" CRT, and the coding standard forced so much white space vertically that you had to basically remember all the code.
I can't stand seeing the closing brace of an if statement sharing a line with an else, like so:
if( condition ) {
statement1;
} else {
statement2;
}
I've always found the Joint Strike Fighter's coding standards document an interesting read. It is available from Bjarne Stroustrup's website (pdf)
This sounds like a fairytale but I work for a very large IT firm which is very well known. Serious company doesn't mean good however.
In certain files (not all apparentely) all constant variables have to be declared globally. We are talking C++ here.
Think what you want, but I don't like it. The reason for the variables placements are so "that they will be easy to find".
First off, I'd suggest printing out a copy of the /. comments, and NOT read it. Burn them, it's a great symbolic gesture.
My new standard comes from a 1950's comp sci book.
"Programs consists of input, output, processing and storage."
Lose focus of that and the project will be late, over budget and most likely broken in ways no one will understand for years.
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:
Of course, we let developers to add suppresions for the 1% of false positives. In fact, there are very few suppresion rules set.
Build a tool even an idiot can use and only an idiot will want to use it. -S.O.B.
Without developer buy-in, whatever coding standards you come up with will be useless. IOW, ask your developers to create the standard together.
Je ne parle pas francais.
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.
All my liberal friends think I'm a conservative, all my conservative friends think I'm a liberal.
One of my friends worked at a place where you'd have to insert whitespace to place certain elements (variables, evals, etc.) to begin at a specific col in the code within every line; in addition to standard indentation of the line. At one level, I see the concept, but seriously - highlighting and search is made to solve the same problem there.
He left that job quickly.
Returned Peace Corps IT Volunteer
At my first industry job (internship) I quickly realized their coding standards were very different from mine. So I spent the first 2 hours after lunch on day 1 with GNU Indent, working up a script that would convert my code into the company's coding standard for indentation.
Let the tools do the work for you. Just don't forget to run 'indent' before you check in your code.
Also found I prefix in .NET really bad pracitce for marking interfaces like ICollection, what about when You decide turn interface to abstract class?..
Well. The whole point of having interfaces is allowing the implementation of a certain method set to the world, which later can be used in your APIs using polymorphism. If you later decide to break the contract and make an interface a class, then probably a name change (made also automatically in tools like Eclipse or NetBeans) won't be any worse.
As for the Hungarian notation, the standard form is indeed worthless. But we tend to use simple maximum three letter abbreviations of Swing components, to know that we are taking the username from txtLogin and listening for pressing btnOK. Code is more often read than written and this quasi-Hungarian style actually works pretty well.
In fact, having interfaces named like "IPasswordProvider" is something very similar. It enables easy reading of your code and when you want to make a change, you instantly see that this type is an interface, therefore you cannot instantiate it directly, but you can implement this interface in any arbitrary class you may already have written, etc. Plus, Sun coding standards encourage you to name interfaces in a passive adjective way like "Serializable", "Comparable", etc. To comply with this format is not very natural for interfaces like "IPasswordProvider" or "IModelContext".
Build a tool even an idiot can use and only an idiot will want to use it. -S.O.B.
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 worked for a company that was destroyed by a bad coding standard.
This was a small company, that, back in '96, was awarded the contract for a POS application for a regional store chain, with back-office servers that would be updated nightly by modem.
The guys who ran the company weren't programmers (though one of them knew enough to be dangerous); they were technical salesmen. They were also big fans of Microsoft, with "MVP" plaques on the walls, and every employee except me having Microsoft certs.
I worked for them part-time while also working for another company. I advocated Unix (mostly BSDI and SunOS at the time), and always argued with them about why Unix was better (technical superiority vs. potential for big profits).
When their big project was well underway, they brought me in to do the communications part of it, where the POS terminals would contact one of several servers by modem each night ("why not just ethernet them together, get a dialup PPP connection, and use IP? the interface is so much more reliable..." Request denied).
The app was Visual Basic, with third-party "custom controls" for things like talking to modems. My part went fairly smoothly, and I was eventually asked to help out with the main application, which was suffering from unexplained crashes. When I looked at the code, I found something... strange.
For error handling, they had elected to use a program called "VB Rig" (the name came from the rigging used on sailing ships, which prevents a sailor from falling to his death. Sometimes.) What this program did was to examine the source code, and then add error handling boilerplate at the start and end of each and every function. It inserted the exact same error handling code into every function.
Because the error handler had to be all purpose, it was about 20 lines of code per function - sometimes much larger than the regular part of the function. And, worse, because it was the same for every function, and it made use of the same variable names, that meant either every variable had to be global, or you'd have to declare the ten or so standard variable names at the start of every function (they opted for the "everything is global" approach).
Which led to things like this (forgive the syntax errors, it's been years since I've touched VB):
On Error goto my_data_file_read_function_VBRIG_TRAP
open MyDataFile for writing ...
goto my_data_file_read_function_VBRIG_CLEANUP
my_data_file_read_function_VBRIG_TRAP:
on error 101 'Permission Denied
delete MyDataFile
resume
on error 102 'File Not Found
MessageBox 'Cannot read ' + MyConfigFile
resume
my_data_file_read_function_VBRIG_CLEANUP:
blah blah
my_data_file_read_function = SUCCESS ' return
As you see, the error handling code - which had to be exactly the same for every function - made use of global variables (names like DataFile1, MyFile1, UserName, etc.) to figure out what to do for each error. That meant, that if there was any possibility you might have a "File Not Found", you had to expect the filename where that might happen to be in a particular global variable - say, MyFile1 - and hope that the calling function wasn't using that name too, for the same reasons.
Naturally, files were being created and deleted at random, and the programmers often spent hours on the phone with the customer trying to figure out why the Access database had disappeared *again*.
I asked if we could just write the error handling by hand, and use appropriate local variables; or take the standard VBRig error handling and trim out the lines that weren't relevant for a particular function (as subsequent VBRig runs wouldn't touch its code region if it saw that it had been customized).
Request Denied. "This is our coding standard. We carefully reviewed the options before making the decision to use t
.
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.
On the strange side is the omission of vowels on functions and varible names to save text space (it's not required, but should be consistent for similarily names objects). It sounds weird, but is still quite readable.
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
My Karma: ran over your Dogma
StrawberryFrog
Make it "cut and paste" friendly, and as small as possible.
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.).
Wait.. are you trying to say that copying the same lines of code over and over again must be avoided? So tell me genius, how else would you implement such a function without copying?
You just got troll'd!
The difference between ""cut and paste" friendly code" and "use Cut and paste" is the difference between "bake a nice cake" and "get obese and prone to illness".
Code that is well-factored can be (incidentally) suitable for cut-and-paste, but using cut and paste is the problem.
My Karma: ran over your Dogma
StrawberryFrog
Each language or environments have their own features, and require different standards. One of the big things is that hopping from one project/company to the other should be intuitive (something thats basically forfeited in environments such as C++, and accepted as to not be possible, more or less) When the language is mainly controled or orchestrated by a single entity (Sun for Java, Microsoft for .NET, etc), people should set aside their own opinion and stick to the main guidelines (and complete them for areas where the main design guidelines do not cover).
So for example, in .NET, stick FxCop or Code Analysis on, disable the rules that aren't relevant to your company (ie: localization rules on an app that doesn't require them), and stick to that. C++/VB6/Java/Smalltalk conventions have no place in there, so leave em out.
Same holds true for any other environment. Don't use VB6 conventions in Java/C++ (I know the thought alone seems mind boggling, but I've seen it countless of times....ugh!), and so on.
The biggest issue with conventions is just that: people take conventions that are specific to one language/environment, and don't realise that they are, so they port them everywhere else, so you have a program in language X that literally looks like if it was written in language Y (and takes twice as much code, is twice as buggy, is half as readable...)
multiple return statements were off-limits
Despite the fact that it's not part of the coding standard where I work, I have a few coworkers who take this to the extreme. They surround every single function they write with: ... } while(0);
do{
And then, inside the "do" block, they just put "break" in any place where they would have otherwise put "return." It drives me insane; they insist that having a single exit point from your function makes it easier to debug, but I just don't get it. I've never even seen them use gdb, anyway, so I think that abusing "printf" is their idea of "debugging"...
One thing in our coding standard that I do like is that all variables that store units must have a unit specification at the end of their name -- in other words, all frequencies might have "Hz" or "MHz", distances might have "m" or "mm", times have "sec" or "msec", and so on. This is really helpful in my field -- it's not uncommon for me to open up a file that I've never looked at before and need to make modifications to it, and if the units everything things are stored in weren't immediately obvious, I'd have to go track down somebody and ask them. The annoying thing here is when people decide not to follow this standard because they think it should be obvious...
Karma: Terrifying (mostly affected by atrocities you've committed)
Duh, you so need to learn about this little thing called structured programming, which can totally help cut down on code duplication like that crap.
Here's a hint:
See? Much easier to understand than your spaghetti code, and much more maintainable too.
If it's assembler then write pseudo ADA comments which bear no resemblance whatsoever to the badly commented code following - Bonus points if the pseudo code itself has bugs...
If it's Delphi code make all units UNITx, all forms FORMx and all variables equally inanely named - if it's good enough for most Delphi books then obviously it's the right way to do things
Avoid function prototypes - if it was good enough for Brian and Dennis it's good enough for anyone
Overload operators in surprising and pleasing ways, preferably so that "-" does bitwise set inclusion
Use macros extensively (without ()() because everyone knows only losers need them).
Mix tabs and spaces indiscriminately
Pick at least *two* styles for braces - Bonus points for gratuitously adding them where they aren't needed - (to really make the reader happy use the "{" on next line style here)(extra points if you are mixing tabs and spaces)
Use if (1==x) , (x==1) and just plain old if (foo) randomly to add variety
Write big huge case (switch) statements spanning 5 pages because no one would possibly understand dispatch tables
Seriously though, if you're programming anywhere you're expected to conform to the local customs, wacky and out of it or not. It's part of the adaptability expected of a programmer...
Andy
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.
I'm sorry, but that code goes against our coding standard by having non-const parameters and a goto. I suggest the following before submitting your changes:
-1, Dense
Try multiplying it by -1 and see if your stack is large enough.
Now why would anybody do this? I've always assumed code like this was basically what inexperienced people would use:
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.
:/- spoon(_).
For one thing, they is grammatically plural.
--MarkusQ
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)
When you copy code you also copy whatever bugs exist in that code. If something needs to be reused several times then it should be made into a function.
Crap, you're right! Fortunately there's an easy way to fix this :
:%s/x+=b/x = addition(x, b)/
You just got troll'd!
How about just x = a * b;
How about just WHOOOOOOSH!!
You just got troll'd!
Even in languages that recurse properly that'll overflow on big numbers. To not overflow in properly recursing languages, you need:
When doing maintenance on someone else's code, use their style, even if it is one you hate.
Personally I write stuff like
if
(
a > b
)
{
doSomething();
}
else
{
zomg();
}
Then I charge the client 5 * the number of lines in a all source and header files.
"Ubuntu" -- an African word, meaning "Slackware is too hard for me". - stolen from Dan C alt.os.linux.slackware