Mark Needham

Thoughts on Software Development

Brownfield Application Development in .NET: Book Review

without comments

The Book

Brownfield Application Development in .NET by Kyle Baley and Donald Belcham

The Review

I asked to be sent this book to review by Manning as I was quite intrigued to see how well it would complement Michael Feather's Working Effectively with Legacy Code, the other book I'm aware of which covers approaches to dealing with non green field applications.

What did I learn?

  • The authors provide a brief description of the two different approaches to unit testing - state based and behaviour based - I'm currently in favour of the latter approach and Martin Fowler has a well known article which covers pretty much anything you'd want to know about this topic area.
  • I really like the section of the book which talks about 'Zero Defect Count', whereby the highest priority should be to fix any defects that are found in work done previously rather than racing ahead onto the next new piece of functionality:

    Developers are geared towards driving to work on, and complete, new features and tasks. The result is that defect resolution subconsciously takes a back seat in a developer’s mind.

    I think this is quite difficult to achieve when the team is getting pressure to complete new features but then again it will take longer to fix defects if we leave them until later since we need to regain the context around them which is more fresh in our mind the earlier we fix them.

  • Another cool idea is that of time boxing efforts at fixing technical debt in the code base - that way we spend a certain amount of time fixing one area and when the time's up we stop. I think this will work well as an approach as often when trying to fix code we can either get into the mindset of not fixing anything at all because it will take too long to do so or ending up shaving the yak in an attempt to fix a particularly problematic area of code.
  • I like the definition of abstraction that the authors give:

    From the perspective of object- oriented programming, it is the method in which we simplify a complex “thing”, like an object, a set of objects, or a set of services.

    I often end up over complicating code in an attempt to create 'abstractions' but by this definition I'm not really abstracting since I'm not simplifying but complicating! This seems like a useful definition to keep in mind when looking to make changes to code.

  • Maintainability of code is something which is seriously undervalued - I think it's very important to write your code in such a way that the next person who works with it can actually understand what's going on. The authors have a fantastic quote from Perl Best Practices:

    Always code as if the guy who ends up maintaining your code is a violent psychopath who knows where you live.

    Writing code that is easy for the next person to understand is much harder than I would expect it to be although on teams which pair programmed frequently I've found the code easier to understand. I recently read a blog post by Jaibeer Malik where he claims that it is harder to read code than to write code which I think is certainly true in some cases.

  • There is a discussion of some of the design patterns and whether or not we should explicitly call out their use in our code, the suggestion being that we should only do so if it makes our intent clearer.
  • While describing out how to refactor some code to loosen its dependencies it's pointed out that when the responsibilities of a class are a bit fuzzy the name of the class will probably be quite fuzzy too - it seems like this would server as quite a useful indicator for refactoring code to the single responsibility principle. The authors also suggest trying not to append the suffix 'Service' to classes since it tends to be a very overloaded term and a lot of the time doesn't add much value to our code.
  • It is constantly pointed out how important it is to do refactoring in small steps so that we don't break the rest of our code and to allow us to get rapid feedback on whether the refactoring is actually working or not. This is something that we've practiced in coding dojos and Kent mentions it as being one of his tools when dealing with code - I've certainly found that the overall time is much less when doing small step refactorings than trying to do everything in one go.

    I'm quite interested in trying out an idea called 'Bowling Scorecards' which my former colleague Bernardo Heynemann wrote about - the idea to have a card which has a certain number of squares, each square reprsenting a task that needs to be done. These are then crossed off as members of the team do them.

  • An interesting point which is made when talking about how to refactor data access code is to try and make sure that we are getting all the data from a single entry point - this is something which I noticed on a recent project where we were cluttering the controller with two calls to different repositories to retrieve some data when it probably could have been encapsulated into a single call.
  • Although they are talking specifically about poor encapsulation in data access layers, I think the following section about this applies to anywhere in our code base where we expose the inner workings of classes by failing to encapsulate properly:

    Poor encapsulation will lead to the code changes requiring what is known as the Shotgun Effect. Instead of being able to make one change, the code will require you to make changes in a number of scattered places, similar to how the pellets of a shotgun hit a target. The cost of performing this type of change quickly becomes prohibitive and you will see developers pushing to not have to make changes where this will occur.

  • The creation of an anti corruption layer to shield us from 3rd party dependency changes is suggested and I think this is absolutely vital otherwise whenever there is a change in the 3rd party code our code breaks all over the place. The authors also adeptly point out:

    The reality is that when you rely on another company's web service, you are ultimately at their mercy. It's the nature of third-party dependencies. You don't have control over them.

    Even if we do recognise that we are completely reliant on a 3rd party service for our model I think there is still a need for an anti corruption layer even if it is very thin to protect us from changes.

    The authors also describe run time and compile time 3rd party dependencies - I think it's preferable if we can have compile time dependencies since this gives us much quicker feedback and this is an approach we used on a recent project I worked on by making use of generated classes to interact with a SOAP service rather than using WCF message attributes which only provided us feedback at runtime.

