Friday, September 19, 2008

TODO or not TODO

Any person doing software development is often faced with the same problem: you come across some code that was developed as a quick’n’dirty answer to the project timetable (for speed reasons), or some code that works in convoluted way but could not be changed (for legacy reasons). What are your choices? One choice – you have time, skills and mandate from the management to fix the problems, revamp the architecture, refactor the code whenever you feel like doing it; so you just resolve the problem there and then. Another choice – you do not have time, do not own the code, have higher priority items etc., so you just put “TODO: Fix later” comment for later resolution.

After a (surprisingly long) while, I came to the conclusion that this traditional way of handling code problems is sadly inadequate. Seeing TODO comment in the middle of debugging session is pretty familiar sight; and there is a high chance that this very TODO is responsible for the problem you are looking for. But it is there already – so what is the problem? Short answer is that TODOs are rarely done; it is almost as if it is NEVER TODO.

And the problem is inherent in the TODO comment technique itself – code comments serve for code description, not for task tracking. And once you identified that something needs to be changed – that “something” becomes a task that should have priority, should be assigned to somebody, possible resolutions should be analyzed etc. All of which cannot be done in a code comment. And code comments do not have high visibility; if anything comment is always less visible in IDE that code; most IDEs provide extensive code navigation but hardly any comment navigation (As an anecdote on TODOs visibility, in an effort to improve it, I tried alternative technique of refactoring problem method/class names by adding telltale suffix in a belief that developers would be averse to using PerformCalculationCrap method or CustomListenerCrap class. I should tell you, having repulsive name did not affect the bad code proliferation at all :).

Thus one obvious solution is to create a task entity instead of in-place comment (if you use Team Foundation Server, that task readily maps to work item with all data formatting and reporting capabilities available there); create this entity separately from the code and ideally have it linked to the code in question. While the approach is easily the best one, it will not work well for everyone: certain overhead is involved (both with creation and maintenance of task artifacts; and at the very least it requires some task tracking system) that is not always justifiable; for example, you may still wish to mark the code for later review tomorrow morning without moving away from code here and now (after all, creating task artifact will necessitate context switch).

So nowadays I try not to use any TODO comments even in the absence of task tracking system; some alternative approaches (for Visual Studio 2005/2008) are summarized below. While not the replacement of tracking code issues in task tracking systems (and to say even more – design/architecture issues always MUST be tracked elsewhere), I believe in general these approaches work better than code comments, since they provide a) better visibility (visible in compilation log) and b) easy navigation (possible to navigate between issues) and c) require intentional action to suppress (you do not need to suppress comments at all).

And yes, I am aware that I am stretching the “intended usage” paradigm in most scenarios; all I can say is make the decisions depending on your specific scenario.

Alternative TODOs in C++

In C++ (both managed and unmanaged), the easiest way I have found is to use deprecated pragma directive. Given the method name as the parameter, the pragma will generate preprocessor warning for the method. Not so easy to ignore and easy to navigate to:

In theory, this pragma ought to be used to identify deprecated functions; however, in the projects I was a part of I have never seen it used for the declared purpose and thus it could be used as TODO indicator on methods. Of course, if on your project this pragma is used for its intended purpose, you won’t be able to use it. In such case, one may use less elegant approach; for example, the one below

#ifdef BACKLOG
   #error DoSomethingWrongWay is quick and dirty
#endif
void DoSomethingWrongWay(){ ... }

Once you define BACKLOG on your project, you can easily review and navigate the TODOs. While less elegant, in this manner you can mark any line of code (whereas using deprecated you can only label whole methods).

Alternative TODOs in C#

For C# there are two alternatives that I used in lieu of TODO comments.

First approach is to use #warning pragma. This way you can mark any line of code and then easily navigate between your TODOs.


Another way is to use Obsolete attribute (much like the usage of deprecated pragma in C++ discussed above); when attributed member is used, warning gets generated. However, since the attribute is widely used for its intended purpose, I try not to deviate from this intended usage, so it is more of the “caveat” rather than TODO comment. For example, if there is some crazy method that cannot be changed right away for legacy reasons, it can be labeled Obsolete so no one will be using it in the future (since using it will generate new warning right away).

Using traditional TODOs efficiently

If you are still not convinced, and prefer to use TODO comments in your code, there are still ways to use them in a more efficient manner in Visual Studio.

Just open “Task List” tool window, select “Comments” in the “Categories” combo box – and voila! You will immediately see all TODO comments in one list so you can easily navigate between them [by the way, the comments shown in the task list are not limited to TODO; one can define any comments to be shown there by changing settings in “Tools”->”Options”, “Environment”->”Task List”].


This feature would be even more useful if all files in currently open solution/project would be scanned. Currently only open files are scanned, which means search through files would still be a better method to iterate over all TODO comments in project.


After writing this post, I thought I’d do quick search on the subject; while I was not able to see what’s the prevalent opinion, I found an interesting blog post from Ben Pryor that argues similar point of view.


And by the way, TODO comments are not specific to any development methodology. I’d assume that is more of general phenomenon.


Have a strong opinion about the subject? Leave a comment!

3 comments:

Carel Lotz said...

Great read. Btw, if you are using ReSharper you can get a solution wide list of TODO comments by using the To-Do Explorer.

eugenez said...

Carel,

Thanks for the remark. I am not a big fan of resharper, but it is nice to know.

That solution still requires having Resharper company wide.

Brownie said...

Create a TODO and use the comment pane to create a TFS Work Item on the TODO...that's how I do it.