Mark Needham

Something which we discussed quite frequently when studying Domain Driven Design in our technical book club earlier this year was where the code which checked whether we had setup an object correctly should reside.

Shortly after that I suggested that I didn’t think it should go in the constructor of an object but that we should rely on objects to be good citizens and not pass in null values or the like to other objects.

However, for code that’s on the edge of our system we can’t really rely on that approach so our code needs to be a bit more defensive.

On those occasions an approach which seems to work relatively well is to introduce a factory method which is responsible for creating an object or throwing an exception if the object can’t be created in a consistent state.

Dave and I made use of this pattern while working on some code to get a ‘User’ object into our system.

We realised that there were three different scenarios that we needed to handle:

• All the authentication details for a user were available in the headers of the HTTP request
  • Create a logged in user
• There were no authentication details available for a user
  • Create an anonymous user
• Some of the authenticated details for a user were available – something has gone wrong
  • Throw an exception

We created a method with the following signature:

public static User CreateUserFrom(NameValueCollection headers)  { }

‘headers’ are the values that we retrieve from the HTTP request headers.

Whenever I’ve used static factory methods on an object previusly, it’s mainly been for providing a way of creating an object while giving the client more information on the specific type of object that they are about to create.

As Dave pointed out, the factory method is also useful for providing a simplified interface to clients and allowing the factory to take away some of the complexity of the object creation which is what we wanted to do here.

We ended up with code that looked a bit like this:

public class User
{
	public static readonly User Anonymous = new AnonymousUser("", "");
 
	private readonly string userName;
	private readonly string customerId;
 
	private User(string userName, string customerId)
	{
		this.userName = userName;
		this.customerId = customerId;
	}
 
