Belfry Images

The Extract Method refactoring is a basic refactoring technique where a part of a method is extracted out into a new method. This is used to:

• Simplify a complex method,
• Promote DRY, and
• Make testing easier

Here’s my Union-required contrived example:

interface IFoo { }
interface IFooRepository
{
    IEnumerable<IFoo> GetFoos();
}

class FooProcessor
{
    IFooRepository FooRepository { get; set; }
    public FooProcessor(IFooRepository fr)
    {
        this.FooRepository = fr;
    }

    public IEnumerable<IFoo> GetProcessedFoos()
    {
        foreach (var foo in this.FooRepository.GetFoos())
        {
            // Do some processing on foo
            // ...
            // ...

            yield return foo;
        }
    }
}

The inside of the loop in FooProcessor.GetProcessedFoos() might suit being extracted into a new method:

public IEnumerable<IFoo> GetProcessedFoos()
{
    foreach (var foo in this.FooRepository.GetFoos())
    {
        yield return this.GetProcessedFoo(foo);
    }
}

public IFoo GetProcessedFoo(IFoo foo)
{
    // Do some processing on foo
    // ...
    // ...
    return foo;
}

Now I can test the foo processing code without having to worry about setting up an IFooRepository mock just to pass in a single IFoo. Unfortunately the FooProcessor class still has the IFooRepository dependency, which I could work around by passing null to the constructor:

var fooProcessor = new FooProcessor(null);
// test fooProcessor.GetProcessedFoo(...)

But that isn’t very clear, and when there are lots of tests and dependencies things can get very, very silly. Extracting the method into a separate class can solve the problem:

public IEnumerable<IFoo> GetProcessedFoos()
{
    foreach (var foo in this.FooRepository.GetFoos())
    {
        yield return new GetProcessedFooMethod().Execute(foo);
    }
}

public class GetProcessedFooMethod
{
    public IFoo Execute(IFoo foo)
    {
        // Do some processing on foo
        // ...
        // ...
        return foo;
    }
}

Now the method can be tested directly without concerning FooProcessor‘s dependencies:

var getProcessedFooMethod = new FooProcessor.GetProcessedFooMethod();
// test via getProcessedFooMethod.Execute(foo)

Not long ago I started using Ninject to inject dependencies into the day job’s in-house system. Up until then all the dependencies in the system were strongly bound, often created within a method. Not surprisingly this made testing this old code (also something only started in the last year) quite difficult. So using Ninject was a pretty painless way of loosening these strongly bound dependencies (and the way I thought about dependencies).

I pretty quickly found an issue with this approach. Say I’m integration testing B.MethodToTest() in the following code:

interface IInterfaceForA
{
    string DoSomethingInA(string s);
}
class A
{
    [Inject]
    public ISomeInnerDependency SomeInnerDependency { get; set; }

    public string DoSomethingInA(string s)
    {
        this.SomeInnerDependency.DoBlah(s);
        // ...
    }
}

class B
{
    [Inject]
    public IInterfaceForA A { get; set; }

    public string MethodToTest(string s)
    {
        this.A.DoSomethingInA(s);
        // ...
    }
}

So class B has a dependency on IInterfaceForA (which Ninject will inject with an instance of A), and class A has a dependency on ISomeInnerDependency. Now say that the implementation of ISomeInnerDependency that gets injected hits a database that doesn’t exist during the test and dies. I’m testing B.MethodToTest() and its interaction with A.DoSomethingInA() but I don’t know beforehand of A‘s dependency on ISomeInnerDependency, so when I run tests I’ll get unexpected failures which will depend on how I’ve set up Ninject prior to the test:

• if the test setup for Ninject doesn’t inject ISomeInnerDependency at all, I’ll get NullReferenceExceptions
• if Ninject is injecting the production implementation of ISomeInnerDependency, any type of error at all from just about anywhere in ISomeInnerDependency‘s implementation’s call stack

Either way I’m getting errors from code that I didn’t even know about beforehand. I can resolve this by doing something like injecting a mock into A.SomeInnerDependency but there’s no way I could have known about the dependency before spelunking into the A class.

If the method I’m testing happens to have five dependencies, and some of those dependencies have more dependencies, my workflow devolves into: Compile, Run test, Watch fail, Sort through call stack, Find dependency, Figure out some way for Ninject to inject some harmless stub, Repeat for next nested dependency. I’m spending more time finding, stubbing, and injecting dependencies then I am writing tests and production code combined.

There is an argument that the code I’m testing is too complex and should be refactored. Fair enough, and my new code is much cleaner now that I’m thinking about testability now, but:

