Monday, May 23, 2011

Lot's todo

All developers are familiar with the infamous "//TODO" comment in code. It means "I know this code is not finished and will need work", or "I have deadlines and Fred's code is broken so this is the best that can be done until he fixes his library".  The problem is that these thing can linger forever. I know, I'm guilty of it myself.

Who hasn't seen things like:

//TODO fix after June

(But that comment was checked in 2 years ago)

//TODO see Fred for a fix.

(Fred doesn't work there anymore, by the way).

Amongst the many code quality techniciques and policies, such as unit testing, treating warnings as errors, and using static analysis tools, I propose a system for dealing with a "TODO" - something that has been identified as needing rework eventually.

In my Android Game, I have recently started using a class to denote such beasts. Behold:




//TODO optimize/minimize allocations....
if (DebugConfig.FailTodo)
{
DebugHelp.todo("Optimize Allocation");
}



Now, whenever I feel the need to write a //TODO comment, I accompany it with a construct like above. This is in Java, in C++ or C# you could get a little fancier. I check a static final "FailTodo" flag outside of the call to give the compiler an optimization hint. This is the closest thing to an #ifdef you will find in standard Java. If the flag is false, as it is normally, the compiler just optimizes it all away, so you don't pay a price when you're not using it.


The body of the todo method simply throws an exception:

public class DebugHelp 
{
public static final void todo(String todoText)
{
throw new RuntimeException("Failed TODO: " + todoText);
}
}


A C# variant of the same thing would be:

public class DebugHelp 
{
[Conditional("FAILTODO")]
public static void Todo(string todoText)
{
throw new InvalidOperationException("Failed TODO:" + todoText);
}
}

A little cleaner, since I can call Todo directly without the flag check, and the compiler will optimized it out if the "FAILTODO" is not defined at the project level.


On more example for the C++ playas in the house:

#ifdef FAILTODO
#define TODO(x) ASSERT(false, "Failed TODO: " x);
#else
#define TODO(x)
#endif

(or insert your own assert mechanism, every good project has one :) )


The real epiphany of this for me was not the trivial implementations I showed, but the general concept of a checked "todo". These are things that really should not be in comments, where they are summarily ignored. Before shipping, or when I enter the next cleanup/optimization pass, I can deal with these issues. You could also hook this into an automated testing or regression system. 



-P

P.S. (Ignore what I actually do in my implementation of 'todo' or my "exception correctness", they're contrived examples for this post.)


No comments:

Post a Comment

I welcome you're thoughts. Keep it classy, think of the children.