In Summary

This book starts off with the very basics of any software development project covering things such as version control, continuous integration servers, automated testing and so on but it gets into some quite interesting areas later on which I think are applicable to any project and not necessarily just 'brownfield' ones.

There is a lot of useful advice about making use of abstractions to protect the code against change both from internal and external dependencies and I particularly like the fact that the are code examples showing the progression of the code through each of the refactoring ideas suggested by the authors.

Definitely worth reading although if you've been working on any type of agile projects then you're probably better off skim reading the first half of the book but paying more attention to the second half.

Written by Mark Needham

July 6th, 2009 at 12:43 am

Posted in Books

Tagged with , ,

Domain Driven Design: Conformist

with 2 comments

Something which constantly surprises me about Domain Driven Design is how there is a pattern described in the book for just about every possible situation you find yourself in when coding on projects.

A lot of these patterns appear in the 'Strategic Design' section of the book and one which is very relevant for the project I'm currently working on is the 'Conformist' pattern which is described like so:

When two development teams have an upstream/downstream relationship in which the upstream has no motivation to provide for the downstream team's needs, the downstream team is helpless. Altruism may motivate upstream developers to make promises, but they are unlikely to be fulfilled. Belief in those good intentions leads the downstream team to make plans based on features that will never be available. The downstream project will be delayed until the team ultimately learns to live with what it is given. An interface tailored to the needs of the downstream team is not in the cards.

We are working on the front end of an application which interacts with some services to get and save the data from the website.

We realised that we had a situation similar to this originally but didn't know that it was the conformist pattern and our original approach was to rely completely on the model in the service layer to the extent that we were mapping directly from SOAP calls to WCF message objects and then passing these around the code - I originally described this as being an externally defined domain model.

This led to quite a lot of pain as whenever there was a change in the service layer model our code base broke all over the place and we then ended up spending most of the day fire fighting - we were too tightly coupled to an external system.

At this stage we were reading Domain Driven Design in our Technical Book Club and I was fairly convinced that what we really needed to do was have our own model and create an anti corruption layer to translate between the service layer model and the new model that we would create.

We changed our code to follow this approach and created repositories and mappers which were the main places in our code base where we cared about this external dependency and although the isolation of the end point has worked much better we never really ended up with a rich domain model that really represented the business domain.

We had something in between the service layer model and the real business model which didn't really help anyone and meant we ended up spending a lot of time trying to translate between the different definitions that were floating around.

Writing the code for the anti corruption layer also takes a lot of time, is quite frustrating/tedious and it was hard to see the value we were getting from doing so.

We've now reached the stage where we know this is the case and that it probably makes much more sense to just accept it and to not spend any more time trying to create our own model but instead just adapt what we have to more closely match the model we get from the services layer.

We will still keep a thin mapping layer as this gives us some protection against changes that may happen in the service layer.

I think a key thing for me here is that it's really easy to be in denial about what is actually happening since what you really want is to be in control of your own domain model and design it so that it closely matches the business so that they would be able to read and understand your code if they wanted to. Sometimes that isn't the case.

