subcommanderblog.wordpress.com

Subcommander Development Blog

Extract Implementer

leave a comment »

In my current work of tdd’ing any new subcommander code (clean code of course :-) I want to create some test code that leads me to a class that compares the old working copy status and new working copy status of a single directory to notify only items to the gui that have changed.

I hope to achieve a much smother and faster working copy view this way. The updates will be fed to a new QAbstractItemModel implementation that provides the data for the working copy view. Currently I have about 50 tests that exercise the item model implementation and its helper classes.
I guess this is a ridiculous number of tests for a single interface implementation and a couple of helper classes!? It is not even fully implemented yet. I wonder if forcing any code line by test code is a little bit to much…
I also wonder if there are tests I could remove because “their” code (that is the production code I wrote because of the test) is now hit by other more interesting tests.

Back to wc (working copy) status. The wc status information of any single item is represented by a svn::WcStatus object. When I created the class a few years ago I didn’t want to have it a lot of setter methods. So I passed an svn_wc_status2_t* pointer to its constructor and it extracts all information from it. svn_wc_status2_t is struct from subversions api which contains all the status info.

WcStatus (const sc::String&, svn_wc_status2_t*);

Great or better not great, this is bad for testing. I don’t want to setup an svn_wc_status2_t struct just to create a couple of WcStatus objects for testing. I need a WcStatus without it. Tdd’ing the new code has to wait. First I need to break some dependency.

Typically you would do an extract interface here. Or even better, as I want to keep WcStatus as the interface name, it is used everywhere in the code, extract implementer (Working Effectivly with Legacy Code).

This is easy and straightforward:

  • copy WcStatus.h to WcStatusImpl.h
  • move WcStatus.cpp to WcStatusImpl.cpp
  • rename the class to WcStatusImpl in WcStatusImpl.h and cpp
  • #include and derive WcStatusImpl from WcStatus
  • Remove the constructors and fields from WcStatus.h

No problem, easily done even without ide support. At this point it compiles again but it does not link. There is one annoying step left. You have to make all methods pure virtual in the interface. It is a bit boring to add the virtual and = 0 to 25 methods…

But with this rather small refactoring I have improved my code:

  • There is now one dependency less in my old legacy code which allows me to implement WcStatus test doubles

It is only a small improvement, but it is an improvement.. :-)

Advertisements

Written by hauner

Sunday, 8 November, 2009 at 18:44

Posted in subcommander

Tagged with

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: