Slashdot Mirror
Defining Useful Coding Practices?
markmcb writes "A NASA engineer recently wrote about his disappointment that despite having well-documented coding practices, 'clever' solutions still made the code he has to maintain hard to follow. This got me thinking about the overhead spent at my own company regarding our code. We too have best practices that are documented, but most seem to focus on the basics, e.g., comments, modularity, etc. While those things are good, they don't directly ensure that quality, maintainable code is written. As the author points out, an elegant one-liner coupled with a comment from a few revisions ago makes for a good headache. I'm curious what experience others have had with this, and if you've seen manageable practices that ultimately offer a lot of value to the next programmer down the line who will have to maintain the code."
Simple, clever doesn't pay the bills, reliable and maintainable do. It's a cost benefit analysis: if that clever one-liner saves many man-hours of work, then probably a good idea. If it saves you two or three lines of code and all of an addition 45 seconds, probably not worth the blow to maintainability and readability. It's always a tradeoff...just have to decide which is more important in any given context, and at most companies, reliability and maintainability is king compared to a slight runtime increase or 45 seconds/3 lines shaved off.
As the author points out, an elegant one-liner coupled with a comment from a few revisions ago makes for a good headache.
A oneline that had multiple revision should be completely rewritten, because if you manage to get multiple changes in one line it surely is a mess... somwhere.
To my mind, the best coding standard is Don Knuth's Literate Programming: http://www.literateprogramming.com/. Quoting:
Incorrect documentation.
... trust .. but verify!
.. then the documentation is unreliable at best, and dangerous at worst.
And the only correct documentation is the code itself. Anything else is a opinion and should be viewed accordingly.
In other words, when it comes to reading documentation
I don't know how many times over my 30 year career that I've read documentation and started work only to find out later that it hadn't been updated. The first standard in your documentation rules should be that all relevant documentation is created and updated before code goes into production. No excuses.
If it doesn't have that
I rarely read replies, it's my opinion and if you thought about your opinion a little more, I'm OK with that.
Review the code that gets checked in (you are using version control, are you?). If it doesn't obviously do what it is supposed to do (including the case where it is not obvious what it is supposed to do), something is wrong.
You can spend any amount of time writing best practices, but that doesn't ensure they are good, workable, and actually applied. In the end, what matters is if other people can understand the code. And you can measure that by just having them do that. As an added bonus, you get more people familiar with the code.
Please correct me if I got my facts wrong.
I wouldn't ask here. Some of the worst code advice I've ever seen comes from Slashdot, especially from the idiots who think all programming is like their PC or Linux box. The worst are those that think Java is suitable for everything, or that what's good for the web is good for every situation.
I work in an environment where there are 4 programmers, of varying skill. What is "clear" to one is not necessarily clear to another, even if it is straight-forward coding. For example, I might program a loop to iterate through an array of data, extracting the important information, while another might explicitly write out each iteration, because he is less familiar with arrays. His code is quite clear - but inflexible, and subject to errors if he makes a required change to 19 of 20 copies of the calculations. Mine is quite clear, but subject to some obscuration if there's something special that has to be done in 3 of the 20 iterations.
In the case of TFA, if the author were this other programmer, he might consider my code to be "too clever".
Much was made of the use of "obscure" variables. Yes, that's one I dislike a lot, too... but I haven't made too much progress on that score in 25 years, with regard to others. When you're working on code that requires a lot of manipulation of a variable, typing a long, descriptive name 65 times is a bit of a PITA, and subject to its own bugs, when you misspell it a few times!
the op has it mostly right. There's little value in fancy show-off code that may earn a programmer some machismo points from like-minded colleagues but results in maintenance headaches down the line.
Elegance is often misused to mean terse, clever code, but that is really far from what elegance ought to mean.
I define good, elegant code to be code that is clear, well commented, self-explanatory, and easy to modify.
I'm dealing right now with some "object-oriented" Perl programs that are nearly comment-free. Sure, eventually it'll start to look familiar and I'll know where to go to fix stuff, but it pisses me off at the programmer.
I don't want people cursing and mocking my name after I've left a position, so I always strive to make my code as obvious as possible, at the cost of some high falutin' fanciness that just bogs people down and keeps a company from getting its business done efficiently.
To me, the highest compliment is when a newbie says "I just read your {Java|Perl|C++|SQL} program and I was surprised at how easy it was to understand. I just wanted to thank you for that."
it's = "it is"; its = possessive. E.g., it's flapping its wings.
Why should I write my code so that the lowest-common-denominator dull thud who took CSci just for the money can come along five years later and maintain it? I agree that good coding practices produce readable code. But too often this whole topic leads into the idea that all code should be McCode, meaning blah standards-conforming blocks, often not even approaching the best possible solution to a problem.
Sorry. Not a priority. And no, I don't care that you won't hire me. You want an assembly line slinger. And will pay accordingly.
One person's clever, obscure trick is another person's common practice.
Communicate with the other coders in your project. Write decent comments. TALK to the other coders. Cooperate and share ideas.
Optimization is fine, as long as you're "optimizing" the code to be more clear. This is a good way to redirect the energy that programmers often put into pointless performance optimization. Most understand that optimizing for one thing often de-optimizes somthing else, so they understand that you can't optimize for speed and clarity in many cases. Always looking for ways to make code clearer can become an enjoyable habit, as optimizing for speed is for many. Then you spend your idle moments eliminating many lines of code that you realize are unnecessary, bringing the code closer to its essence. My experience anyway.
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it."
Functions have their place, Subroutines have their place. Global variables have their place. Static variables, Heap variables, and stack variables have their place. Even a Go To has it's place (in rare cases).
The point is to use these tools in such a way that someone else can figure out what you're doing.
On the other hand, as Albert Einstein is reputed to have said, things should be simplified as much as possible, but no farther. If the code is obtuse, sometimes it is because the programmer doesn't understand the background of what it is supposed to do. In other words, if you understand Digital Signal Processing, someone's code may look quite elegant. If you don't understand it, it looks obtuse.
We'd all like to think that good programming is simply a matter of writing reasonably concise and well documented code. But it isn't. One needs an education on the subject being programmed. If you don't know the subject well enough, no amount of documentation is going to help you.
Nearly fifty percent of all graduates come from the bottom half of the class!
for(ss = s->ss; ss; ss = ss->ss);
Pretty standard way to traverse a linked list.
erating is fine, especially if it's in a struct, to give you the context, or when it's first declared: int erating // employee rating
risk() is also better. Since the author says its a method, it's in a class. Investment.risk() is a hell of a lot better than Investment.calculateRiskInvestment().
The author is complaining because the code is all in c. Awww - c has different style conventions. Don't try to make it look like java.
There is increasingly a meme that pretty code in a strongly typed language id OK, everything else is CRAP.
//increment i
Well no, neatly written code, in whatever, is good, as are good code and clear comments where they are needed, ie NOT,
i = i++;
Some of the worst code I have seen written is in C++, Java and C#. OO over the top, full of OO jargon, code that wanders around a thousand method calls without solving the problem, and TOO MUCH CODE, zillions of complex libraries and dependancies.
Clarity, order and simplicity are the secret of writing good code, not language, tools or methodologies, and then it can be written in anything from Lisp to C#.
I've worked with teams a large percentage of (supposedly) hotshot programmers, and on teams with a similar percentage of mediocre talent. In my experience, it is the team with the so-so developers that deliver the more maintainable code. Why? I think it's because they know their limitations and are not afraid to "talk out the process" of writing code. They ask for feedback and opinion.
What that leads to is a collaborative development process, where everyone has some idea of what the other is doing. And in these environments, for some reason, people take ownership and responsibility for their code, end-to-end.
Contrast that with the hotshot teams: they know too much to ask for help, resent questioning of their implementation, mess around in other peoples domain "because they know better", and engage in heated arguments over trivial or religious matters. And in the final analysis, the lack of cooperation leads to disfunctional interfaces between components, idiosyncratic code, and incomplete functionality or low-quality performance.
Whereas the so-so teams learn to collaborate to get things done, the hotshot teams rely on heroics: they take on too much, show exaggerated progress by declaring their code complete even though it fails edge cases, and then spend outrageous overtime fixing 'bugs' (or worse, they're too important to work on bugs, they do features, so the lesser mortals on the team get to clean up after them). Because bug fixing doesn't get scheduled up front, the schedule slides and more work hours are demanded to catch up (with decreasing effect).
To my mind, the MAIN criteria for arriving at a maintainable codebase is OWNERSHIP, OPENNESS, and COLLABORATION. A collaborative team can do more in 40 hours than a heroic team can do in 60. They just don't look so impressive.
If you post it, they will read.
While I don't understand why he did have a problem with seeing that this is a linked list traversel, I think he do have a point about fucked up naming, because in what implementation would you call the node to the next element ss instead of next, or nextNode.
As I've explained many times at work, code is not documentation.
Code only tells you what it does. It doesn't say why it was done that way...and the "why is it doing this" is really almost always the problem.
They are just as serious. Updating/"maintaining" a piece of code and not updating the comment
should be made into close to a firing offense, after the suitable number of admonitions.
The biggest weakness I find with code I review is that the programmers seem to be either
inarticulate, lazy, smug, or exhibiting Aspergers syndrome (lack of empathy), in that they forget to
include the most important comment for any method or class;
The "What the h*ll is this?" comment that explains the gist of the method/class and why and
in what contexts one should care about it.
Also frequently missing is the considerate: "Mind the low headroom" comment.
Where are we going and why are we in a handbasket?
Elegant and clever are not the same thing.
Elegant describes the algorithm or solution as using resources efficiently, having no unnecessary steps in getting to the solution, and easily readable to another programmer familiar with the subject at hand (as AB3A mentioned inre to DSP).
Yes, if we use Webster's definition, there is a certain amount of "cleverness" in beautiful, elegant code. However, when we say "clever" we usually mean cutesy-clever, like mashing several traditional lines of code into a one-liner. That's fine for a geeky contest of "who can write this using the fewest number of characters," but (as most of us agree) does not have a place in professional code. One-liners and similar stunts might be clever, but are rarely elegant, and cause headaches for later maintainers of the code.
After you've been in this business for more than 20 years without giving it up to become a manager or guru, you may find that coding practices are orthogonal to code quality.
Just do the best you can with the cards you've been given.
That is bad code, or at least, hideously idiomatic.
The variable names are apparently chosen for conciseness over legibility. The choice of 'ss' as both the loop variable and as an important field name appears deliberately confusing, especially when combined with its terseness. And the termination condition relies on the fact that a null pointer is treated as false in a boolean context - this is a non-obvious and largely arbitrary design choice in the language, and is arguably a type error.
I've been in this game long enough to recognise a linked list traversal when I see one, but I still had to read it twice to be sure. If it had read "for (currentNode = list->firstNode; currentNode != NULL; currentNode = currentNode->nextNode)" I'd have saved valuable milliseconds. If it had gone all OO and used some iterator object in a while loop, instead of that horrible for construct, even better.
You make some good points but
for(ss = s->ss; ss; ss = ss->ss)
could be better written
for(p = s->next; p; p = p->next)
Other than about the pointers, I tend to agree with the author rather than you on the points you mention. Which is fine, I suppose; we don't all need to program the same way, and it's probably good that we don't.
But my thinking is that longer, more descriptive variable names are simply part of documentation; it can eliminate the need to look through the code and find the comment tied to the declaration. (Although I personally dislike the 'initial lower case, capitalize all subsequent words' standard.) As far as skipping the brackets on single line blocks after conditionals... I think that is ALWAYS a bad idea. It's a pointless, inconsistent shortcut that can cause a number of different mistakes, just for the convenience of saving two characters. Yes, you can learn to always watch for that; but that's a learned response to a problem that doesn't need to exist. Better to avoid the problem in the first place. Adding the brackets never hurts anything, it increases the consistency of your formatting, and reduces potential mistakes... that's exactly the sort of thing that belongs in a standard.
i worked on fortranIV code that had been migrated from fortranII that had been migrated from whatever language was used on an ibm drum-memory machine;-) there were variables named for drum memory locations, 4col...i wasn't about to change those;-)
and those codes were written by the resident gas properties genius, who was still on the job;-) it was then i realized that the clarity of code is inversely proportional to the brilliance of the coder;-)
Yes, you are right. It is Slashdot's fault that you didn't preview before you posted, and it's the compilers fault when you don't #include the header file that resolves external references.
You just run willy nilly with all the rules of writing, now don't you ;-)
Guns don't kill people; Physics kills people! - John Lithgow as Dick Solomon on Third Rock From The Sun
I love how his example uses the most confusing variable names possible. Just using proper names should make it clear to almost anyone:
So you don't put up with "shit" like using widespread idioms and relying on standard language features? Sounds like a lovely place to work.
Knuth is not suggesting one should be incorrect for the sake of maintainability, understandability, or anything else. In programming, if a solution is possible, there are infinitely many solutions, a few of them are easier to understand than most of them, and understanding is the key to verification and so to the writing of correct code.
Well - one of the reasons we use Python. I'm sure some people will consider this a troll, but it's true. Some languages encourage readability, some don't. You can write horrible, awful, unreadable Python and you can write elegant, readable, maintainable Perl (Calcium was the best I've seen in a real product), but neither is encouraged and you know what you tend to get out of your programmers.
This means our code takes a bit longer to write up front than just banging it out in Ruby, but has the advantage that when we come back to it six months later it's still obvious what's going on (even if someone else wrote it) and it's easy to maintain.
Ruby's great (and a close match for Python in niche and functionality, which is why I'm using it specifically), but our Ruby guys can't do that because they love to make things as concise as possible - it allows them to show how clever they are. Perl-style magic variables and punctuation everywhere. They sometimes rewrite functionality from scratch because it's easier than deciphering their (own!) old stuff, which at least unit tests help with. On the other side, Python's indentation generally limits how much you can cram into one line without gross misuse of lambdas, which frees you from the need to even try it.
Perhaps this is self selection - maybe boring people like me who learned to value maintenance considerations (thanks to hard experience) gravitate towards easy maintenance languages, which exacerbates the effect. But I think still think your choice of language will strongly inform how maintainable your code will be; you can mandate code policies for any language, but how often (and for how long) are those actually enforced in practice?
It is shorthand, and shorthand is contextual.
If suddenly in the middle of a module to calculate the trajectory to return to earth, there is a reference to an out of scope notion like the employees rating; then a more descriptive name is very appropriate.
If it is in a context which is doing computations about employee, department and company ratings, I would hope the person would be smart enough to use shorthand; otherwise all that annoyingly redundant crap just makes it more difficult to read and maintain.
Why use sin() when it could be "RatioOfLengthOfFarSideOfRightTriangleToHypotenuse()"?
How about "YCoordinateOfPointOnUnitCircleAtAngleFromOrigin()"?
What happens, then, when a single structure needs to be contained in multiple lists?
You might try to dismiss that as not a good idea, but I'm thinking in particular about some structures in the Linux kernel where this is the case...
You assume wrong. I've had to correct code because people coming from the Windows world NEVER turn on warnings. Many don't even know how to edit a make file. If there's no clicky thingee, they're lost.
Anyone who cannot use make should not be writing c code. This is basic stuff. Then again, so is using ctags and fgrep.
Ditto anyone coming from java. It's NOT the same, the mindset isn't the same, the conventions aren't the same, and someone with 10 years of java experience is totally useless when it comes to designing in c or c++. They try to make everything look like java, and they break things. And they keep on whining about needing garbage collection or smart pointers or the whole stl because they can't even write a simple linked list.
If you can't write and manage a simple link list, a stack, a queue, and proper memory allocation in both c99 and c++, you should go back to java and leave c programming to c programmers. The guy who wrote the original article is a java programmer - look at the complaints and the references he provides.
I look at the language standard. Look at the if() statement. It is followed by one line, and a semicolon. No parenthesis. Parenthesis allow you to group multiple lines (blockify) - and its' always possible to replace any block of code with a one-liner (either a function call, a macro, or even using the comma operator).
Indent your code properly and you'll never have a problem with parenthesis - and your code will be more concise, thus easier to read.
Adding brackets increases the visual pollution,
WRONG.
It's a familiar pattern that the brain can process much more quickly and correctly. And thus much easier to comprehend than your "on-again, off-again" use of braces that introduces two patterns when there only needs to be one.
So, from a conceptual perspective, it's YOUR style that's more cluttered.
and I really hate it when people put the opening brace on its own line - its a waste of a line.
I'm sure that terabytes of storage would be wasted if you were to write all your code that way.
NOT.
Nice to see that you code to make your "folding editor" happy instead of the developers who have to maintain your code.
Folding editors can handle braces at the end of the line just fine - why can't people? Concise can be clear as well.
Well, let's see...
Good fucking God. Because humans aren't "folding editors"!!!!!
Because the human brain doesn't work the same way a computer does? Because a human brain sees and understands familar patterns faster and more correctly? Thereby making your code "sometimes there, sometimes not there" use of braces confusing at a fundamental level, and conceptually more cluttered than always using braces?
Do you REALLY expect humans and "folding editors" to work the same?
And YOU are trying to tell everyone how to write code?
Wow.
Just wow, someone who goes off on a rant about coding standards, and can't even defend his preferences without resorting to subjective characterizations like "cluttered"? And wonders why people can't understand code the way and editor can?
Even nicer to see you post that under your real name. Now I know where to file your resume, if I ever see it.
There is certainly a happy medium between clear code and inefficient code, but in general clear code wins on two counts. One is program correctness: when I'm reviewing clear code, I am more confident that it will do the right thing and won't cause a bug. The other is maintainability. How much time does a maintainer have to spend to understand the code in order to modify it?
Both of those are cost avoidance strategies. It generally costs me more to have a developer messing around with complex code than it does in run-time cycles. And a bug or a crash is horribly expensive -- handling a bug means you've got phone calls to support, testing, writing up incident reports, fixing the bug, etc. A crash is way more expensive than inefficient code.
I'm not saying "I like inefficient code." Far from it -- correctness also includes efficiency. Calling a database with a hundred individual queries from inside a loop because the developer doesn't understand how to process a multi-row recordset is horribly wrong, even though it may "look clear". But the correct version of the code doesn't have to read like a plate of spaghetti, either. Invoking the God of Efficiency doesn't magically give a developer license to write unreadable, unmaintainable code.
Again, this comment is about code "in general." You also have to consider the problem domain. I work on a business application, with a user typing input and processing it in real time. If some rarely used path of the code makes the user stand there for an extra 62 nanoseconds, it's not a problem to me. I don't care as much about things that change response time by less than a human can perceive (8 milliseconds is the threshold for the response time of a human with extraordinary reaction times.) But if I were working on the primitives inside a graphics rendering engine, something that's going to be called a million times per frame, you can bet I'd value efficiency far higher than readability.
John
And proper indentation would have saved the day - no braces required.
It's one thing python actually gets right.
And your example just shows that a lot of people can't read code. If you're not dependent on braces to "clue you in", you'll spot the error immediately.
That's a problem with YOUR editor, not mine. there's a TAB key on your keyboard for a reason.
One of the reasons people try to use 4 spaces instead of an 8-space tab is because they let their code get fugly - WAY too many levels of indent and way too long variable names - and the only way they can see it on-screen without resorting to havig their code wrap every second line is to cheat - to only indent 4 spaces instead of a hard tab.
They're too unimaginative to come up with concise descriptive variable, function, class, and method names, and to stupid to realize that when your code has too many levels of nesting, it's "broken by design" and a great place for bugs to breed..
Arrogant little piss-ant, aren't we? I'm quite comfortable using vim on a remote box via ssh. I was writing code before the terms tui and gui were even in common use - we only had text screens. We had to code by pushing the bytes uphill, both ways, in a storm (well, not quite that bad, but monochrome amber monitors and hercules video cards give you an idea?). And in your example of "no bandwidth", a hard tab is better, since it's only 1 character. So who's the nobody? According to your definition, you failed since you sure wasted bandwidth. Like your examples - but at least you're consistent in that respect.
I guess you need to learn to be more fluent in the language. Next, you'll say that the comma operator should be banned because YOU might make a mistake in interpreting how it works. Or the hook operator. Or *gasp* those evil macros. And you'll also insist that we use that piece of shit called the stl when we're trying to make a multi-threaded app, and then wonder why performance is absolute crap.
Really? Next you'll say you can't read the first line of a paragraph, then skip to the next paragraph if you realize that what you're not looking at the right code. Do you move your lips when you read?
Look, if YOU have a problem with concise code, that's your problem. Your style went out back in the early '90s. It's still taught that way because teachers are inevitably a decade behind the curve - at the minimum. Only old farts who can't adapt, the people who learned from them, people who are trying to increase their LOC count, and old koreans still put the opening brace on its' own line, or are so "un-fluent" in the language that they need to blockify everything, and can't handle something as simple as an if() without braces. What next - putting braces around the contents of case statements "just in case"? I've seen people do crap that, and it tells me one thing - they don't really know c. For them, pascal is probably a better option, since you HAVE to put begin and end ...
Visual conciseness isn't a question of saving disk space - it's a question of making the code easier to read by reducing clutter. It works - problem is, you're so used to the crutch (yes, all those superfluous braces that the standard says aren't required ARE a crutch) of the clutter that you can't do without it. You're a crip. Admitting it is the first step in overcoming your handicap.