Chatting with Dave about this he suggested that a lesson for us here is that it's important to know which pattern you are following which Andy Palmer also pointed out on twitter.

Written by Mark Needham

July 4th, 2009 at 10:17 am

Coding Dojo #19: Groovy Traveling salesman variation

without comments

Our latest coding dojo involved working on a variation of the traveling salesman problem in Groovy again.

The Format

We had 8 people participating this week so we returned to the Randori format, rotating the pair at the keyboard every 7 minutes.

Give the number of people it might have actually been better to have a couple of machines and use the UberDojo format.

What We Learnt

  • The importance of just getting started stood out a lot for me in this dojo - there have been quite a few times when we've met intending to do some coding and spent so long talking about coding that we didn't end up writing anything. Luckily Dave took the lead in this dojo and got the ball rolling. The code we wrote originally wasn't perfect but it helped create the momentum to keep the session going so it was valuable in that way.
  • Another interesting feature of dojos for me is that it really doesn't matter if you make mistakes - if you write really terrible code in a dojo it's probably a good thing since you'll probably not go and repeat the same mistake on a real project. I learnt a lot about the perils of not refactoring early enough and having too much state in our code from our Isola Dojo a few months ago.
  • We refactored much earlier than we normally do in this dojo and I think it worked really well for allowing us to progress later on. Often we fall into the trap of just chasing the green bar a bit too much and we forget to clean up the code after each cycle but we had that a bit better in this one.

    We also backed up a bit after around 3 cycles after realising that the code was becoming a bit horrific and spent 1 cycle working it into shape for the next one.

  • We fell into the trap of going several cycles with broken tests while trying to do some redesign on the code - the steps were clearly not small enough!

    Later on we corrected this when refactoring the code into a more functional style by taking very small steps and running the tests after each small change - this was a far more effective approach.

  • Although we were working in a dynamic language it didn't feel that the conversations were that different when discussing the code - we were still talking about types when working out what to do. I'm not sure whether this means we haven't quite got the idea of dynamic languages or whether there isn't such a big difference between the way you talk about your code in them.

For next time

  • We might continue with another problem in Groovy - it's been quite fun working in a language that runs on the JVM without the verbosity you sometimes get when writing Java code.

Written by Mark Needham

July 4th, 2009 at 9:36 am

Posted in Coding Dojo

Tagged with

F#: Pattern matching with the ':?' operator

without comments

I've been doing a bit more reading of the Fake source code and one interesting thing which I came across which I hadn't seen was an active pattern which was making use of the ':?' operator to match the input type against .NET types.

  let (|File|Directory|) (fileSysInfo : FileSystemInfo) =
    match fileSysInfo with
      | :? FileInfo as file -> File (file.Name)
      | :? DirectoryInfo as dir -> Directory (dir.Name, seq { for x in dir.GetFileSystemInfos() -> x })
      | _ -> failwith "No file or directory given."

I thought maybe this was just a wild card operator to say that we don't care what the value is as long as it matches 'FileInfo' or 'DirectoryInfo' respectively but I couldn't see it defined on the list of operators on the Microsoft Research website.

A bit of googling led me to Matthew Podwysocki's post about pattern matching which explained the purpose of the operator (about 1/3 of the way down):

What the above example does is check for the corresponding .NET types by using the ':?' operator especially reserved for this behavior.

I've been playing around with a simple 'add' function to try and understand F#'s type inference and one thing I noticed is that if you just define it with minimal code you end up with a function which takes in 2 integers and returns an integer as the result:

let add a b = a + b
 
val add: int -> int -> int

I had thought that the signature and result of that function might remain generic due to the fact that there are more types than just 'int' with which you can make use of the addition operator.

For example, it is possible to add two string together but in fact you need to be more explicit about that:

let add (a:string) (b:string) = a + b
 
val add: string -> string -> string

