Procedural methods in OO code

A discussion on the Yahoo XP group regarding refactoring long methods into smaller ones coincided with me writing the code below. This is from my most recent test class, which is 180+ lines and growing:

    public void testThatRefundingTwoToysBoughtOnDifferentDaysDoesNotCauseError() throws Exception {

        goBackTwoDaysFromRealDate();
        String transactionRefA = buyToy();

        goBackOneDayFromRealDate();
        String transactionRefB = buyToy();

        goBackToRealDate();
        String refundTransactionRef = refundItems(transactionRefA , transactionRefB);

        verifyNoErrors(refundTransactionRef);
    }

I’m glad I did this, because now when I go back to it after finishing this quick post I’ll know exactly what I was doing. I don’t have to wade through the 180+ lines to find out which bits have the TODO comments in. Plus, when the next person comes to look at the weird configuration file I’m about to change, they’ll be able to tell from a glance at the code why I removed the lines I have. They can work with the change instead of around it.

This exercise made me realise that there seem to be a lot of very long tests around. In case it’s a prevalent thing and not just a statistical glitch, please let me beg on behalf of the campaign for legible tests – split ’em up! Make them readable. Your tests define the behaviour of your application. If you’re racing along in an Agile environment then they may well be the only documentation you have.

(Did you know that in JUnit you can write an abstract test class, and any tests in it will be called when the subclass test is run? You did? Did you know that you can also do it with behaviour classes in <plug>JBehave</plug>?)

Of course, one has to be careful when refactoring tests. It’s easy enough to tell if your test fails as a result of refactoring. It’s harder to see if it no longer tests the same thing as before, and passes regardless. For instance, when writing abstract JUnit test classes, be careful not to put tset or should instead of test at the beginning of the method, unless you have a particular fondness for the word “Doh!”.

Update: Time travel is now parameterized too:

    public void testThatRefundingTwoToysBoughtOnDifferentDaysDoesNotCauseError() throws Exception {

        goBackInTime(2);
        String transactionRefA = buyToy();

        goBackInTime(1);
        String transactionRefB = buyToy();

        goBackInTime(0);
        String refundTransactionRef = refundItems(transactionRefA , transactionRefB);

        verifyNoErrors(refundTransactionRef);
    }

Thanks to the XP group for hints and tips on how to make vaguely similar bits of code turn into almost identical bits of code (Summary: put all the similar stuff together and move the dissimilar stuff somewhere else.)

This entry was posted in Uncategorized. Bookmark the permalink.

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