Mark Needham

About a month ago or so I described how we did some work to ensure that we were calling a class the same way in our tests as in our production code and while I think that was a good choice in that situation we came across a similar problem this week where we weren’t so sure.

The piece of code in question was being used to create the view model for a page and one of the pieces of data that we wanted to show on this page was the date on which something would be valid which is currently today’s date.

We were working with some code that looked a bit like this:

public class SomeRandomModel
{
	...
 
	public static SomeRandomModel CreateForSomeSituation(string value1, string value2)
	{
		return new SomeRandomModel(value1, value2);
	}
 
	public string SomeDate()
	{
		return DateTime.Today.ToDisplayFormat();
	}
}

public static class DateTimeExtensions 
{
	public static string ToDisplayFormat(this DateTime aDateTime)
	{
		return String.Format("{0:MMM d, yyyy}", aDateTime)
	}
}

Initially we put the following test around the code:

[Test]
public void ShouldConstructModelForSomeSituation()
{
	var model = SomeRandomModel.CreateForSomeSituation("", "");
 
	...
 
	Assert.AreEqual(DateTime.Today.ToDisplayFormat(), model.SomeDate()); 
}

The problem with this test is that it could theoretically fail if the ‘DateTime.Today’ in the test and the ‘DateTime.Today’ in the code were fired near enough to midnight that they returned different days.

It’s not quite as big a problem as we would have if we were actually comparing the equality of the two dates but it’s still not great to write a test which might fail sporadically.

We therefore need to find a way to control the input to the code so that we can test the output.

The problem is that the date is always going to be today at the moment so it seems to unnecessarily complicate the ‘CreateForSomeSituation’ method if we had to pass in ‘DateTime.Today’ as a parameter.

Our current solution was therefore to create a test only method which takes in a date and then get the original method to delegate to the test only one while passing in ‘DateTime.Today’ as the extra parameter.

public class SomeRandomModel
{
	private readonly DateTime aDateTime;
	...
 
	public static SomeRandomModel CreateForSomeSituation(string value1, string value2)
	{
		return CreateForSomeSituation(value1, value2, DateTime.Today);
	}
 
	public static SomeRandomModel CreateForSomeSituationForTest(string value1, string value2, DateTime aDateTime)
	{
		return new SomeRandomModel(value1, value2, aDateTime);
	}
 
	public string SomeDate()
	{
		return aDateTime.ToDisplayFormat();
	}
}

Our test now becomes:

[Test]
public void ShouldConstructModelForSomeSituation()
{
	var model = SomeRandomModel.CreateForSomeSituationForTest("", "", new DateTime(2009, 10, 10));
 
	...
 
	Assert.AreEqual("10 Oct 2009", model.SomeDate()); 
}

We can now specify our expectation a bit more cleanly since we aren’t tied to ‘DateTime.Today’.

It helped solve our problem reasonably well but it still seems a bit weird to me to have ‘test only’ code but I’m not sure what a better solution would be.

My colleague Matt Dunn and I have been putting a lot of tests around some code over the last few days so that we can safely make some changes around that area and having finally created our safety net we’ve moved onto adding in the new functionality.

We’re test driving the new bit of functionality whereas with the previous code only the code had been written with no unit tests and it’s been quite interesting seeing the contrast in the style of code which seems to come out from these differing styles.

While we were writing the tests around the other code I found it quite surprising how often we came across code which wasn’t actually needed – I guess if you only write the code then you need to make sure that all potential scenarios are covered even if it’s impossible for them to happen.

In one case a null check was being done on a dependency of a class which always has its dependencies injected by a DI container and thus as far as I understand could never actually be null.

It seems to me that the outcome of this approach to coding can contribute greatly to what Fred Brooks coins ‘Accidental Complexity’ in ‘The Mythical Man Month‘ – extra code which makes the code base more difficult to read as a whole but which adds no value apart from that. Essentially waste.

When test driving code I find that the process of coming up with examples/tests before you write any code leads you to question how the system actually works.

For example Matt and I had thought that the new piece of functionality we were working on would need to make a call to a repository to retrieve some data since that’s what was being done for the existing functionality.

However, while writing our tests it become quite clear that we wouldn’t need to do that.

On this occasion we wrote a test with an expectation for a call on the repository, then wrote the code to make that test to pass before moving onto our test where we would assert that the values retrieved from the repository would be populated into the appropriate object.

It was at this stage that we realised that there was actually no need to make that call because we already had the data available to us from another source.

This has happened on quite a few occasions since and while I think pair programming helps put us in a state of mind where we are more inquisitive of the code being written, I think the TDD approach is quite useful as well.

I’m sure it’s possible to avoid some of the problems I’ve described without a test driven approach but I certainly find it more difficult.

I’ve been re-reading a post my colleague Ian Cartwright wrote earlier this year about treating test code the same way as production code and one thing which stands out as something which I’m certainly guilty off is copying and pasting tests.

Ian lists the following problems with doing this:

The first one is cut & paste, for some reason when it comes to unit tests people suddenly start cutting and pasting all over the place. Suddenly you find a file with 20 tests each of which repeats exactly the same few lines of code. I don’t think I need to describe why this is bad and I expect we’ve all seen the outcome: at some point later those tests all start breaking at the same time, if we are unlucky a few tweaks have happened to the cut & pasted code in each so we spend a lot of effort figuring out how to make each one pass again. There is no rule that says we can’t have methods in test fixtures so ExtractMethod still applies, and using SetUp sensibly often helps. The same rational for avoiding cut & paste and the same solutions we know from production code apply to test code.

Despite this I’ve often felt that at some stage we should have factored tests down to a stage where removing any more of the ‘duplication’ would actually harm the readability of the test and that at this stage perhaps it’s ok to copy the skeleton of a test and reuse it for your next test.

Since I first read Ian’s post I’ve been playing around with the idea of never copying/pasting tests when I’ve been working on my own on my project and with stuff I play around with outside work and I like the approach but hadn’t really fully appreciated the benefits that we could get from doing so.

Over the last week or so my colleague Matt Dunn and I have been writing tests around pre-existing code and we decided that we would try wherever possible to not copy tests but instead write them out from scratch each time.

I found it quite difficult to start with as the temptation is always there to just grab a similar test and tweak it slightly to test the new bit of functionality but an interesting early observation was that we were far less accepting of duplication in tests when we had to type out that duplication each time than if we had just copied and pasted it.

I tend to extract method quite frequently when I see test code which is similar but we were seeing potential areas for extracting out common test code which I would never have spotted.

There’s a chapter in Steve Freeman and Nat Pryce’s book ‘Growing Object Oriented Software, guided by tests‘ titled ‘Listening to the tests‘ which I quite like and I think we become much more aware of the tests if we have to write them out each time.

An example of this which Matt spotted was that in one bit of test code we were working on there was a method called ‘StubOutSomeRepository’ which was called by every single test and I instinctively made the call to that method as the first line in our new test.

Matt questioned whether it was actually even needed so we removed it from every single test in that test fixture and they all still worked!

I’m pretty sure that we wouldn’t have spotted that problem if we’d just copy/pasted and I’m also fairly sure that all the other tests in that test fixture were copy/pasted from the first test written in the test fixture when there may actually have been a need to stub out the dependency.

Apart from enabling us to simplify our tests I think that writing out our tests each time keeps us thinking – it’s really easy to turn off and work on auto pilot if you’re just copy/pasting because it’s essentially mindless and doesn’t engage the mind that much.

Keeping in the thinking mindset allows us to see whether we’re actually testing properly and I think it also makes the pairing process a bit more interesting since you now have to talk through what you’re doing with your pair.

One of the other reasons I thought copy/pasting might be useful sometimes is that it can save time if we’re typing out similar types of tests repeatedly and I often feel that it would be quite annoying for my pair to watch me do this.

