TODO or not TODO, That is the Question

By Ran Arad

Categories: Development, SDKs

January 21st, 2009

I recently was put in charge of the development of Diameter here at RADVISION.

The other day, I got a visit from the Diameter PM. One of the customers went over the code of the last release, he said, and they found many TODOs in the code. Now they are asking questions about them and complaining that the stack is half-baked.

Ok, I said, that’s a little unpleasant. Not just a little, he exclaimed, I want a new release, without any TODOs in it. Of course, he didn’t mean that we should actually DO all the TODOs; he meant that we should just remove the forbidden word from the code.

That made me think: from the client’s side, isn’t checking for TODOs in our code a little bit counter-productive? The direct result would be that we will remove the comments from our code, and will be more likely to forget about them.

TODO this, TODO that

When a programmer writes a TODO comment, it could mean one of several things:

1. Programmer Joe realizes that there should be a better way to do what he is doing right now, but he does not have the time or the skills to do it. He leaves a comment for the future programmer that will review this piece of code to indicate that yes, there may be a problem with this code and no, there isn’t some hidden-Joe-logic behind it and yes, you should replace it, and here is what Joe thinks you should do. It doesn’t mean that the code does not work; it just means that there probably is a way to make the code work better. Examples:
   TODO: combine these into a single block
TODO: split these into different blocks
TODO: change recursion to loop or vice versa
TODO: move these into another file/module
2. Programmer Joe is implementing phase A of the project, but he already knows there will be a phase B (Here we call this tactic “making room for the air conditioner”). Phase B will add many cool new features, and these features will be integrated inside the phase A code. Wanting to save the schmoe who’ll write phase B (probably Joe himself) hours of work trying to figure out where to integrate the phase B code, Joe adds helpful comments where he thinks it would be best to add the new code. These comments do not indicate bugs at all, but instead they show forward thinking design by good old Joe. Examples:
   TODO: handle more cases
TODO: expand here
TODO: make this more generic
TODO: enable this
3. Programmer Joe found a serious bug in the code. However, Joe isn’t sure his fix is correct. Someone will have to review this, Joe thinks, just not him, and not right now. He writes a handy comment explaining what he thinks should be done to solve the problem for good. Since the fix holds, no one reaches that part of the code and the comment just sits there. Unlike the two previous types, this type of TODO could be trouble, but it’s still not a bug. Probably. Examples:
   TODO: check how we got to this state
TODO: prevent this from happening
TODO: Print a friendly message telling the user not to do this
TODO: fix - better
4. Programmer Joe is on a tight schedule. His manager is breathing down his neck, and the RFC he has to implement turned out to be a real Pandora’s Box. He won’t be able to deliver if he doesn’t cut a few corners. Joe decides to hell with all this, he needs something that will work now, and will write something that will work right later. He tries to do the best he can, and knowing that he will have to get back to this code and correct it just as soon as he goes past the milestone, he leaves comments in the code describing what he would have done if he had the time. Unlike type 2, this isn’t forward thinking, and unlike type 3 this is likely to be trouble later, especially since Joe never did get back to correct the code, now did he? Examples:
   TODO: do something intelligent instead
TODO: handle this
TODO: rewrite!
TODO: implement section 5.3 of the RFC better

TODO: a Tool or a Crutch?

I believe that the amount of TODOs is related to the maturity of the code, but not in a bad way. A stack that is still under development has a lot of room to grow into, and most of the time, these TODOs indicate the growth directions of the stack. Some tools, like Eclipse, actually detect TODOs in the code and maintain a list of project TODOs. Checking TODOs only makes sense when receiving a piece of code that the developer does not intend to touch anymore, such as outsources projects. However, outsourcing companies would know better than to leave such incriminating evidence in their code. Closed source developers needn’t worry about these things at all. As for us SDK developers – leave us our TODOs in the code, we’ll get to them. Eventually.