• This is an integration test
• The code base has three years worth of (my) beginner crap thrown in
• I should be able to inject dependencies to existing code to increase testability and increase my efficiency, not to have to struggle with the DI framework and seemingly random run-time errors
• There is one developer, and this is an in-house business application that services 15 staff and 300 clients. No resources, no time.
• An oversight meant that Ninject didn’t inject some dependencies that made it into production. Nasty.

In short, Ninject ended up causing more problems than it solved. Time to take the power back:

interface IInterfaceForA
{
    string DoSomethingInA(string s);
}
class A
{
    public ISomeInnerDependency SomeInnerDependency { get; set; }

    public A(ISomeInnerDependency someInnerDependency)
    {
        this.SomeInnerDependency = someInnerDependency;
    }

    public string DoSomethingInA(string s)
    {
        this.SomeInnerDependency.DoBlah(s);
        // ...
    }
}

class B
{
    public IInterfaceForA A { get; set; }

    public B(IInterfaceForA a)
    {
        this.A = a;
    }

    public string MethodToTest(string s)
    {
        this.A.DoSomethingInA(s);
        // ...
    }
}

I’ve done two things here. First I dropped Ninject. Then I added constructors to A and B so that the dependencies are explicit. To create an instance of B I now have to provide the dependencies up-front rather than configure them beforehand (and hope I’ve covered everything):

var b = new B(new A(new InnerDependencyMuchoDatabase("database configuration...")));

In the test for B.MethodToTest() I can just pass a stub to A:

var b = new B(new A(stubInnerDependency));

So I’ve been getting my console-fu on since moving to Mercurial at work, and finding ways to get more efficient. One thing that bugs me is having to spend time grabbing the mouse to search Google via a browser. I’ve also wanted to spend some time with Ruby, so to acheive both goals I wrote a little script that uses the Google AJAX Search API (via a REST interface), which returns search results as a JSON object. The full code is available as a gist on my GitHub. There are a couple of interesting (for me anyway) parts of the code that I’ll go through now.

The request/response is done via a HTTP GET responding with a JSON-serialised object graph. A query string is built up and parsed into a URI object (using the same reference), then the request is made and parsed from the text JSON representation into an associative array (the result of JSON.parse):

require 'net/http'
require 'json'
...
queryURI = "http://ajax.google....."
queryURI = URI.parse(queryURI)

data = Net::HTTP.get_response(queryURI)
data = data.body
data = JSON.parse(data)

I’ve used both pre- and post-fix conditional structures:

if command.to_i.to_s == command && (1..results.length).include?(command.to_i)
  url = results[command.to_i - 1]['url']
  puts "Opening #{url}"
  `start #{url}`
  command = ''
end

page += 1 if command == '+'
page -= 1 if command == '-'

Rather than using a not operator like ! the unless operator can be more fluent and easier to understand:

command = '' unless ['+','-','q'].include?(command)

A problem with C#’s foreach loop syntax is that there is no easy way to include an index short of rolling your own and incrementing in the loop. Usually it is easier to resort to the less pretty for (var i = 0; i < ..; i++) syntax. Ruby includes a variant of Array::each that takes care of the index:

results.each_with_index do |result, index|
  # ...
end

And of course writing two line breaks is very fun:

2.times { puts '' }

All up it was very educational spending some time building something a bit significant in Ruby, and the result should hopefully be pretty fun to use. Once again hit up GitHub for the full code.

Anonymous objects are a way to create strongly typed objects without having to declare a class or struct in C# 3.5 and above. Declaring an anonymous object is easy:

var breakfast = new
{
    Cereal = "High fibre",
    Coffee = "Latte",
    Bacon = "Crispy"
};

In the scope of the object’s declaration, accessing the properties of breakfast is as simple as breakfast.Cereal. However accessing the properties outside of that scope is not as simple. Say we have an object ben with a method Eat(object meal). Within ben.Eat() we can’t do something directly with meal.Coffee because Coffee isn’t known in ben.Eat()‘s scope.

Getting a property value using reflection is pretty basic but takes a couple of steps. There are much more advanced uses of reflection that allow access to hidden properties, fields and methods, but picking public properties is probably the easiest case. The following method returns the value of a public property of an object. This could be used on an anonymous object, or on any other class of object.

using System.Reflection;
...
object GetPropertyValue(object o, string propertyName)
{
    var prop = o.GetType().GetProperty(propertyName);
    if (prop == null) return null;
    return prop.GetValue(o, null);
}
...
var cereal = GetPropertyValue(breakfast, "Cereal");
Assert.That(cereal, Is.EqualTo("High fibre"));