	public static User CreateUserFrom(NameValueCollection headers)  
	{ 
		var userName = headers["user-name-header-tag"];
		var customerId = headers["customer-id-header-tag"];
 
		var invariants = new List<string> { userName, customerId };
 
		if(invariants.All(i => i == null)
		{
			return Anonymous;
		}
 
		if(invariants.Any(i => i == null)
		{
			throw new Exception("Attempt to create user failed as not all header tags were available");
		}
 
		return new User(userName, customerId);
	}
}
 
public AnonymousUser : User
{
	public AnonymousUser(string userName, string customerId) : base(userName, customerId) {}
}

In this example ‘AnonymousUser’ is an implementation of the null object pattern.

When discussing our approach with Liz she reminded me that Eric Evans actually talks about invariant handling in the book and suggests that checking invariants in factory methods certainly is appropriate on some occasions:

A FACTORY is responsible for ensuring that all invariants are met for the object or AGGREGATE it creates; yet you should always think twice before removing the rules applying to an object outside that object. The FACTORY can delegate invariant checking to the product, and this is often best.

But FACTORIES have a special relationship with their products. They already know their product’s internal structure, and their entire reason for being involves the implementation of their product. Under some circumstances, there are advantages to placing invariant logic in the FACTORY and reducing clutter in the product. This is especially appealing with AGGREGATE rules (which span many objects). It is especially unappealing with FACTORY METHODS attached to other domain objects.

I’m not sure if what we have is an aggregate root since ‘User’ doesn’t act as the gateway to access any other objects but the approach seemed to work reasonably well here nonetheless.

In this weeks book club we discussed Arlo Belshee’s paper ‘Promiscuous Pairing and Beginner’s Mind‘ where he presents the idea of rotating pairs more frequently than we might usually, suggesting that the optimal rotation time is 90 minutes.

I remember coming across the idea of promiscuous pairing a couple of years ago but I hadn’t read the paper all the way through and so far haven’t worked on a team where we’ve really tried out his ideas.

These are some of my thoughts and our discussion of the paper:

• I found the section of the paper where he talks about skills and competencies quite interesting – the suggestion seems to be that for any given task the least skilled person for that task should be the one to do it but that the person should still have the necessary competency to execute it.

  I’m not entirely sure of the distinction between skills and competencies – Belshee suggests:

  The difference is simple. People can learn skills in a matter of months. People can’t learn competencies in less than several years. There aren’t many things that fall between — qualifications are almost always skills or competencies.

  Software development wise this would suggest that a skill such as object orientation would be more likely to be a competency but what about a specific programming language?

  It is possible to learn your way around a language to the point where you can do some productive things with it relatively quickly but for me at least there are still things I don’t know about in the languages I work with and I’ve used some of them for a few years now.

• There is a nice quote from the paper when discussing the idea of giving tasks to the least qualified person, that ‘the least qualified teams produced the code that had the fewest surprises‘ – I imagine this is probably down to the fact that the least qualified person probably doesn’t yet know how to do clever things with a language so they just do the most obvious implementation. I think this is certainly what we want to happen when a team is working on code together.
• I liked the discussion of beginner’s mind where he talks about it being a transitionary state that we move into when are in a situation that is unfamiliar but near the limits of our comfort zone and that we will move out of once we are comfortable with the current situation.

  It seems like this state of mind links quite closely with the idea of deliberate practice that Geoff Colvin talks about in ‘Talent is Overrated‘ – the idea being that in order to improve most effectively we need to be doing activities which are just beyond our current competence.

• I’ve frequently noticed that people are reluctant to swap pairs until they have finished the story that they’re working on – Matt Dunn pointed out that this is probably linked to human’s natural desire to finish what they’ve started!

  Belshee seems to get around this problem by ensuring that the tasks being worked on are sufficiently small in size that they can be completed within one or two pairing sessions.

  A lot of the work that we do is integrating different systems where there is quite a bit of exploratory work to find out what we actually need to do first – it would be interesting to see if quicker rotations would be appropriate for this type of work or whether a lot of time would be spent bringing the new person on the pair up to speed with what’s going on.

• We had some discussion on pair programming in general – Raphael Speyer described the idea of ‘promiscuous keyboarding‘ as an approach to try within a single pair. The idea is that the keyboard switches between each person every minute which hopefully leads to both people being more engaged.

  I find that quite often on teams people will roll in and out of pairs when there help is needed on something – Nic Snoek described this as being ‘guerilla pairing‘ and I think it is something that a technical lead of a team is most likely to engage in as they move around the room helping people out.

I often feel that pair programming is a skill that we take for granted and we assume that we can just put two people together and they’ll be able to work together effectively.

From what I’ve found this doesn’t always work out so I think it’s important to keep innovating and trying out different things in this area so that we can find approaches that allow us to work better together.

*Update*
As Dave suggests it should be ‘guerilla’ and not ‘gorilla’ pairing that Nic suggested as being a useful idea.

In our latest coding dojo we played around with Scala and lambdaj while attempting to solve some of the problems on the Project Euler website.

The Format

We started off on two different machines with two of us having a look at solving the first Project Euler problem in Scala and the other two trying to solve it in Java while using the lambdaj library.

What did we learn?

• Fabio and I worked on the Scala solution to the problem and we were pretty much playing around with different ways to aggregate all the values in the list:

  1.to(999).filter(x => x%3 == 0 || x%5 == 0).foldLeft(0)((acc,x) => acc + x)
  1.to(999).filter(x => x%3 == 0 || x%5 == 0)./:(0)((x,y) => x+y)
  1.to(999).filter(x => x%3 == 0 || x%5 == 0).foldRight(0)(_+_)
  1.to(999).filter(x => x%3 == 0 || x%5 == 0).reduceLeft(_+_)
  1.to(999).filter(x => x%3 == 0 || x%5 == 0)./:(0)(_+_)

  We decided to work through how ‘foldLeft’ and ‘foldRight’ work for summing a simpler collection of data, the numbers 1-5, which goes something like this:

  fold_left

  (((((0 + 1) + 2) + 3) + 4) + 5) = 15

  fold_right

  (1 + (2 + (3 + (4 + (5 + 0))))) = 15

  When adding the numbers together it doesn’t make any different whether we fold the collection from the left or from the right.

  If we do subtraction we do get different answers though:

  fold_left

  (((((0 - 1) - 2) - 3) - 4) - 5) = -15

  fold_right

  (1 - (2 - (3 - (4 - (5 - 0))))) = 3
   
  -> 5 - 0 = 5
  -> 4 - 5 = -1
  -> 3 - -1 = 4
  -> 2 - 4 = -2
  -> 1 - -2 = 3

  The other way of doing fold_left, ‘/:’, is quite interesting although perhaps unintuitive when you first come across it. Liz showed me an interesting post by Ricky Clarkson where he talks about this method and the reason why it’s in the language.

  Liz and Dave Yeung worked on doing a solution to lambdaj to the problem but it ended up being quite verbose so we decided to play around with Scala for the rest of the session.

• We spent a bit of time playing around with traits via a nice introductory article which effectively acts as a a cut down version of the Programming in Scala book.

  I particularly like the way that you can add a trait to an instance of an object and you get access to all the methods defined on that trait which seems quite similar to Ruby mixins.

  Liz and I were discussing an article written by Michael Norton comparing technical debt and making a mess so that’s where the trait name came from!

  trait Mess  {
    def makeMess = print("tech debt is so funny")
  }
   
  class Liz {
   
  }

  If you try to make use of the trait like this:

  val liz : Liz = new Liz with Mess

  You don’t have access to the ‘makeMess’ method since you explicitly defined the type to be ‘Liz’ which doesn’t have the method:

  liz.makeMess
  > error: value makeMess is not a member of Liz
         liz.makeMess

  If we let the compiler do its thing without trying to explicitly type the value it works much better:

  val liz = new Liz with Mess
  > Liz with Mess

  liz.makeMess
  > tech debt is so funny

  I really like having the ability to do this although I think I need to code a bit more Scala before I’ll appreciate where we really gain by having this feature.

  The conciseness of the language and the lack of ‘{}’ while keeping the same expressiveness in the code is something which reminds me a lot of F# and from what I’ve seen so far I imagine it would be much more fun to code in Scala than in Java.

  It also seems more clear to me that when a static language has really good type inference then one of the main reasons that people prefer dynamic languages – you get to write less code for the same amount of features – is actually much less valid.

• This was a very experimental dojo in nature although Liz has been playing around with Scala a bit so she was able to guide us a bit when we got stuck.

  I’m finding that sessions where everyone is fairly new to the language or technology aren’t necessarily as fruitless as I had previously imagined that they would be – I find that learning something together makes it more interesting as you can then draw on other people’s ideas and understanding of the language as well as your own.

  I think in the cases where everyone is a novice people need to be prepared to get involved and write some code despite that. If that’s the case then I think it’s certainly possible to gain a lot from these sessions.

A pattern in code which I’ve noticed quite frequently lately is that of executing calculations in the constructor of an object and then storing the result in a field on the object.

From the small amount of experience I have playing around with functional languages I have come across the idea of lazy evaluation of functions quite frequently and I think it’s something that we can apply in object oriented languages as well.

When we store a value of a calculation in a field we are opening up the ability for that value to be changed before we use it.

We can certainly reduce/remove the chance of that happening by making fields read only or final but as Dhanji points out in his InfoQ article, if we are storing reference objects there is still a chance they could be mutated before we use them.

Even if we can manage to work our way around that problem I still feel that we increase the complexity of classes by having more fields as well as decreasing the readability of the code in the constructor since we now have all this extra information standing in our way when perhaps we don’t even care about it at all since it won’t be used until later on anyway.

I’m not sure if it’s a given but I never expect calculations to be done in the constructors of objects when I create them – at most I would expect the fields I pass in to be stored but any more than that is surprising and I think it’s good to avoid surprises if we can!

I don’t think the automated properties in C# 3.0 have really helped much and code like this is quite common:

public class SomeObject
{
	public SomeObject(Dependency1 dependency1, Dependency2 dependency2)
	{
		Field1 = dependency1.Calculation1();
		Field2 = dependency1.Calculation2();
		Field3 = dependency2.Calculation1();
		Field4 = dependency2.Calculation2();
		// and so on
	}
 
	public decimal Field1 { get; set; }
	public decimal Field2 { get; set; }
	public decimal Field3 { get; set; }
	public decimal Field4 { get; set; }
}

This one is relatively easy to simplify since we just need to store ‘dependency1′ and ‘dependency2′ and then delegate if the properties are called:

public class SomeObject
{
	private readonly Dependency1 dependency1;
	private readonly Dependency2 dependency2
 
	public SomeObject(Dependency1 dependency1, Dependency2 dependency2)
	{
		this.dependency1 = dependency1;
		this.dependency2 = dependency2;
		// and so on
	}
 
	public decimal Field1 { get { return dependency1.Calculation1(); }
	public decimal Field2 { get { return dependency1.Calculation2(); }
	public decimal Field3 { get { return dependency2.Calculation1(); }
	public decimal Field4 { get { return dependency2.Calculation2(); }
}

Now when we look at the constructor we can see that ‘SomeObject’ depends on those 2 classes and when we want to see what a call to ‘Field1′ does we can see straight away instead of having to scroll back up to the constructor to check.

In a way we have tightened the coupling between ‘SomeObject’ and ‘Dependency1′ and ‘Dependency2′ by storing those in fields and I was pondering the wisdom of doing this sort of thing in a previous post but I think I prefer to take this coupling over the alternative choice.

The choice isn’t quite as clear cut if we are getting a value from a dependency and then performing a calculation on that value.

For example:

public class SomeObject
{
	public SomeObject(Dependency1 dependency1)
	{
		Result = CalculateValueFrom(dependency1.OtherDependency);
	}
 
	public decimal Result { get; set; }
}

In this case I would still favour storing ‘Dependency1′ and then executing that calculation later although a law of demeter violation is more often than not staring us in the face and perhaps this isn’t the right class to be performing this calculation.

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.