From what I can tell if we wanted to write a generic add function we would need to do something like this - I originally tried just returning 'new A + new B' from each of the pattern matches but the return type of add3 then becomes 'string' since the first path in the pattern matching returns a 'string'.

    let add3 a b =
        match (box a,box b) with
            | (:? string as newA),(:? string as newB) -> newA +  newB |> box
            | (:? int as newA),(:? int as newB) -> newA + newB |> box
            | (:? decimal as newA),(:? decimal as newB) -> newA + newB |> box
            | _ -> failwith "you can't add these together"

Which is slightly verbose and has a type of "'a -> 'b -" obj' - I haven't been able to work out whether it's possible to create a generic function like this without needing to cast the result down to 'obj'.

I thought it might be possible to get rid of the boxing by making use of the downcast operator:

You can also use the downcast operator to perform a dynamic type conversion. The following expression specifies a conversion down the hierarchy to a type that is inferred from program context.

I tried surrounding the 'newA + new B |> box' code with a call to 'downcast' but that just resulted in the following error message when trying to make use of the function:

Value restriction. The value 'it' has been inferred to have generic type
	val it : '_a
Either define 'it' as a simple data term, make it a function with explicit arguments or, if you do not intend for it to be generic, add a type annotation.

I'd be intrigued to see if anyone has worked out how to do this as I'm out of ideas.

Written by Mark Needham

July 2nd, 2009 at 11:10 pm

Posted in F#

Tagged with ,

Book Club: Logging - Release It (Michael Nygaard)

without comments

Our latest technical book club session was a discussion of the logging section in Michael Nygard's Release It.

I recently listened to an interview with Michael Nygard on Software Engineering Radio so I was interested in reading more of his stuff and Cam suggested that the logging chapter would be an interesting one to look at as it's often something which we don't spend a lot of time thinking about on software development teams.

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

  • An idea which Nick introduced on a project I worked on last year was the idea of having a 'SupportTeam' class that could be used to do any logging of information that would be useful to the operations/support team that looked after our application once it was in production.

    This is an approach also suggested by Steve Freeman/Nat Pryce in Growing Object Oriented software (in the 'Logging is a feature' section) and the idea is that we will then focus more on logging the type of information that is actually useful to them rather than just logging what we think is needed.

    One thing which Dave pointed out is that it's often difficult to get access to the operations team to try and get their requirements for the type of logging and monitoring they need and so often ends up being something that's done very late on. On projects I've worked on there has often been a story card for logging and I think this is a good way to go as they are a stakeholder of the system so logging shouldn't just be dealt with as a nice extra.

  • Something which I hadn't considered until reading this book is the idea of making logs human readable and machine parseable as well. The default format of most of the logging tools is not actually that useful when you're trying to scan through hundreds of lines of data and it was intriguing how a little indentation could improve this so dramatically with the added benefit of making it much easier to create a regular expression to find what you want.
  • One thing I'm interested in understanding is how we work out what's too much logging and what's too little since it seems that it seems that the answer to this question is fairly context sensitive. For example on a recent project we logged all unhandled exceptions that came from the system as well as any exceptions that happened when retrieving data from the service layer. In general the data we've had available has been enough to solve problems but we could probably have done more, just working out what would be useful doesn't seem obvious.
  • I think it was Alex who pointed out that it's often useful to have an explicit step in the build to remove any debug logging from the code so that it doesn't end up in production by mistake. This seems like a pretty neat idea although I haven't seen it done yet - it also leads towards the idea that logging is for the operations team which I think is correct although it is often suggested that logging is actually for developers since it is assumed that they would be the ones to eventually solve any problems that arise.
  • The idea of having message codes for specific errors messages seems like a really cool idea for allowing easy searching of log files - we've done this on some projects I've worked on and not on others. I guess the key here is to ensure we don't end up with too many different error codes otherwise it's just as confusing as not having them at all.

Written by Mark Needham

July 2nd, 2009 at 12:04 pm

Posted in Book Club

Tagged with , ,

F#: What I've learnt so far

without comments

I did a presentation of some of the stuff that I've learnt from playing around with F# over the last six months or so at the most recent Alt.NET Sydney meeting.