Now I’m not so convinced by my own argument and I think that the time that we save by writing more readable tests probably easily outweighs any short term time gains.

Our conversations have become more about working out ways that we can

On a related note I recently watched a video of a performance kata by Corey Haines where he works through a problem guided by tests and as far as I remember doesn’t ever copy and paste a test but instead looks for ways to reduce the duplication he has in his test code so that he doesn’t have to do too much typing for each test.

During J.B. Rainsberger’s presentation at Agile 2009 titled ‘Integration tests are a scam‘ he suggests that having lots of integrationt tests covering our code can give us a false sense of confidence that we are testing our code and I think the same can happen with unit tests as well if we’re not careful how we write them.

It’s important to ensure that our unit tests are actually testing something useful otherwise the cost of writing and maintaining them will outweigh the benefits that we derive from doing so.

I’ve come across two type of test recently which don’t test an awful lot but can lead us to thinking our code is well covered at the unit level if we just glance at the test names and don’t take the time to examine exactly what’s going on which is what seems to happen a lot of the time.

Only testing for ‘IsNotNull’

I think testing that a value we want to do some assertions against isn’t null is a great start since it can help us avoid null reference exceptions in our tests but it’s only the first assertion that we want to make, not the only one!

The name of these tests is nearly always misleading and it often seems like these tests are a work in progress but one which never gets finished.

The following is not unusual:

[Test]
public void ShouldPopulateParentModelOnAction()
{
	var someController = new SomeController(...);
 
	// There'd probably be more casting to actually retrieve the actual model 
	// but we'll ignore that for the sake of this example
	var parentModel = someController.SomeAction();
 
	Assert.IsNotNull(parentModel);
}

The problem I have with this type of test is that the name suggests we have completely tested that the controller action is correctly populating the model but in reality all we’re testing is that we got a model and not that its contents are accurate.

It’s surprisingly easy when skimming through tests to not even notice that this is the case.

The danger we’re toying with by doing this is that others might assume that this area of the code is tested and subsequently do refactorings around this code assuming that the tests are providing a safety net when in actual fact they aren’t.

Certainly sometimes ‘IsNotNull’ is all we care about and in those cases it’s fine just to test for that but I think that most of the time it’s just a lazy way out of properly testing a piece of code.

Testing expectations on stubs

I’m not sure if this is just a Rhino Mocks problem but I’ve come across quite a few tests recently which make expectations on stubs and then sometimes verify those expectations.

Most of the time the creation of the stubbed dependency is hidden away in a setup method so the tests wouldn’t be quite as clear cut as this but this is what’s happening:

[Test]
public void ShouldRetrieveSomeValuesFromTheRepository()
{
	var theRepository = MockRepository.GenerateStub<ITheRepository>();
	theRepository.Expect(r => r.SomeMethod()).Return("someValue");
 
	var systemUnderTest = new SystemUnderTest(theRepository);
 
	systemUnderTest.DoSomething();
}

or

[Test]
public void ShouldRetrieveSomeValuesFromTheRepository()
{
	var theRepository = MockRepository.GenerateStub<ITheRepository>();
	theRepository.Expect(r => r.SomeMethod()).Return("someValue");
 
	var systemUnderTest = new SystemUnderTest(theRepository);
 
	systemUnderTest.DoSomething();
 
	theRepository.VerifyAllExpectations();
}

The problem with both of these tests is that they aren’t doing anything useful for us.

From my understanding of the Rhino Mocks code what we are effectively doing is creating a stub on ‘theRepository’ which will return a value of ‘someValue’ the first time that ‘SomeMethod’ is called on it. The ‘VerifyAllExpectations just falls through and does nothing because we’ve created a stub and not a mock.

We could just as easily not call any method on the system under test and we’d still get a green bar.

If we intend to test the way that the system under test interacts with its dependencies then we want to make sure we’ve actually created a mock and then ensure that we check that the expectations have actually been called.