prop is a PropertyInfo object that lets the value of a property be retrieved via reflection. The same method can be used to get a dictionary of [property name, value] from an anonymous object:

prop is a PropertyInfo object that lets the value of a property be retrieved via reflection. The same method can be used to get a dictionary of [property name, value] from an anonymous object:

IDictionary<string, object> ObjectToDictionary(object o)
{
    var dict = o.GetType().GetProperties().ToDictionary(
        prop => prop.Name, prop => prop.GetValue(o, null)
            );
    return dict;
}

This sets up a dictionary where the key is the name of the property, and the value is (ahem) the value of the property:

var breakfastDictionary = ObjectToDictionary(breakfast);
Assert.That(breakfastDictionary.Count, Is.EqualTo(3));
Assert.That(breakfastDictionary["Coffee"], Is.EqualTo("Latte"));

Using anonymous objects and reflection is a bit slower to execute than using strongly-typed objects, but once the methods are in place to access the properties, the savings in developer time can be great. Leaving more time for breakfast. Speaking of which, I’m late for work.

This is pretty basic and has been around for a while but I always forget about it until it’s too late. From C# 3.0 onwards we’ve been able to declare methods that extend a class. There’s no magic, the method doesn’t actually get added to the class and can’t access the internals of the class, it’s just a shorthand way of making a static call.

The extension method is declared in a static class:

public static class PimpMyStringExtension
{
    public static string Pimp(this string s)
    {
        return s + " is pimped out";
    }
}

Adding the this keyword to Pimp‘s string s argument is what indicates that Pimp is an extension method to the string class. The Pimp method can now be called against any string:

Assert.That("test".Pimp(), Is.EqualTo("test is pimped out"));

As long as the PimpMyStringExtension is accessible to the consuming code, the String gets the Pimp() method added. "test".Pimp() actually just calls the static method under the covers, which is actually still an option:

Assert.That(PimpMyStringExtension.Pimp("test"), Is.EqualTo("test is pimped out"));

Extra arguments can be added to the extension method as well:

 public static string Pimp(this string s, int i)
{
    return s + " has been pimped with " + i;
}
...
Assert.That("test".Pimp(7), Is.EqualTo("test has been pimped with 7"));

On Monday I was home with manflu and, facing a terrible lack of sympathy from a considerably pregnant wife, decided to start learning Ruby. I finished the Ruby Koans, a set of failing tests that walks you through aspects of the language. I now think that, after ‘Hello, World!’ and some basic algebra, TDD would be the best way to teach and learn programming in general.

Last night I found my way to CodeKata and started at Kata 2 for instant gratification. Kata 2 is to implement a binary search algorithm. My first try was just a linear search, which passed the tests, but wasn’t a binary search and for very big sets of test data is impractical. My second try is a recursive binary search:

def chop(needle, haystack)
  # Special cases
  return -1 if haystack.length == 0
  if haystack.length == 1
    return 0 if haystack[0] == needle
    return -1
  end

  # find the middle element
  mid = (haystack.length / 2).round
  return mid if haystack[mid] == needle

  if haystack[mid] > needle
    #needle is in first half
    return chop(needle, haystack[0..(mid-1)])
  else
    #needle is in second half
    result = chop(needle, haystack[mid..haystack.length])
    return result == -1 ? -1 : result + mid
  end
end

My third try converts the recursive algo into an interative one:

def chop(needle, haystack)
  depth = 0

  while true
    # Special cases
    return -1 if haystack.length == 0
    if haystack.length == 1
      return depth if haystack[0] == needle
      return -1
    end

    # find mid point
    mid = (haystack.length / 2).round
    return mid + depth if haystack[mid] == needle

    # trim haystack based on which end to follow
    if haystack[mid] > needle
      #needle is in first half
      haystack = haystack[0..(mid - 1)]
    else
      haystack = haystack[mid..haystack.length]
      depth += mid
    end
  end
end

I found that while I was writing the different implementations I had a lot of love for these little chips of Ruby. Maybe it was the very expressive syntax (such as the Yoda-esque return X if condition) or just the nature of TDD itself. Either way I’m really enjoying Ruby.

In my last post I compared calling Ninject inside a loop during object contruction vs calling Ninject once and using a factory class to return a singleton. That wasn’t an entirely fair comparison as Ninject was creating a new instance of the injected class every time, which I thought may be part of the cause of the difference in performance. So I’ve changed my test a bit so that Ninject uses a singleton as well.

Originally the Ninject kernel binding was like this:

NinjectContainer.Kernel.Bind<IAdditionProvider>().To<AdditionProvider>();

That tells Ninject to create a new instance of AdditionProvider every time. To get it to use the same instance – in effect a singleton – I changed the binding to this:

NinjectContainer.Kernel.Bind<IAdditionProvider>().ToConstant(new AdditionProvider());

I expected to see a huge performance gain, and feel like an idiot. There was a bit of a gain:

That’s roughly 8 seconds (or 12%) less then the 1m5s taken the first time around. So there’s a good gain to be had using ToConstant() to avoid creating unnecessary instances of the injected class, but still nothing like the benefits of only directly using Ninject outside of the loop.

I may have gone overboard with implementing dependency injection using Ninject on a project at work. I ended up with multiple calls to the kernel when creating each object. Worse yet I’ve used the [Inject] attribute to implement the binding, building up hundreds of dependencies on the Ninject library. At the time this seemed to make implementing things easier, especially as this was the first time I had used a dependency injection library, and I didn’t see a problem with a dependency on what seems like a core component.

I separated out 30 or so repositories, set up a heap of fakes, hooked up a handful of tests, and jumped into implementing the latest feature. Everything went well in development, I deployed, and all was shiny. Then the trouble started.

First the users started complaining about speed. Some operations were taking 10-20 times as long to run as before. That was because of calls to Ninject happening whenever many objects were being created, and inside of loops. Then some older Winforms components turned out to crash in the designer. That was due to the Ninject kernel not being set up when the component is executed by the designer – the new dependency on Ninject was failing. Then I read an article by Bob Martin and several of his tweets on the subject where he talks about the dependency framework itself becoming a dependency. Got that right.

I decided to replace the direct dependency injection via Ninject with a factory. This way the factory is the only place with the dependency on Ninject, and has the option of returning a mock if there is Ninject isn’t configured (to deal with the designer issues). It also lets me inject the dependency only once and reuse the result.

I set up a simple test to see what effect moving the dependency injection out of the loop would have on the speed. The first consumer of the test is has the DI on construction:

class InjectionTester
{
    [Inject] public IAdditionProvider Adder { get; set; }
    public int DoAdd(int a, int b) { return Adder.Add(a, b); }
}
...
// test using injection in the loop
for (int i = 0; i < 1000; i++)
{
    for (int j = 0; j < 1000; j++)
    {
        var tester = NinjectContainer.Kernel.Get<InjectionTester>();
        tester.DoAdd(i, j);
    }
}

The second consumer of the test uses a factory to get the dependency:

class AdditionProviderFactory
{
    public static AdditionProviderFactory Instance = new AdditionProviderFactory();

    IAdditionProvider additionProvider;
    object additionProviderLock = new object();
    public IAdditionProvider AdditionProvider
    {
        get
        {
            lock (additionProviderLock)
            {
                additionProvider = additionProvider ?? NinjectContainer.Kernel.Get<IAdditionProvider>();
            }
            return additionProvider;
        }
    }
}
class FactoryTester
{
    IAdditionProvider Adder = AdditionProviderFactory.Instance.AdditionProvider;
    public int DoAdd(int a, int b) { return Adder.Add(a, b); }
}
...
// test using the factory
for (int i = 0; i < 1000; i++)
{
    for (int j = 0; j < 1000; j++)
    {
        var tester = new FactoryTester();
        tester.DoAdd(i, j);
    }
}

The results are pretty impressive (looping a million times):

Thinking about it now it’s pretty obvious that DI is going to be a bit expensive, but seriously, using a factory is over 200 times faster. That should keep em quiet.

Using the Jeditable plugin for jQuery enables inline editing of a block of text. This makes it easy to take a static view and simply drop in editability. Say we start with the following HTML:

<label>Name:</label> <span class="employeeName">Ben Scott</span><br/>
<label>Rating:</label> <span class="employeeRating">Highly awesome</span>

We want to be able to edit the employeeName and employeeRating spans. We need two actions (asssuming an MVC framework) to update the name and rating. The URLs might be something like /employee/set_name/{id} and /employee/set_rating/{id}. Each action should accept HTTP POST, take the new value in $_REQUEST['new_value'] or similar, and return a HTTP status of 200 on success and 500 on failure, with the error message in the response. For example, using Slab (my PHP5 MVC framework, in development) the set_name action might be like this:

class EmployeeController extends AppController {
	function set_name($id) {
		$this->Employee->save(array(
			'id' => $id',
			'name' => $this->data['new_value']);
		));
		return $this->ajaxSuccess($this->data['new_value']);
	}
}