I've included the slides below but there was also some interesting discussion as well.

  • One of the questions asked was around how you would deal with code on a real project with regards to structuring it and ensuring that it was maintainable. I'm not actually sure what the answer is to this question as I haven't written any code in F# that's in production but there are certainly applications written n F# that are in production - the main one that I know a bit about is one which Amanda Laucher worked on which she spoke about at the Alt.NET conference in Seattle.
  • There was some discussion about dynamic v static languages - Phil spoke of not caring about what type something is rather caring about what it does. I pretty much agree with this and I think when using languages which have quite strong type inference such as F# (and more-so Haskell from what I hear) then I think we do move more towards that situation.
  • Erik raised the point that functional languages aren't the solution for everything and I certainly feel it's niche is probably around operations with heavy data parsing/mining involved. I'm not sure I'd fancy doing an ASP.NET MVC application only in F# although I've seen some WPF code written using F# (unfortunately can't remember where) which looked reasonable so I'm not sure we should write it off just yet.

I've put the code that I walked through in the presentation on bitbucket.

Written by Mark Needham

June 30th, 2009 at 11:09 pm

Posted in F#

Tagged with ,

F#: Setting properties like named parameters

without comments

One of the most frustrating things for me lately about interacting with C# libraries from F# has been setting up objects through the use of properties.

While I am against the use of setters to construct objects in the first place, that's the way that a lot of libraries work so it's a bit of a necessary evil!

In C# we would typically make use of the object initializer syntax to do this, but in F# I've been writing code like this to do the same thing:

type MessageBuilder(?message:string, ?user:string) =              
	let buildMessage message user =
	   let twitterStatusBuilder = new TwitterStatus()
	      twitterStatusBuilder.Text <- message
	      twitterStatusBuilder.User <-
	      	let userBuilder = new TwitterUser()              
	         userBuilder.ScreenName <- user
	         userBuilder
	      twitterStatusBuilder
 
	member self.Build() = 
		buildMessage (if message.IsSome then message.Value else "") (if user.IsSome then user.Value else "")

This is more verbose than strictly necessary but I wanted to try and ensure all mutations to objects were being done within the function creating it rather than creating an object and then mutating it which feels strange to me in F# land.

I recently realised that it's actually possible to call properties in the same way that we can create objects using named parameters.

We therefore end up with the following code:

type MessageBuilder(?message:string, ?user:string) =              
    let buildMessage message user = new TwitterStatus(Text = message, User = new TwitterUser(ScreenName = user))
 
    member self.Build() = 
        buildMessage (if message.IsSome then message.Value else "") (if user.IsSome then user.Value else "")

Which is much more concise and does the same thing.

Written by Mark Needham

June 29th, 2009 at 12:28 am

Posted in F#

Tagged with ,

F#: More thoughts on the forward & application operators

without comments

I've been spending a bit of time reading through the Fake source code to try and understand how it works and one of the things which I quite like about it is the way the authors have made use of different F# operators to make expressions easier to read by reducing the number of brackets that need to be written and reordering the functions/values depending on the particular context.

One which I hadn't seen before is the application operator which is the opposite of the forward operator which I have previously written about.

The application operator (<|) applies a value to a function, the function being on the left and the value on the right.

It is used in FileHelper.fs as part of the DeleteFile function:

let DeleteFile x =   
  let file = new FileInfo(x)    
  if file.Exists then 
    log <| sprintf "Deleting %s" file.FullName
    file.Delete()
  else
    log <| sprintf "%s does not exist." file.FullName

The log function is of type 'string -> unit' and the sprintf call helps create that string. Without the application operator we would have to put in extra parentheses:

log (sprintf "Deleting %s" file.FullName)

The code also makes use of the forward operator which I think panders more to the object oriented style of programming whereby you have have some data/object and then apply a method/function to that. I find that code written in this way reads more intuitively to me at the moment.

One example of this is the SetDirReadOnly function in FileHelper.fs

  let rec SetDirReadOnly readOnly (dir:DirectoryInfo) =
    dir.GetDirectories() |> Seq.iter (fun dir ->
      SetDirReadOnly readOnly dir
      setDirectoryReadOnly readOnly dir)
    dir.GetFiles() |> Seq.iter (fun file -> file.IsReadOnly <- readOnly)