The test above could become a bit more like this for that purpose:

[Test]
public void ShouldRetrieveSomeValuesFromTheRepository()
{
	var theRepository = MockRepository.GenerateMock<ITheRepository>();
	theRepository.Expect(r => r.SomeMethod()).Return("someValue");
 
	var systemUnderTest = new SystemUnderTest(theRepository);
 
	systemUnderTest.DoSomething();
 
	theRepository.VerifyAllExpectations();
}

In summary

I’ve previously just ignored tests like this and just got on with what I was doing but I think it’s important to try and clean them up so that they can make a greater contribution to our testing efforts – Uncle Bob’s Boy Scout Rule applies here – leave the tests in a better state than you found them.

That way we can move towards having justifiable confidence that we can make changes to our code without fear that we’ve broken some piece of functionality.

While the test data builder pattern is quite a useful one for simplifying the creation of test data in our tests I think we need to be quite careful when using it that we don’t lose the intent of the test that we’re writing.

The main advantage that I see with this pattern is that by using it we can provide default values for properties of our objects which aren’t important for the bit of functionality that we’re currently testing but which need to be provided otherwise the test can’t actually be run.

In order for our tests to express their intent clearly it is still important to explicitly set the values that we care about for the current test otherwise it will become quite difficult to work out why a test is failing when it does.

For example I recently came across a failing test similar to this:

[Test]
public void ShouldTestSomeCoolStuff()
{
	var foo = new FooBuilder().Build();
 
	var yetAnotherObject = someOtherObject.ThatTakesIn(foo);
 
	Assert.AreEqual("DEFAULTBAR", yetAnotherObject.Bar);
	Assert.AreEqual("DEFAULTBAZ", yetAnotherObject.Baz)	
}

It was failing because the actual values being returned were ‘defaultBar’ and ‘defaultBaz’ which initially made us wonder if there was some capitalisation going wrong in one of our objects.

As it turned out the values being returned were just the default ones from the ‘FooBuilder’ and we were asserting the wrong thing:

public class FooBuilder
{
	private string bar = "defaultBar";
	private string baz = "defaultBaz";
 
	public FooBuilder Bar(string value)
	{
		bar = value;
		return this;
	}
 
	public FooBuilder Baz(string value)
	{
		baz = value;
		return this;
	}
 
	public Foo Build()
	{
		return new Foo(bar, baz);
	}
}

While it didn’t take us too long to get to the cause of the problem I think it would have made our lives easier if we’d been able to tell why it was failing just from looking at the test instead of having to look in a couple of different classes to figure it out.

A test written more like this would help us to achieve that:

[Test]
public void ShouldTestSomeCoolStuff()
{
	var foo = new FooBuilder().Bar("myBar").Baz("myBaz").Build();
 
	var yetAnotherObject = someOtherObject.ThatTakesIn(foo);
 
	Assert.AreEqual("myBar", yetAnotherObject.Bar);
	Assert.AreEqual("myBaz", yetAnotherObject.Baz)	
}

Then if we want to remove duplication we can just put the expected values into more descriptive variables:

[Test]
public void ShouldTestSomeCoolStuff()
{
	var expectedBar = "myBar";
	var expectedBaz = "myBaz";
	var foo = new FooBuilder().Bar(expectedBar).Baz(expectedBaz).Build();
 
	var yetAnotherObject = someOtherObject.ThatTakesIn(foo);
 
	Assert.AreEqual(expectedBar, yetAnotherObject.Bar);
	Assert.AreEqual(expectedBaz, yetAnotherObject.Baz)	
}

There’s a bit more test code than in the first example but if the test fails again we should be able to work out why more quickly than before because we are now more explicit about which values are actually important for this test.

In a post I wrote earlier in the week I described a dilemma we were having testing some code which made use of abstract classes and Perryn Fowler, Liz Keogh and Pat Maddox pointed out that a useful approach for this problem would be to make use of an abstract test class.

