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.
To my mind, the best coding standard is Don Knuth's Literate Programming: http://www.literateprogramming.com/. Quoting:
"Quality" can be both objective and subjective, and it seems this post is leaning towards the latter in terms of what it's getting at... while I believe in having formalized guidelines of some kind, I've found the best way to seek out and improve the elements that can't be easily quantified is through (drum roll) code reviews/walkthroughs. It seems like these are rare, at least where I'm at, and it's hard to get buy-in when you're talking about contractors charging by the hour, but in my opinion a single quality code review can save TONS of time down the road and is necessary for projects over a certain size. Also, read "The Pragmatic Programmer" and "Code Complete" for some of the best guidance on this topic.
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 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.
"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!
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.
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?
No, so that you can come along five years later and maintain it.
The author of TFA seems somewhat confused and inexperienced.
Find free books.
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.
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.
A "hotshot" programmer will aggressively abstract. A mediocre programmer -- one who keeps his nose to the grindstone and is ever mindful of deadlines -- is more likely to "inline" constants (i.e. not abstract them at all) or even repetitive blocks of code. When ccde and constants are "inline", code is much easier to read because you're not constantly looking up in other files what the abstractions are.
Supposedly, both abstraction and locality aid maintenance.
The truly master programmer balances abstraction with locality. And takes into account who will be maintaining and the expected lifespand of the code. If the code is to be retired in two years, why bother abstracting? The general rule of thumb is that abstracted code has to be utilized six times in order to make the investment worthwhile.
When it comes to abstraction, the master programmer abides by the words of the Federation President, "Let us redefine progress to mean that just because we can do a thing, it does not necessarily mean we must do that thing."
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)
We deal with this by using something that can be described as code editors.
These are people that edit raw code committed by other programmers and refactor it in order to comply with a set of standards. Editing may entail just changing a few variable names, but it can also be almost a complete rewrite of the code. Of course, if code rewrites become the rule rather than the exception, something is wrong and some serious talks with the original programmer will be necessary.
As long as the code editors are knowledgeable people and agree among themselves about the specific style, best practices and architecture to be used, this can greatly improve the quality and consistency of the code.
Five years, feh. I can't barely recognise the code I wrote 3 months ago.
I regularly look at some code and say, "who wrote this crap?". After checking the history I find it was me.
meh