tisdag 18 mars 2008

Stop documenting your code!

I've been reading this interesting book for a while now, The Pragmatic Programmer by Andrew Hunt and David Thomas. I was actually planning to blog about it first when being finished, but my mind is just going crazy about some stuff I've read recently, and thus I feel the need to blog about it now!

The book is based on "tips" that can easily be memorized and used again later when you put your programming hat on. One of these tips, which especially caught my attention, and which is going to form this post, describes the following principle:
DRY - Don't Repeat Yourself
This basically means that you should not repeat knowlegde. Further the book states that:
Every peace of knowledge must have a single, unambiguous, authoritative representation within a system.
Well, you say, this is trivial, you should not write two peaces of code that does the same type of work. If you for example have two functions being very similar, then you use refactoring and extract the common code into a new function. If you have two classes representing two different branches of some common structure, then you extract what is common, put it in a new base class, and let the two classes inherit from this new base class. But this you probably already know, so I will not bore you with this anymore.

Instead, think about the following for a second: being a programmer, does repeated knowledge only apply to code? What about comments? What if you comment a function, doesn't this mean you repeat knowledge? For example, look at the following code:

// Sets the name of the player
// @param name The new name of the player to set
void SetPlayerName(string name)
mPlayerName = name;

This is obviously repeated knowledge, right? Both the name of the function and the implementation perfectly describes what it does. Is there really a need for documentation here? I know from experience that in many cases you write comments like this, just because you should write something.

So what about a more difficult case:

// Returns the name of the currently loaded level
// or "" if none is loaded.
string GetCurrentLevelName();

If the comment didn't state that "" is returned when no level is loaded, then you could actually think it should throw an exception when no level is loaded, or that you would get the name of the previously loaded level (or if you are using C# or Java that null should be returned). But once again, knowledge is repeated. Just look up the implementation and it will state exactly what it does.

string GetCurrentLevelName()
if (mCurrentLevel != null)
return mCurrentLevel->mName;
return "";

Fact is, the implementation is the only "valid" documentation, because it's the only one you will know for sure is true. How many times haven't you written a function, documented it, then later in the development process realized that this function needs to be changed. You change the code but are a bit too lazy to update the documentation. Suddenly you have invalid knowledge! Isn't it better then just to remove the comment and let anyone who wants to know what it does open up the implementation and actually read the code?

If you find it too hard to find out what a function does, you probably need to refactor the function by giving it a more descriptive name, extracting larger independent blocks of statements, rename your variables (members, parameters and local) to something more descriptive (i.e. don't use "temp"!) and so on. If this doesn't work, then you are either bad at reading others code (which is a good practice in order to become a better programmer!) (I hope you know how to read your own code!? :) ) or the code you are looking at is really bad and should be removed as quickly as possible. You don't want to maintain such code.

Reading code should be like reading a book! You don't want the author of a book to add a lots of comments everywhere describing everything a second time in other words. The same goes for writing code, you should not add comments repeating what you have already written in another language.

So, my tips to you is this: refactor your code, and remove those comments!

2 kommentarer:

Peterh sa...

Commenting code isn't as much about pinning down knowledge, as it is communication. And when it comes to communication, redundancy is almost never a problem :) and you can never be clear enough. A well written method header and comments inside the method or function, is a much faster way of communicating the ideas and behavior of a method - for the people who maintains your code. And for the sake of misunderstanding. I can see two major cases here. One, there are comments, but they are somewhat wrong because the code changed but not the documentation. The second case is no documentation at all. And then there's the situation for the reader. He/She might misunderstand correct documentation (happends all the time), but can then try to read the code. Or, he/she reads the comments and they are faulty, and then tries to read the code. And in the third case, with no comments, the user must read the code, and, as well as before, the user may misunderstand or simply don't get it.

And another thing, without documentation, I might misunderstand the code I'm reading and think that I find defects or suboptimal solutions.

And then there's the case with APIs that you don't have the code to... Ok ok, thats another story.

But hey, you can't possibly mean that I shouldn't document my Assembly code, do you? Super haxxor might read and understand ASM as I understand English, but the vast majority don't.

Saying that documenting/commenting code is wrong, is saying 'I don't care if other people are able to read my code, and if they don't understand their lazy and dumb'. And thats lazy and dumb :-)

Peterh sa...

Hey, here is my comment in my blogg instead ^_^