The idea here is that we create an equivalent hierarchy to our production code for our tests which in the example that I provided would mean that we have roughly the following setup:

public abstract class ParentModelTest
{
}
 
public class BusinessProcess1ModelTest : ParentModelTest { }
public class BusinessProcess2ModelTest : ParentModelTest { }
public class BusinessProcess3ModelTest : ParentModelTest { }

We then want to create an abstract method on the ‘ParentModelTest’ class to create the object under test and we’ll implement this method in the sub classes.

We can then put the common tests into the abstract class and they will be run when we run the tests for each of the sub class tests.

My colleague Matt Dunn and I were looking at how you would implement this and he pointed out that it provided quite a nice opportunity to use generics to achieve our goal.

If we make the abstract test class take in generic parameters then it allows us to create our specific type in the ‘create’ method rather than having to create ‘ParentModel’ and then casting that for our specific sub class tests.

public abstract class ParentModelTest<T> where T : ParentModel
{
	protected abstract T CreateModel();	
 
	[Test]
	public void ShouldTestThoseCommonDependencies()
	{
		var model = CreateModel();
		// Do some awesome testing that's common across all 3 sub classes here
	}
}

[TestFixture]
public class BusinessProcessModel1Test : ParentModelTest<BusinessProcessModel1>
{
	protected override BusinessProcessModel1 CreateModel()
	{
		return new BusinessProcessModel1(...);
	}
 
	[Test]
	public void ShouldDoSomeEquallyOutstandingStuff() 
	{
		var model = CreateModel();
		// and test away
	}
}

It seems to work quite well and we realised that it’s actually a pattern that we had also used to test that our controllers would actually be injected with all their dependencies from our DI container or whether we have forgotten to register some of them.

We have an abstract class similar to this which all our controller tests extend:

public abstract class ContainerTest<T> where T : Controller
{
	[Test]
	public void ShouldHookUpAllDependencies()
	{
		var container = CreateTheContainer();
 
		var controller = container.Resolve<T>();
 
		Assert.IsNotNull(controller);
	}
}

public class AwesomeControllerTest : ContainerTest<AwesomeController> { }

When we haven’t got all the dependencies injected correctly the code will actually blow up on the resolve step so the last line is not strictly necessary but the test seems like it’s not testing anything at first glance without it.

We ran into another interesting testing dilemma while refactoring the view model code which I described in an earlier post to the point where we have an abstract class and three sub classes which means that we now have 3 classes which did the same thing 80% of the time.

As I mentioned in a post a couple of weeks ago one of the main refactorings that we did was to move some calls to dependency methods from the constructor and into properties so that those calls would only be made if necessary.

After we’d done this the code looked a bit like this:

public abstract class ParentModel
{
	private readonly Dependency1 dependency1;
 
	...
 
	public decimal Field1 { get { return dependency1.Calculation1(); } }
	public decimal Field2 { get { return dependency1.Calculation2(); } }
}
 
public class BusinessProcess1Model : ParentModel { }
public class BusinessProcess2Model : ParentModel { }
public class BusinessProcess3Model : ParentModel { }

We wanted to ensure that the tests we had around this code made sure that the correct calls were made to ‘depedency1′ but because ParentModel is an abstract class the only way that we can do this is by testing one of its sub classes.

The question is should we test this behaviour in each of the sub classes and therefore effectively test the same thing three times or do we just test it via one of the sub classes and assume that’s enough?

Neither of the options seems really great although if we cared only about behaviour then we would test each of the sub classes independently and forget that the abstract class even exists for testing purposes.

While the logic behind this argument is quite solid we would end up breaking 3 tests if we needed to refactor our code to call another method on that dependency for example.

I suppose that makes sense in a way since we have actually changed the behaviour of all those classes but it seems to me that we only really need to know from one failing test that we’ve broken something and anything beyond that is a bit wasteful.

In C# it’s not actually possible for ‘Field1′ or ‘Field2′ to be overriden with an alternate implementation unless we defined those properties as ‘virtual’ on the ‘ParentModel’ which we haven’t done.