In this case if we didn't have the forward operator then in theory we should be able to just put the 'dir.GetFiles()' can be passed as the second argument to 'Seq.iter':

  let rec SetDirReadOnly readOnly (dir:DirectoryInfo) =
    dir.GetDirectories() |> Seq.iter (fun dir ->
      SetDirReadOnly readOnly dir
      setDirectoryReadOnly readOnly dir)
    Seq.iter (fun file -> file.IsReadOnly <- readOnly)  dir.GetFiles()

In fact what we get is a compilation error:

Successive arguments should be separated by spaces or tupled, and arguments involving function or method applications should be parenthesized.

In this case we need to paranthesise the 'dir.GetFiles()' method call:

  let rec SetDirReadOnly readOnly (dir:DirectoryInfo) =
    dir.GetDirectories() |> Seq.iter (fun dir ->
      SetDirReadOnly readOnly dir
      setDirectoryReadOnly readOnly dir)
    Seq.iter (fun file -> file.IsReadOnly <- readOnly)  (dir.GetFiles())

Which leads to another compilation error:

Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved

In this case what we're being told is that the compiler is unable to work out the type of 'file' in the function being passed to 'Seq.iter'. We can fix this by specifically stating its type:

  let rec SetDirReadOnly readOnly (dir:DirectoryInfo) =
    dir.GetDirectories() |> Seq.iter (fun dir ->
      SetDirReadOnly readOnly dir
      setDirectoryReadOnly readOnly dir)
    Seq.iter (fun (file:FileInfo) -> file.IsReadOnly <- readOnly)  (dir.GetFiles())

It works but it seems to miss the point of getting the F# compiler to infer which types you're talking about - the forward operator simplifies the code a lot. I also think the code is more readable having 'files' at the beginning as it seems more obvious that the function is being applied to the sequence of files when written this way.

These operators are pretty cool and I've found it quite useful to look at the full list of the F# operators available on the Microsoft Research website as there may well be even more built in functions that can help simplify our code further.

Written by Mark Needham

June 27th, 2009 at 10:55 pm

Posted in F#

Tagged with

Coding Dojo #18: Groovy Bowling Game

with 2 comments

This week's dojo involved coding a familiar problem - the bowling game - in a different language, Groovy.

The code we wrote is available on bitbucket.

The Format

Cam, Dean and I took turns pairing with each other with the code projected onto a TV. As there were only a few of us the discussion on where we were taking the code tended to included everyone rather than just the two at the keyboard.

What We Learnt

  • I've sometimes wondered about the wisdom of running dojos in newer languages but this one worked quite well because Cam has been learning Groovy and he was able to point us in the right direction when we started writing Java-esque Groovy code. The particular syntax that I didn't know about was that you can define and put items into a list in a much simpler way than in Java:

    I was starting to write code like this:

    def frames = new List<Frame>()
    frames.Add(frame)

    Which Cam simplified down to:

    def frames = []
    frames << frame

    I didn't feel that I missed the static typing you get in Java although IntelliJ wasn't quite as useful when it came to suggesting which methods you could call on a particular object.

  • I'm even more convinced that using various languages functional equivalent of the 'for each' loop, in this case 'eachWith' and 'eachWithIndex' is not the way to go and we could see our code becoming very complicated when trying to work out how to score strikes thanks to our use of it!
  • I think we actually got further this time in terms of the implementation although we did slow down when it came to scoring strikes to try and work out exactly how we wanted to do it.

    Prior to this we had been following the idea of just getting the tests to pass and driving the design of the code that way but we at this stage it seemed foolish to keep doing that as the code would increased dramatically in complexity by doing so.

    The two approaches we were thinking of involved using the state pattern to determine what the current frame outcome was and then work out the cumulative score based on that by looking forward to future frames or an approach that would make use of functional collection parameters (not sure exactly which ones!) to calculate the score in a more function rather than OO way.