Slab catches uncaught exceptions and returns an AJAX failure with the exception message as the body of the response.

To hook up the fields to the actions, modify the HTML to include the URLs to the actions. While this means the markup has behavioural elements and isn’t purely presentational (and has a non-standard attribute), it makes the script a bit simpler and easy to move into a static .js file, and is a quick way to get a page working.

<label>Name:</label> <span class="employeeName" editUrl="/employee/set_name/7">Ben Scott</span><br/>
<label>Rating:</label> <span class="employeeRating" editUrl="/employee/set_rating/7">Highly awesome</span>

Now for the script itself:

$(function(){
	$('.employeeName, .employeeRating').editable(
		function(value, settings) {
			return $.ajax({
				url: $(this).attr('editUrl'),
				data: { 'new_value': value },
				async: false,
				type: 'post'
			}).responseText;
		}, {
			indicator: 'Saving...',
			tooltip: 'Click to edit',
			onblur: 'submit'
		}
	);
});

The first argument is a function that returns the new value of the field. This is important when doing things like replacing line breaks with <br/> which we’re not worrying about, but it also gives us the ability to write our own AJAX code. By default Jeditable makes assumptions about how the update is done. The async option in the call to $.ajax() blocks until the call returns, and lets the function return the response of the AJAX call. The second argument are options to set some text to show while calling the update function, the tooltip to display when hovering over the span, and to submit changes on blurring the input which makes it seem a bit more usable when there are multiple fields.

So today I hit a hat trick. I needed to test a class that had an injected dependency, and needed some functionality that I didn’t want to add to a fake and would rather isolate to the test. I needed to use a mocking framework. This is my first time using mocks (and only my second week of dependency injection) so this may not use best practices, but this _is_ a blog after all.

I’m using Ninject 2 for dependency injection, NUnit 2.5.3 for unit testing, and Rhino Mocks 3.6 for mocking. NUnit has a mocking framework built in but it doesn’t use strong typing, which I think was causing problems with Ninject.

What I’m really showing here is an example of how to inject a dynamically declared mock instead of a concrete fake. The fact that it is in the context of a test is actually irrelevant, but using mocks and fakes are obviously important in testing to reduce the complexity of the test.

Initially I’m using the default ConcreteFoo implementation of IFoo in the test. This test fails as intended:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

using Ninject;
using NUnit.Framework;
using Rhino.Mocks;

namespace Trifecta
{
    public interface IFoo
    {
        string Name { get; }
    }
    public class ConcreteFoo : IFoo
    {
        public string Name { get { return "You don't want to see me"; } }
    }

    public class FooConsumer
    {
        [Inject] public IFoo Foo { get; set; }

        public FooConsumer() {}
    }

    [TestFixture]
    public class FooConsumerTestFixture
    {
        IKernel Kernel { get; set; }

        [SetUp]
        public void SetUp()
        {
            // This is the standard setup that will need to be overridden in the
            // test.
            Kernel = new StandardKernel();
            Kernel.Bind<IFoo>().To<ConcreteFoo>();
        }

        [Test]
        public void FooConsumerGetsTheFoo()
        {
            // This test depends on a specific behaviour in IFoo, but ConcreteFoo
            // is going disappoint right now.
            var fooConsumer = Kernel.Get<FooConsumer>();

            Assert.That(fooConsumer.Foo.Name, Is.EqualTo("The foo for you"));
        }
    }
}

Settting up and binding the mock is all done in the test method. The mock IFoo instance is created, and the functionality that the test requires is added. IFoo is then bound to a delegate which returns the mock instance. The binding has a condition that causes the mock binding to be used rather than the ConcreteFoo binding, this seems to be easier then setting up the kernel from scratch for the test (possibly I’m just missing how to rebind).

[Test]
public void FooConsumerGetsTheFoo()
{
		// Create a mock implementation of IFoo that has the behaviour the test requires
		var mocks = new MockRepository();
		var mockFoo = mocks.StrictMock<IFoo>();
		Expect.Call(mockFoo.Name).Return("The foo for you");
		mocks.ReplayAll();

		// Bind the mock when injecting IFoo into FooConsumer. This overrides the binding
		// created in SetUp()
		Kernel.Bind<IFoo>().ToMethod(context => mockFoo).WhenInjectedInto<FooConsumer>();

		// fooConsumer's Foo should now be the mock IFoo created above
		var fooConsumer = Kernel.Get<FooConsumer>();

		Assert.That(fooConsumer.Foo.Name, Is.EqualTo("The foo for you"));
}

The test should now pass.