We could however use the ‘new’ keyword to redefine what those properties do if the callee had a reference directly to the sub class instead of to the abstract class which means it is possible for a call to ‘Field1′ to not call ‘dependency1′ which means that maybe we do need to test each of them individually.

I’m not sure which approach I prefer, neither seems better than the other in my mind.

I wrote previously how we’d been doing some work to change the way that we get a ‘User’ object into our system and one mistake that we made intially was to have another constructor on the ‘User’ object which was being used in all our unit tests which involved the user in some way.

The original reason that this ‘test constructor’ was created was to make it easier to construct a ‘fake user’ which we were using in some of our functional tests but had ended up being used in unit tests as well.

The constructor being exposed for testing was pretty much the same as the private constructor that we have now.

public class User
{
	...
	public User(string userName, string customerId)
	{
		this.userName = userName;
		this.customerId = customerId;
	}
}

The problem with this approach to testing is that we aren’t actually testing the code which we’re using in production so even if the tests pass it doesn’t actually tell us very much about our code.

We could be doing everything right in this test constructor and doing something crazy in the static factory method and we wouldn’t find out until much later on.

The interesting thing is that the method that we call from the production code isn’t as testing friendly as the one we had made public just for testing:

	public static User CreateUserFrom(NameValueCollection headers)
	{
		var userName = headers["user-name-header-tag"];
		var customerId = headers["customer-id-header-tag"];
 
		// and so on
 
		return new User(userName, customerId);
	}

In order to make our test setup code use this method we had to create a NameValueCollection containing key/value pairs with the appropriate keys that reside in the headers of requests coming into our application.

We therefore end up with code similar to this in the test data builder:

public class UserBuilder
{
	...
 
	public User Build()
	{
		var headers = new NameValueCollection();
		headers.Add("user-name-header-tag", "randomName");
		headers.Add("customer-id-header-tag", "customerId");
		User.CreateUserFor(headers);
	}
}

This leaks a bit of the implementation of ‘CreateUserFrom’ into the tests but I prefer this to testing something which is never actually used.

I previously wrote about some duplicated code we’d taken the time to remove from our code base and one something else that we found when working with this code is that a lot of the tests around this code were testing the implementation/internal state of the object rather than testing the behaviour that they expected to see.

I find it makes more sense to test the behaviour since this is the way that the object will most likely be used in our production code.

For example, in the code I posted previously we were setting up the way that a navigation bar was going to behave in different scenarios.

This was one of the tests we had:

public void ShouldSetLogin()
{
	var navigationModel = new NavigationModel(Options.Login);
	Assert.IsTrue(navigationModel.IsConfiguredWith(Options.Login));
}

For this (cut down for example purposes) code:

public enum Options 
{
	Login = 1,
	Logout = 2
	// and so on
}

public class NavigationModel
{
	public NavigationModel(Options options)
	{
		Configuration = options;
	}
 
	public Options Configuration { get; private set; }
 
 
	public bool IsConfiguredWith( Options expectations)
	{
		return expectations == Configuration & expectations);
	}
 
	public bool ShowLogin() 
	{
		return IsConfiguredWith(Options.Login);
	}
}

There were 7 different types of options as I mentioned in the previous post and the NavigationModel was being setup with each of them and the ‘IsConfiguredWith’ method was then being called to check whether the value had been set.

The strange thing was that everything we wanted to test could be done by calling methods like ‘ShowLogin’ which made us of the ‘IsConfiguredWith’ method anyway.

The first refactoring was therefore to change the tests to make use of these methods instead of calling on an implementation detail of the NavigationModel:

public void ShouldShowLogin()
{
	var navigationModel = new NavigationModel(Options.Login);
	Assert.IsTrue(navigationModel.ShowLogin());
}

