I wish more developers would consider this. I recently took on a website maintenance project for a transportation agency - the system is a custom-designed CMS that is so poorly implemented it requires a programmer to add a new page! I spent MANY hours rewriting the fare system just to handle a simple fare update.
Documentation? Practically non-existent. Comments in the code? yeah, right. Good coding practices are not even attempted (such as naming and use of variables and functions). And talk about spaghetti code - it’s almost impossible to follow.
What makes it worse is the original developer’s site says such things as “an award winning development firm”, and they’ve received awards like “Entrepreneur of the Year”.
They’re really good at marketing. They’re really lousy at delivering quality code. But, unfortunately, the quality of the code doesn’t matter at all to the businesses that hire such developers - so those of us who actually care about such things are going to have to keep dealing with this crap for a very long time.
BTW - I don’t usually do this, but in this case, the code was SO bad, I actually quit the project.
“A very good habit I was taught in university was to comment first, code later. What approaching a programming task, write out the comments for the code that you haven’t written yet. Helps you plan and you end up with detailed comments when you’re done.”
That’s why I love languages like Ada where you are more or less bound to write the interface first. If you do your comments in the same way, obvious design flaws show up, before you even started to implement the code.
Well, in the current C-Code I am thrown in to maintain it, it seems to be tending the other way around. First the code was written and then some interface (.h) was hacked together. The only comment in the header files usually is the copyright notice, everything else is just the list of subroutines that happen to be in .c file. I’m always thinking what a waste, nobody would steal that code anyway.
The most problematic thing for the maintenance programmer with such code is that he is forced to look at the code (where are sometimes the functions are even explained to some extent) to figure out what a function does and then you tend to ignore any comment anyway, because the code is much easier to read.
But beware, it can make a huge difference between what a function actually does and what is was supposed to do. Reverse Engineering at its best and that’s where I definitely agree with Jeff - you need smart people to do that kind of job and not some junior programmer who has barely touched any real world code yet.
Oh, last but not least, about the most “funny” thing: Some stupid prick (pardon my french) sometimes even documented the interface that way:
/*
Input : int Some_Parameter
Returns: TRUE or FALSE
*/
BOOL Something (int Some_Parameter)
Hell, yes. I can see that from the prototype. But what does it actually DO?
Never install the Java Decompiler, JAD, and look at the library code you are dependent on.
Just don’t.
If you do, however, never run the FindBugs utility (which I recommend for all lazy code reviewers btw) against the decompiled code.
Just don’t.
Or you will cry.
I’ve picked up lots of code in my time, and the hardest stuff was Java “o-o” code, because everything is buried so deep and bugs very hard to find. Oddly, Ruby is easy to debug once you’ve understood how metaprogramming works and it is an o-o language. But most of the time you just write code to do what you need to do and write tests for that small thing.
UML etc. etc. - guys you are working at the wrong level. UML blah … once you’ve gone past the initial system design it’s another thing to maintain and it won’t be. Throw it away and make the code base clean. I’m very old fashioned - I tend to start from a clear data model and view the rest as just a layer on top of it. I don’t care about the class model - it’s usually not worth worrying about and a lot of code is procedural code hidden in static classes anyway - be honest, now. This is particularly true if you are using an o-o to relational mapper - that’s where all the complexity is and you have decided that you want the mapper library to do the work. Data matters, the rest is fluff.
I also recommend finding a formatter for the language you are using if you are trying to understand someone else’s code. It’s amazing how many bugs you find when the formatter changes the indentation to reveal what the compiler/interpreter will actually do with the code.
Read Fowler’s refactoring book, even if you are not a Java programmer, it repays close study.
I have to maintain certain programs just working on my own pretty much and did actually meet the programmer. I asked him about documentation/comment his excuse was the code documents itself. Great
there is so much useless/old code that is now defunct here comes a rewrite
Jeff, I really enjoy reading some of your stuff, but man, you got to learn one thing: if you don’t have a good idea for a blog entry, don’t submit a bad one, PLEASE.
I agree with the article’s sentiment but not all of these comments.
Comments can be useful but if they’re wrong then they’re mis-leading.
So they aren’t just good++great as some people here are stating.
Don’t write comments that can get out of date easily. And don’t write undocumentation as that’s just a waste of everyone’s time and makes me angry (growls at GhostDoc):
i.e.
// gets somename
public string SomeName
{
get{}
}
I’d rather the comments just give the intent of the code.
// this class is here to make problem x
// architecturally simpler because of abc
public class SomeClass
{
}
As opposed to any implementation details (apart from maybe commenting hacks, oddness in lower levels/hardware or seriously odd semantics).
// for some crazy-ass reason we
// have to wait for a bit or otherwise
// the next call wont work
Thread.Sleep(100);
DoNextCall();
We have metadata to comment with as is. Variable names, method names, class names. These should be the primary forms of commenting your code.
The biggest crime for unmainability is copy + paste and people not tidying after heavily modifying classes.
Commenting out code is another one, we have source control! Just delete the code if it’s no good otherwise there is too much noise to easily make sense of whats going on.
Stick to the coding guidelines, do not duplicate code or create parallel logic paths that basically do the same thing but create two maintenance spots and refactor as you change. Those principles are much more important than comments.
Boy, I hate the picture. Now I need to skim porn sites to get that off my mind.
The best way to make your code easy to maintain is to keep it simple. Ideally, so simple it does not need any comment whatsoever (through good choice of variable and function names, no “smart” techniques, function that does only one thing and does it well, abstraction, modularity etc.) Use comment only if it adds value. If you can speed-read (accurately) a piece of code without looking at the comment, then delete the comment. If not, then keep the comment. But if you cannot understand the code even after repeated read, delete the code and write a better one. Make sure you have the authority to do this, though.
’ Below is the constant set to ensure we treat all customers the same
’ if we assume customers will not complain we will be disappointed,
’ however if we assume they will complain, but then they do not
’ and perhaps are even happy, then we are pleasantly suprised.
’
On the use of break/continue… Like goto they should be used sparingly. The upside to them is that they are more specific than a goto, i.e. you can’t really create infinite loops with them and fun stuff like that, but they do help create spaghetti code.
I remember way back in programming 101 the first thing my instructor said about break and continue was “don’t use them”. Usually when you find no other way, it’s a clear sign there’s something wrong with the way you set your code up and it would probably work lots clearer and better if you refactored the code a little so you don’t need to do all sorts of tests in the middle of it.
Flipping around a program in a speed reading code review, some of the code looked very familiar. Then I saw:
/* If you do not understand the semantics of the communications return codes or the protocol as used by the partner program, do not change this page of code */
:
code
:
/* END OF: If you do not understand the semantics of the communications return codes or the protocol as used by the partner program, do not change this page of code */
The program was, in part, an nth generation mutated clone of something I had done a decade before. The comments had successfully inflicted sufficient FUD. The code between the comments had not mutated at all.
I am currently porting some SPEC benchmarks to Mac OS X and even the SPEC code isn’t too well written. Some “maintenance” on it would definitely make 'em run faster than better hardware.
@Tim Almond
That’s not my point at all, for me this quote means “write quality/maintainable code even if it feels like a hassle or you are going to be murdered” and my point was that changing “be murdered” by “clean a spaghetti code mess” was more motivational.
If we write our code perfectly, then do we really have to comment it? Doesn’t this just give lesser developers the ability to pick up where we left off? Where is the job security in that?
