Last week, I wrote a script that would ìscrubî twitter, match the messages based on a set of criteria and format that criteria to a specified output. We needed this script for auto-generating ìEngineering Journalsî for our senior project because our instructor has required it. I wrote the utility and did some basic refactoring until I got to a point where I found a design mistake. This design mistake was something that I needed to rectify but didnít have the time to fix, at the time. So, I accepted the technical debt and decided to revisit it later. Luckily, I am able to revisit it now.

Since bad design decisions are a part of programming and refactoring these designs are good practice for software craftsmen, I decided to chronicle the changes. Here are the tools and technologies that I will be using for this project:

  • TwitterScrubber, a utility for scrubbing and parsing twitter messages
  • Ruby, TwitterScrubber runs on ruby
  • Twitter4r, a popular Twitter API library for Ruby applications (RubyGem)
  • E Text editor, a clean, powerful text editor for Windows
  • Git/GitHub, source control system that will allow me to separate the changes from the previous source control.

And here are the things I want to change:

  • Add unit tests
  • Refactor Twitter::Client to be injected for testability purposes
  • [Feature] Add support for XhXm syntax in time parsing

Iím ashamed to say that my first go at this was devoid of unit tests (and TDD, obviously). I kicked myself in the ass for doing that as soon as I realized what I was doing. With TDD, I would have avoided that design mistake.

The design mistake is located in the following code:

require "rubygems"
require "twitter"

class TwitterScrubber
    attr_accessor :formattedValues

    def initialize
        @values = []
        @formattedValues = "Data not found"
    end

    def Scrub(config)
        client = Twitter::Client.new(:login => config["username"], :password => config["password"])
        timeline = client.timeline_for(:user) do |status|
            if( config["comparer"].Valid?(status.text)  )
                @values << { :user => config["username"], :message => status.text, :date => status.created_at }
            end
        end

        return self
    end

    def Format(formatter)
        @formattedValues = formatter.format(@values)
        return self
    end
end

Do you notice it? Twitter::Client is not injected into this class. This makes for some serious testing headaches. First, you cannot properly isolate this class. Also, each test counts as a strike against your total API calls. And, it introduces strong coupling of Twitter::Client and TwitterScrubber. How do we solve this? Well, TwitterScrubber is a business logic object. It brings together several parts of the application (and is, indeed, the focal point of the application) and delegates. Unfortunately, this design attempts to do that and also manage multiple Twitter::Clients. Instead, each TwitterScrubber should scrub what is passed to it.

So, we refactor TwitterScrubber to this:

require "rubygems"
require "twitter"

class TwitterScrubber
    attr_accessor :formattedValues

    def initialize
        @values = []
        @formattedValues = "Data not found"
    end

    def scrub(client, comparer)
        client.timeline_for(:user) do |status|
            if( comparer.Valid?(status.text)  )
                @values << { :user => status.user.screen_name, :message => status.text, :date => status.created_at }
            end
        end

        return self
    end

    def format(formatter)
        @formattedValues = formatter.format(@values)
        return self
    end
end

This allows us to write this test file:

require 'test/unit'
require 'flexmock/test_unit'
require "../TwitterScrubber"

class TwitterScrubberTests < Test::Unit::TestCase
    def test_ThatFormattedValuesIsInitializedToDataNotFound
        target = TwitterScrubber.new

        assert( target.formattedValues == "Data not found", "formattedValues not initialized properly" )
    end

    def test_ThatScrubCallsTimelineFor
        target = TwitterScrubber.new

        client = flexmock("temp")
        client.should_receive(:timeline_for).and_return([])

        target.scrub(client, nil)
    end

    def test_ThatScrubCallsValid
        target = TwitterScrubber.new

        client = flexmock("temp")
        client.should_receive(:timeline_for).and_return([flexmock("temp")])

        comparer = flexmock("temp")
        comparer.should_receive(:Valid?).and_return(false)

        target.scrub(client, comparer)
    end

    def test_ThatFormatChangesFormattedValues
        target = TwitterScrubber.new

        formatter = flexmock("temp")
        formatter.should_receive(:format).and_return("Test")

        assert( target.format(formatter).formattedValues == "Test", "Should have formatted values" )
    end
end

So, now, weíve eliminated the bad code smell and created a better, more maintainable TwitterScrubber. Also, we have tested the class and now I am confident in how my own code works. I donít need to test the Twitter::Client because the Twitter API Ruby library has tests for itself.

Now, Iím free to continue to write tests and add the new feature. The new feature will only modify one class, as was a goal of this sort of design. The new tests will affirm the new functionality and hopefully provide automated tests for any future additions to the scrubber.