Slashdot Log In
Best and Worst Coding Standards?
Posted by
Soulskill
on Sun Jul 20, 2008 08:21 AM
from the buffer-overrun dept.
from the buffer-overrun dept.
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?"
Related Stories
Submission: Best and Worst Coding Standards? by Anonymous Coward
This discussion has been archived.
No new comments can be posted.
The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
Full
Abbreviated
Hidden
Loading... please wait.
Some of those examples (Score:5, Funny)
Sound an awful lot like coding in C... no bad coding practice needed...
Re:Some of those examples (Score:5, Interesting)
Now that we're talking about 'languages that invite bad coding practices'... Well, one of the best programming books I've read is 'Perl Best Practices'. Not only does it list out best practices but it tries to explain (well I might add) why you should code a certain way and why other ways aren't good to follow.
One of the habits I picked up from 'Perl Best Practices is:
instead of:
The else tends to get 'lost' when just following the closing bracket.
Parent
Re:Some of those examples (Score:5, Insightful)
"no NO!! Must not put elses near braces, my precious!" - Larry Wall
I, for one, have never understood Larry's War on Cuddled Elses. It's almost symptomatic of OCD.
Besides, how is the "else" getting "lost?" I mean, it's only two characters from the left margin! Saves lines too.
Maybe it's that I prefer reading source that is not so vertically spread out. The more code and logic on the screen, the better. Density factor.
Parent
Re:Some of those examples (Score:5, Insightful)
Density is the opposite of readable and maintainable. One of the main aspects of maintainable code is being able to find and change a single line of code quickly and without having to worry about breaking other nearby lines. Having more code per line means it takes that little bit longer to find and is that little bit riskier to change the one line of code. "Lost" in this case is only a minor delay, but when you add that delay up across several thousand bugs (i.e. any project in the Real World), it means you are wasting your time tracking down bugs in dense code rather than adding new features or working on other projects.
Pop quiz: find and remove the bracketed clause in the above paragraph. Then think about how much faster you would have done that if it had been on its own line. Then think about how much faster you could have done that to 100 different paragraphs. It may not make a difference on the projects you work on, but in something the size of a perl interpreter or a web browser, it makes a huge difference.
Parent
Re:Some of those examples (Score:5, Insightful)
Bollocks. It's a tradeoff just like every other debate in the programming world. Sure, Perl gives you the ability to put way to much code on a single line. But the opposite problem of putting loads of white space all over the place is almost as bad.
The more you spread out the code, the more you have to scroll. White space is valuable when it means something, like to separate discrete tasks within a long function. But the whole
}
else {
thing is just a waste of space. It's one line less of code I can see. I visually parse } else { instantaneously. Similarly, some compound expressions or chained method calls make perfect sense. The right place to break out multiple lines depends on the reader's own cognitive abilities and familiarity with the symbols being manipulated.
Otherwise
writing
like
this
would
be
much
easier
to
read
Parent
Re:Some of those examples (Score:5, Insightful)
I've been a professional developer for 8 years and I've never spent significant time dealing with that bug. I'd rather have the code consistent than have it one way or the other. Developers have strong feelings about these things because of our attention to detail. But the sign of a good programmer is being able to say, "It's not worth my time to even think about this". If it was a clear win one way or the other then of course I'd get on board. But it's not. Being able to see more code on one screen has value.
Parent
Re:Some of those examples (Score:5, Insightful)
My dear Doctor, readability metrics boil down to personal tastes - subjective, in other words. While perhaps what you choose is even the preference of the majority of coders, it's not mine.
Advocating for braces and the negatory conjunctive "else" on the same line is not the same as "having more code per line," e.g. more than one statement per line.
In the responses to the OP, we've if/else on three lines-
if (...) {
...
...
...
...
} else {
}
-and six lines-
if (...)
{
}
else
{
}
Whilst reading six lines is not a problem for me, I prefer the three line variant, as it means less scrolling over slow ssh connections. Thank goodness we have automagic reformatting IDEs for those who won't accept other formats.
However -- it is a sorry state of affairs that on /. the replies to an enquiry about "coding standards" end up focusing on code formatting... I'd much rather have been debating architectural design patterns as the response to "coding standards."
Positions on design patterns, over the last few years, appear to have accreted into two clusters, those for and those against. I am one of those in the "for" camp, where learning the whys and wherefores of a particular set of data structures and classes, and behaviors arising from said structures, determine architecture.
Those "against" appear, to my reading, to be willing to forgo such learning and accept whatever baked-in design patterns the platform's designers chose.
Now, on the one hand, I accept that that's the case, as there is an observable stratification of programming ability existing in the world of software developers. One most go with one's strengths, and not everyone is suited to solving the issues architecture.
On the other hand, if a developer is so inclined, there is still plenty of latitude available when structuring applications.
Finally, there appears to have been a rise in the strongly anti-design patterns camp - the learning and applying thereof, that is. Most particularly, the anti-Java, pro-Ruby/RoR camp, where seemingly one must accept the baked-in design patterns chosen by the platform's architect, without variation.
A direct descendent of that camp, the adherents to the prototype.js and scriptaculous libraries, accept the original author's patterns to the point where performance deficits due to overuse of lambda functions are not only accepted, they are ignored.
That, IMNSHO, is sad comment on the state of software development. Productivity over performance is an awkward choice, to say the least.
Parent
Re:Some of those examples (Score:5, Interesting)
Why does everybody do it that way? That is, with the opening paren on the "if" line? I have always found that difficult to read. Why not
if (something)
{
stuff
}
else
{
other stuff
}
or maybe even
if (something)
{
stuff
}
else
{
other stuff
}
This last has always seemed to me to be the most readable, most obvious way to write the code. Can anyone explain why it is not used? (other than some well-known guru prefers the other?)
Parent
Re:Some of those examples (Score:5, Informative)
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 [wikipedia.org] may be of interest to you.
Parent
Re:Some of those examples (Score:5, Insightful)
Parent
Re:Some of those examples (Score:5, Insightful)
Parent
Re:Some of those examples (Score:5, Insightful)
So we have a shop that has a whole lot of perl code and they're sent around this book as well as run perlcritic on our checked in code (which pretty much everyone ignores).
In my couple of years there I've learned a few things.
1) No one can agree on coding standards
2) What people can agree on is so watered down that it's not very useful.
3) The stuff that really causes headaches isn't bad style, it's general insanity. Hardcoded constants and poorly thought out ad-hoc parsers and general brain damage causes a million times more problems than just about anything anyone can describe in a standard.
That said, in my experience the one thing that almost aways saves me time for anything larger than a couple of lines is to use "use strict."
Parent
Space Usage (Score:5, Interesting)
Re:Space Usage (Score:5, Informative)
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 [wikiquote.org]
Parent
Re:Space Usage (Score:5, Funny)
Nah me neither... maybe it's a US thing? It's generally assumed that if you're a half decent programmer you'll follow what is already there and write clear concise code as much as possible.
One place I was in did try to come up with a coding standard, but it was abandoned as nobody could actually agree on much other than 'don't fuck it up'.
Parent
Re:Space Usage (Score:5, Interesting)
I've seen a number of private sector firms with coding standards. Some are just a few pages of common sense rules (naming conventions, etc) while others were book-length horrors created by people so incompetent that management didn't trust them to write code. I've seen requirements forbidding constants in code (correct practice was #define THIRTY_SEVEN 37, believe it or not,) and crazy Hungarian style naming conventions (nothing like several characters of line noise at the end of every function name.
My current firm's approach is pretty simple:
1. Write clear, understandable code
2. Make it look like all the other code in our system
3. Use the standard IDE
4. The entire codebase is visible to all developers
5. If you code does not conform, an annotated screenshot of it will be posted to our main developer chatroom
6. People will then discuss your code publicly
7. If the code is truly awful, a senior developer will declare it unacceptable, and delete it from the system
Parent
JSF (Lightning II) coding standards (Score:5, Interesting)
It's easy (Score:5, Funny)
My new standard (Score:5, Interesting)
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.
[Java] Use Checkstyle (Score:5, Informative)
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.
developer buy-in (Score:5, Insightful)
Without developer buy-in, whatever coding standards you come up with will be useless. IOW, ask your developers to create the standard together.
Re:developer buy-in (Score:5, Funny)
Why don't you just give them all giant Q-tips and play the Star Trek fight music every time they meet?
Surely that would be at least as productive as asking them to all agree on coding standards.
Parent
badness abounds in visual basic (Score:5, Interesting)
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
Re:badness abounds in visual basic (Score:5, Funny)
I love abbreviations, not being native English speaker I used to think POS was "piece of shit" since its usually being used when talking about software (failed software).
(Or the always common "IANAL", well good for you buddy, but we are talking about legal issues here - the arse pounding is for when you get behind bars)
Parent
Correct focus (Score:5, Insightful)
.
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.
My workplaces' lovely standards... (Score:5, Interesting)
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...
Re:braces (Score:5, Interesting)
If Kernighan, Ritchie [wikipedia.org], and Torvalds [linux.no] does it like that, who am I to do differently.
Parent
Re:braces (Score:5, Insightful)
It doesn't really matter what you do, so long as everyone on the team does the same thing.
Parent
Re:braces (Score:5, Interesting)
I don't like seeing opening braces sharing a line with anything else at all, unless the block gets closed on the same line.
if(something)
{
do_something();
}
else
{
do_something_else();
}
Yeah, it takes a bit more space, but I find it a lot easier to match blocks up when the braces are indented the same amount.
Parent
Re:braces (Score:5, Insightful)
The code is important. The braces are syntactical sugar.
Parent
Dual widescreens (Score:5, Funny)
For one thing, they is grammatically plural.
--MarkusQ
Parent
Re:braces (Score:5, Interesting)
Well, as long as we're admitting that "readable" is an entirely subjective experience.. I'd have to say that I would find that notation less intuitive than the "} else {" construct.
It's too similar to consecutive 'if' statements which of course, breaks the logic.
Also, extending your notation logic fully results in:
if ( condition )
{
statement1;
}
else
{
statement2;
}
Which, although a waste of lines, is less confusing than your example.
Parent
Re:braces (Score:5, Insightful)
There are other good reasons for putting open braces on their own line. The biggest is that most coding conventions have a maximum line width. If you have an 81-character line, you need to break it. When you are scanning down the code, all you see is a line at one indent level followed by another line indented more - you need to read the entire line to tell whether it's the start of a block or not. With braces on their own lines you can tell just by visual pattern matching where every block starts and finishes.
While I'm in holy-war territory, I'll also chime in on the tabs versus spaces argument. The tab character has a well-defined semantic meaning. It means 'indent this line / paragraph by one tabulator.' If you are indenting anything there is only one character you should be using - tab. It does not, however, have a fixed width, and should therefore never be used anywhere other than the start of a line or for aligning two lines. If you have to split a function across two lines, you should indent it like this:
Then, no matter whether the person reading your code thinks tabs should be 1 or 8 characters wide, arg1 and arg2 will always line up. Sadly, vim does not have the ability to distinguish marks used for indenting and marks used for alignment and so this has to be done manually.
Parent
Re:braces (Score:5, Interesting)
Ding ding ding - we have a winner.
Real coders write code that you can take a ruler from any given close brace and draw a vertical line right up to the matching open brace, every time. Everybody else gets fired.
Lines are cheap. Time added trying to figure out an obfuscated code structure because somebody wanted to save lines (ie, put the open brace on the same line instead of doing the above) is expensive.
Parent
Re:braces (Score:5, Interesting)
That's the way I've always done it, but it seems like most published coding standards don't like it.
To reiterate: matching braces should line up horizontally AND vertically. It may "waste" lines, but the code is a lot more clear.
I also, except in some cases (like some class getter/setter methods, where they're just all one after each other and it's obvious what they are), use braces even if it's a single statement that's being executed within. I don't see why code should be inconsistent just because it's a single statement that's being executed.
I often get berated by other programmers for that style, but the only time I've applied for a job and had to stand at a white board and write code, and then describe my coding style, they seemed to appreciate it.
Parent
Re:braces (Score:5, Insightful)
Bollocks.
Draw your line from the closing brace up to the first line with any text on it, that line is the start of your block.
Having your opening braces on an empty line might be more aesthetically pleasing but has zero advantage in making the code clearer.
Either way, the most important thing is to have everyone do it the same way, every time.
Parent
Re:braces (Score:5, Interesting)
I've always found this to be clearer.
Drawing a line up to an opening brace doesn't tell you anything but the constraints of the code block, you then have to take another step to figure out what kind of code block it was.
If you draw the line up to the first text line then you'll not only know the constrains but you'll know immediately without any further inspection if it was a for block, a while block, a function, etc.
Parent
Re:braces (Score:5, Funny)
Messrs Kernighan and Ritchie and their no-necked associates would like to have a word with you out back.
Parent
Re:braces (Score:5, Interesting)
Besides, this is what syntax highlighters were made for. There are very good free ones on every single platform for a reason
Parent
Re:braces (Score:5, Insightful)
In my short experience in the workin' world, I've come across some pretty spectacularly awful implementations of everything under the sun, from production boxes in shambles to network cables wrapped around sweaty water pipes to 2 year old production code passing GET strings straight to the SQL server unmodified (yay nonprofit sector!), and compromising code for a quick and dirty webpage so I can get things running just a bit faster is 1) the least of my worries and 2) a metric fuckton cheaper than new servers (yay nonprofit sector!).
Parent
Re:braces (Score:5, Funny)
True - but at least it keeps thousands of otherwise dangerous PHP developers safely occupied.
Parent
Re:braces (Score:5, Informative)
Like many Python evangelists, you seem to have a remarkably limited experience of computer languages.
Here's a language with no braces:
Here's another:
Here's another:
Here's another:
or, more extensibly,
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.
Parent
Re:braces (Score:5, Funny)
Parent
Re:Keep it simple! (Score:5, Insightful)
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.
Parent
Re:Keep it simple! (Score:5, Insightful)
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.
Parent
Re:Keep it simple! (Score:5, Informative)
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
Parent
Re:Keep it simple! (Score:5, Funny)
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?
Parent
Re:Keep it simple! (Score:5, Funny)
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.
Parent
Re:Keep it simple! (Score:5, Funny)
Try multiplying it by -1 and see if your stack is large enough.
Parent
Re:Well hungarian notation... (Score:5, Informative)
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 [datahedron.com])
Parent