For next time

  • We'll probably keep going with some more Groovy as it's actually more interesting than I thought it would be. I'm also keen to do a coding dojo where we never make use of the if statement.

Written by Mark Needham

June 26th, 2009 at 6:15 pm

Posted in Coding Dojo

Tagged with , , ,

Safe refactoring: Removing object initializer, introducing builder

with 2 comments

I previously wrote about an approach we took to safely remove some duplication and I recently followed a similar mantra to replace an object initializer call which had around 40 properties being setup with a builder to try and make the code a bit easier to understand.

We did have tests checking the values being setup by the object initializer so I was already able to refactor with some degree of safety - it would probably have been possible to just create the builder and build the object from that and then delete the old code and replace it with the new but I've caused myself too many problems from doing that before that I decided to try a more incremental approach.

The idea was to have the builder and the object initializer both creating the object at the same time and as I built a property from the builder I would set it in the object initializer until eventually all of the properties were being set directly from the builder's object and then I could just return that instead - this approach feels quite similar to what Kent Beck describes as having parallel implementations in his recent presentation on Responsive Design.

The code I started with was something like this:

public Foo CreateMeAFoo() 
{
	return new Foo
	{
		Bar = "someValue",
		OtherBar = "someOtherValue",
		UltraBar = "aValue",
		...
		AnotherArgumentWayDownHere = 1
		...
		AndSoOn = "yes"
 
	}
}

I worked together with one of the business analysts on our team who pointed out that there were actually some clear groupings in what I had just been considering 'data' and we were able to put those explicit domain concepts into the code as part of the builder.

My first step however was to remove the object initializer to avoid making any silly mistakes - an example of one I have made when using object initializers is to set a property using 'Int16.Parse' and then passing in a string with a value of "53700″ which causes the method to throw an exception and the stack trace just directs you to the first line of the object initializer, making it quite difficult to work out which line has failed.

Having worked the code into a sequence of setters I gradually added methods to the builder to create the policy:

public Foo CreateMeAFoo() 
{
	var fooBuilder = new FooBuilder().AddBars("someValue", "someOtherValue", "aValue);
	var newFoo = fooBuilder.Build();
 
	var foo = new Foo();
	foo.Bar = newFoo.Bar;
	foo.OtherBar = newFoo.OtherBar;
	foo.UltraBar = newFoo.UltraBar;
	...
	foo.AnotherArgumentWayDownHere = 1
 
	return foo;
}
public class FooBuilder  
{
	private string bar;
	private string otherBar;
	private string ultraBar;
 
	public FooBuilder AddBars(string bar, string otherBar, string ultraBar)
	{
		this.bar = bar;
		this.otherBar = otherBar;
		this.ultraBar = ultraBar;
		return this;
	}
 
	public Foo Build()
	{
		var foo = new Foo();
		foo.Bar = bar;
		foo.OtherBar = otherBar;
		foo.UltraBar = ultraBar;
		return foo;
	}
}

I created some duplication by doing this - I am now creating the 'Foo' twice - but I didn't check any of the code into the main branch until I had totally replaced the original Foo creation with the builder.

I did about two of three properties at a time and then ran the tests which I thought might be too small a step but actually saved me on a couple of occasions so it probably actually saved me time.

Eventually when I had all the tests passing I got rid of the original Foo and replaced it with the one from the builder:

public Foo CreateMeAFoo() 
{
	var fooBuilder = new FooBuilder().AddBars("someValue", "someOtherValue", "aValue);
	return fooBuilder.Build();
}

This code is still in a state of transition - it is still possible to create an object with half the fields not set by passing in nulls to the builder for example - and I'm trying to work out what the best step is to fix that.

I generally prefer to have everything setup in the constructor and then you know that the object is in a good state as soon as you create it, but in this case moving everything straight into the constructor will probably make the object even more difficult to deal with.

My current thinking is to maybe check the pre conditions for creating the object inside the builder and then refactor out value objects so that there are not so many properties overall but I'm open to other ideas.

Written by Mark Needham

June 26th, 2009 at 12:02 am

Posted in Coding

Tagged with , ,