There’s not really that much difference in what the test does in this case but by changing all the tests to make calls to the methods that we use in our production code we were able to make the ‘IsConfiguredWith’ method private which is quite nice since it was only being used inside the tests and the ‘Show…’ methods so it didn’t really make sense to have them public.

The next step after this was to create the factory methods that I mentioned in the previous post and since each of these methods encapsulated more behaviour the tests started to look a bit better:

public void ShouldShowLoginAndOption1AndOption2ForAnonymousUser()
{
	var NavigationModel = NavigationModel.ForAnonymousUser();
 
	Assert.IsTrue(navigationModel.ShowLogin());
	Assert.IsTrue(navigationModel.ShowOption1());
	// and so on
}

This is quite a simple example but I found it interesting that with just a little bit of tweaking we could change our tests to execute the same methods that will be run in production and combined with the other refactoring we can now encapsulate the way we are determining whether a user can see a certain option or not and potentially change the implementation of that in the future if we want to.

I decided to spend a bit of time on Saturday having another go at writing Rock Scissors Paper while following Keith Braithwaite’s TDD as if you meant it exercise.

We previously did this exercise at a coding dojo but I wanted to see what happens when you code for a longer period of time with this exercise since we typically only code for maybe a couple of hours at a dojo.

I decided to also checkin the code I was writing into Git after every single change I made – an idea I originally learnt from Dan North, and the code is all on github.

What did I learn?

• I was coding for maybe 4 or 5 hours and checked in about 120 times which is far more frequently than I would do normally although perhaps it shouldn’t be.

  I thought it would be quite distracting to have to check in that often but the checkin messages ended up becoming a stream of consciousness of what I was thinking which proved quite useful on a few occasions when I got side tracked a bit and retraced my steps in the history to find out what I was supposed to be doing – Git was pretty much acting as my sounding board since I didn’t have a pair to work with on this exercise.

  I’d definitely try this out again and as I’ve mentioned a few times I want to try it out in a coding dojo some time.

• While trying to remove some duplication that had worked its way into the code I realised that the template method would be the easiest way to remove that duplication and as I mentioned in an earlier post, having functions/actions in C# makes the implementation of this pattern a bit simpler:

      public abstract class ThrowBase : IThrow
      {
          private readonly Func<IThrow, bool> beatenBy;
   
          protected ThrowBase(Func<IThrow, bool> beatenBy)
          {
              this.beatenBy = beatenBy;
          }
   
  	 ....		
   
          public bool Beats(IThrow aThrow)
          {
              return IsDifferentThrowTo(aThrow) && !beatenBy(aThrow);
          }
   
      }
  }

      public class Scissors : ThrowBase
      {
          public Scissors() : base(weapon => weapon.BeatsScissors()) { }
   
  	...
      }

  If we didn’t have functions then we’d need a method on ‘ThrowBase’ which we would need to implement in each of its sub classes.

  As it is we can just pass a function into the constructor which does the same job.

• Another thing I found quite interesting about this exercise was that I was seeing more easily where methods didn’t seem to belong on an existing object and I ended up creating a new object which described the interaction – chatting about the code with Liz she pointed out that I was probably creating domain events which we had discussed a few days earlier in book club.

  Once I started thinking about this interactions as ‘domain events’ the naming of them seemed to be much more obvious – for example after this conversation with Liz I realised that the interaction between two ‘Throws’ would be a ‘ThrowDown’ and that there would probably be a ‘ThrowDownAdjudicator’ to decide the outcome of a ‘ThrowDown’.

• I found it quite difficult to follow the rules of the exercise when I realised that I wanted to return a ‘ThrowDownResult’ which would indicate whether there was a winner for a ‘ThrowDown’ and if so who it was. I couldn’t see a way to write all of this code in the test since I had already created the class previously so I ended up writing this code straight onto the class.

  I think I probably extracted to a class too early on some occasions instead of waiting for a more complete version of a method to be written before moving it.

  Next time I think I’d wait until I’d completed all the examples /tests I want to write around a particular method before moving it onto a class.