Mark Needham

Thoughts on Software Development

Archive for September, 2009

Scala: 99 problems

with 8 comments

My colleague Liz Douglass and I have been playing around with Scala and Liz recently pointed out Phil Gold’s ‘Ninety Nine Scala Problems‘ which we’ve been working through.

One in particular which is quite interesting is number 7 where we need to flatten a nested list structure.

Therefore given this input:

flatten(List(List(1, 1), 2, List(3, List(5, 8))))

We would expect this output:

res0: List[Any] = List(1, 1, 2, 3, 5, 8)

I tried this out on my own using recursion but kept ending up creating a stack overflow by writing code that never terminated!

We decided to try it out while pairing together using a more imperative approach so that we could get something that actually worked to begin with and ended up with this:

def flatten[A](list : List[A]) : List[A] =  {
 
	var flattenedList : List[A] = Nil
	for(item <- list) {
		item match {
		   case x : List[A] => flattenedList = flatten(x) ::: flattenedList
           case _    => flattenedList = item :: flattenedList
		}
	}
	flattenedList
}

It works and we’ve got a bit of recursion going on but we’re still mutatating the state of a variable so it feels like we’re missing out on the strength of a language which provides us with the ability to work in a functional way.

Our second attempt made use of the knowledge we gained from this first attempt with respect to that cases that existed for flattening the list and we made use of a technique which I picked up while learning a bit of F# – the inner function which keeps an accumulator with the flattened list on each recursion:

def flatten[A](list : List[A]) : List[A] = {
	def flat[A](rest : List[A], flattenedList : List[A]) : List[A] = rest match {
			case Nil => flattenedList
			case (head:List[A])::tail =>  flat(tail, flat(head, List()) ::: flattenedList)
			case head::tail => flat(tail, head :: flattenedList) 		
	}
 
	flat(list, Nil).reverse
}

I like the fact that we don’t have to mutate any state with this solution but the second case statement is pretty difficult to understand.

We had to distinguish between whether the head of the list was another list or just an element in order to determine whether we needed to break that value down further or just add it to the new list.

Is there a better way to do that than to explicitly state the type of head as we’ve done here?

Another colleague, Ben Barnard, came over to work with us on the next iteration of this solution which was somewhat driven by seeing how Phil Gold had solved the kth item problem for which he describes a solution which narrows down the initial list each time while still calling the same function:

  def nth[T](n: Int, l: List[T]): T = (n, l) match {
    case (0, e :: _   ) => e
    case (n, _ :: tail) => nth(n - 1, tail)
    case (_, Nil      ) => throw new NoSuchElementException
  }

In this case when we reduce the list we are searching on we also reduce the index that we are looking for by one which works quite nicely.

For the flatten function we were able to recurse on the original function by running the list back through the flatten function whenever we didn’t know if the ‘head’ was an element:

def flatten[A](list : List[A]) : List[A] = list match {
	case Nil => list
	case (head:List[A])::tail => flatten(head) ::: flatten(tail)
	case head::tail => head :: flatten(tail)
}

The nice thing about this solution is that the list is in the correct order at the end so we don’t need to reverse it as we did with the previous solutions.

This way of coding is still not quite intuitive to me but I think the solution is cleaner and easier to read than the others.

Written by Mark Needham

September 30th, 2009 at 11:39 pm

Posted in Scala

Tagged with

Book Club: Design Sense (Michael Feathers)

with 2 comments

In our latest technical book club we discussed a presentation given at the Norwegian Developers Conference by Michael Feathers titled ‘Design Sense‘.

In this presentation he presents quite a number of different ideas that he has learned from his experiences in software development over the years.

These are some of my thoughts and our discussion:

  • The first part of the presentation talks about method size and Feathers observes that there seems to be a power law with relation to the size of methods in code bases – i.e. there are a lot of small methods and fewer large methods but there will always be some a very small number of massive methods.

    Dave suggested that perhaps this is related to Benford’s Law which describes exponential growth processes.

    I wonder whether this observation is somehow linked to the broken window theory whereby if a method is large then it is likley to increase in size since it probably already has some problems so it doesn’t seem so bad to throw some more code into the method. With small methods this temptation might not exist.

    From what I’ve noticed the messiest methods tend to be around the edges of the code base where we are integrating with other systems – in these cases there is usually a lot of mapping logic going on so perhaps the benefit of extracting small methods here is not as great as in other parts of the code base.

  • I really like the observation that protection mechanisms in languages are used to solve a social problem rather than a technical problem.

    For example if we don’t want a certain class to be used by another team then we might ensure that it isn’t accessible to them by ensuring it’s not public or if we dont’ want certain methods on a class to be called outside that class then we’d make those methods private.

    I think this is a reasonable approach to take to protect us from although it was pointed out that in some languages, like Python, methods are publically accessible by default and the idea when using private methods is to make it difficult to access them from outside the class but not impossible. I guess this is the same with most languages as you could use some sort of metaprogramming to gain access to whatever you want if needs be.

    There’s an interesting post on the Python mailing list which talks through some of the benefits of using languages which don’t impose too much control over what you’re trying to do.

  • The observation that names are provisional is something that I’ve noticed quite a lot recently.

    Feathers points out that we are often reluctant to change the names of classes even if the responsibility of that class has completely changed since it was originally created.

    I’ve noticed this a lot on projects I’ve worked on and I wonder if this happens because we become used to certain types being in the code and there would be a period of adjustment for everyone in the team while getting used to the new names – it might also ruin the mental models that people have of the system.

    Having said that I think it’s better to have names which describe what that class is actually doing now rather than keeping an old name which is no longer relevant.

  • I quite like the idea that the physical architecture of a system can shape the logical architecture and that often we end up with a technological solution looking for a problem.

    I’m not sure if it’s completely related but one way that this might happen is that in an environment where we structure a team in layers it’s possible that certain functionality will end up being implemented by the team that can turn it around the quickest rather than being implemented in the layer where it might best logically belong.

    He also mentions Conway’s law which suggests “…organizations which design systems … are constrained to produce designs which are copies of the communication structures of these organizations.” – that seems to link in quite closely with the above ideas.

  • Amongst the other ideas suggested I quite liked the idea that requirements are just design decisions but that they are product design decisions rather than technical design decisions.

    I like this as it opens up the idea that requirements aren’t set in stone and that we can work with them to come up with solutions that actually solve business problems instead of ‘design by laundry list’ as Feathers coins it.

    I think his recent post about the boutique software shop explains these ideas in more detail.

There’s a lot of other ideas in this talk as well but these are some of the ones that made me think the most.

Written by Mark Needham

September 30th, 2009 at 12:42 am

Posted in Book Club

Tagged with

Learning from others/Learning yourself

with 2 comments

Something which has become quite apparent to me recently is that I learn things far more quickly if I try it out myself and make mistakes than if I just rely on someone else’s word for it but some more experienced colleagues seem able to use information explained to them fair more effectively and don’t necessarily need to go through this process.

While reading through the Dreyfus Model one of the ideas that is suggested is that once people reach the level of ‘Proficient’ at any given skill then they are able to learn from the experiences of others without needing to experience something themselves.

Andy Hunt sums this up quite nicely in Pragmatic Learning and Thinking :

Proficient practitioners make a major breakthrough on the Dreyfus model: they can correct previous poor task performance. They can reflect on how they’ve done and revise their approach to perform better the next time. Up until this stage, that sort of self-improvement is simply not available.

Also, they can learn from the experience of others. As a proficient practitioner, you can read case studies, listen to water cooler gossip of failed projects, see what others have done, and learn effectively from the story, even though you didn’t participate in it firsthand.

If you’re not at that level yet it therefore seems to be the case that it’s necessary to experience this situations in order to learn effectively.

The trick thing then is to engineer an environment where it is possible to learn but not one where you jeopardise a code base by doing so.

In a way this is what we did with the setter/constructor injection problem that I described previouslyErik pointed out the disadvantages that we would get from using constructor injection on a class which many classes derived from but I didn’t fully appreciate his concerns and I couldn’t picture the problems we would face until we spent a bit of time putting the dependency into the constructor and I was able to see how this dependency would cascade down into the 30 or so sub classes.

Since we were trying out a setter based injection approach alongside the constructor injection based approach in a time boxed exercise we were able to just throw away the constuctor based injection code when we saw that it wasn’t quite going to work and so no harm was done to the code base.

I sometimes feel that a similar type of learning is probably going on in other situations as well.

One fairly common area of contention on projects I’ve worked on is around whether or not we should put stub/mock calls in a setup method in a test fixture.

On a project I worked on a couple of years ago we ran ourselves into a world of pain by putting mock/stub calls there because when we wanted to test a specific interaction in a test there would often be an unexpected failure because of those calls in the setup method.

It was never entirely obvious what had happened and we would end up wasting a lot of time before we realised what had happend.

As a result of that I’ve been pretty much against that approach although I can see why we might want to do this. It becomes very tedious typing out the same setup code in each of our tests.

Abstracting away that common setup into a method or series of methods which we explicitly call in each test provides one way to solve the problem but it takes the test setup one more click away from us when we’re analysing test failures so I’m not sure whether it solves all our problems.

My current thinking is that perhaps we can make use of the setup method as long as all the tests defined in that test fixture require exactly the same setup but that as soon as we are writing a test which requires a different setup we need to pull that test out into its own test fixture.

The problem with this is that we end up splitting our tests based on how they test interaction with a dependency which seems very strange to me.

I still don’t have the answer for the best way to solve this problem but I think I know a bit better from having experimented with some ideas than I would have by just listening to someone tell me about their experiences.

From my understanding of the Dreyfus Model, someone who’s really good at unit testing wouldn’t necessarily need to do this but would instead be able to learn from someone else’s experiences just by talking to them about it.

I’m assuming that they would be able to do this because they have a wealth of similar situations to relate your stories to and they can therefore fill in any gaps in the knowledge they have.

I think this aspect of the Dreyfus Model helps identify why just explaining something from your experience rarely seems to be successful – unless the other person is ‘Proficient’ at the skill in question they won’t easily be able to relate your experience to the problem without some experimentation of their own.

Perhaps there’s more to it and I’m way off the mark but this theory at least seems reasonably plausible to me

Written by Mark Needham

September 28th, 2009 at 12:02 am

Posted in Learning

Tagged with

The Duct Tape Programmer: Some thoughts

with 9 comments

I just came across quite an insightful post by Jak Charlton titled ‘Ship it or Ship out‘ in which he talks about the importance of shipping the software we work on, referring to Joel’s recent post ‘The Duct Tape Programmer‘.

Unit testing

When I first read Joel’s post I didn’t really like it because it seems to downplay the role of unit testing when coding, something which I believe is quite important from my experience of software development so far.

What didn’t quite make sense to me is that Joel has released the StackOverflow website and it seems to work reasonably well and from my brief use of it I haven’t seen any glaring errors which might indicate a lack of quality.

Having read Jak’s post and some of the comments on his post it seems to me that perhaps the situations in which I work and which Joel describes are just different to each other and that neither of our approaches is inherently good or bad.

FriedBob sums this up quite nicely in the comments section:

What it boils down to is that you can’t make a blanket statement about an sort of programming practice and say this is the best way every time every situation.

You have to use the best tool and approach for the job, and every situation is different.

You have to find a balance between “duct tape programming”, structured tool/framework based coding, code quality, shipping date and maintainability. Find the right mix, and you won’t really have to make any significant sacrifices in any of these areas.

In the type of work I do we typically have a team with multiple developers working together on something and I think the approach to coding in this environment needs to be slightly different to what it might be if you were working alone for example.

When I’m working alone I often just hack things together because I’m generally learning new things and it’s pretty much throwaway code that noone else will ever need to understand.

The problem with this approach comes about when you want to make changes to the code – if it’s been written in a rushed/hacky fashion then it can be really difficult to remember why you did it that way and having unit tests can help in this case.

If the code never needs to be changed then the value of having these tests is reduced to giving us quick feedback on whether or not our code does what we expect it to.

In the projects I work on we need to change the code all the time and in this case tests are also quite useful for giving us the ability to do this while providing a degree of confidence that we haven’t broken something else.

In fact typically it might be someone other than the original writer of the code who needs to make changes so the tests can also give them some idea of what the code is supposed to be doing.

When working on code alone this type of knowledge would be in your head. When working on a team we need to find some way of expressing this so that our team mates don’t need to talk to us every time they want to make a change to the code.

Since I work on code bases with many other people and we want to frequently change our code as we discover better ways of doing things or the business changes their requirements we need some sort of safety net to allow us to do that – unit tests are part of that safety net.

Perhaps Joel doesn’t have any of those and that’s why he doesn’t need the safety net.

Uncle Bob has also written a post where he addresses this and points out that writing unit tests doesn’t necessarily slow us down.

Business/Developer alignment

Jak’s observation on how the business and developers are fundamentally not aligned is quite astute:

And we know that better code will be easier to maintain – and developers HATE bug fixing duty, so we want to ensure we don’t have to do much of it.

To a developer, the most important factor in any project is Quality, followed by Time (because we know our bosses will be mad if we miss our deadlines), but we are happy to slip Functionality because it matters little to us if a new feature isn’t included in the release.

I’ve actually observed that while that might be true for some developers, in general there seems to be an almost unspoken desire to get story points on the board instead of taking a bit more time to ensure that the code we write now won’t hinder our ability to deliver functionality that we need to write, perhaps as soon as the next iteration.

I’ve written a couple of posts recently where I describe the struggle a colleague and I have had working with code that was written in a rushed fashion.

Personally I find myself getting very demotivated when we end up just hacking code together to get it done as quickly as possible and I think Jak is spot on in identifying that the reason for this is that I know the code is going to be more difficult to maintain in the future when we take this approach.

I can certainly see the benefit in hacking some code in if we are near to a release and don’t want to risk rewriting a whole chunk of code just to design it well but in normal development time I prefer to take the time to write something that will be reasonably easy to work with in the future.

I think there is some benefit to the business in knowing about this trade off since if we keep on compromising on the quality of what we’re writing to get more features in it will eventually become very difficult to add these in a timely manner at which stage we will look quite stupid.

Overall

It’s quite interesting to hear different opinions on ways to deliver software since everyone seems to do this slightly differently to each other.

Even though I don’t think I’ll be changing the way I approach software development these posts have certainly made me think more about my favoured approach and why we do the things we do.

Written by Mark Needham

September 26th, 2009 at 5:16 pm

TDD: It makes you question what you’re doing

without comments

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.

Written by Mark Needham

September 25th, 2009 at 11:48 pm

Posted in Testing

Tagged with

Book Club: Versioning your database (K. Scott Allen)

with 5 comments

In our latest technical book club we discussed a series of posts written by K. Scott Allen about getting your database under version control.

These are some of my thoughts and our discussion:

  • We had an interesting discussion around when it’s ok to go and change checked in change scripts – on previous projects I’ve worked on we’ve actually had the rule that once you’ve checked in a change script to source control then you can no longer change it but instead need to add another change script that does what you want.

    Several colleagues pointed out that this approach can lead to us having quite a lot of fine grained change scripts – each maybe adding or altering one column – and that an approach they had used with some success was to allow changes to be made up until a change script had been applied in an environment outside of the team’s control, typically UAT.

  • On a previous project I worked on where we didn’t script the database, we made use of Hibernate annotations to specify the mappings between domain objects and the database so the SQL code for database creation was being auto generated by a Hibernate tool.

    This doesn’t seem to fit in with the idea of allowing us to incrementally change our database but Tom pointed out that it might be quite a useful way to get an initial baseline for a database. After that we probably want to make use of change scripts.

  • We discussed the coupling between the model of a system that we have in a database and the one that exists in our code when using ORM tools and how we often end up with these two different models being quite tightly coupled meaning that changing our domain model can become quite difficult.

    Tom pointed out that the database is just another data source but that it often seems to be treated differently to other data sources in this respect.

    He also suggested the idea of creating an anti corruption layer in our code between the database and our domain model if they start to become too different rather than trying to keep them as similar as possible by some imaginative use of ORM mappings.

  • Another interesting are of discussion was around how to deal with test data and existing data in the system with respect to our change scripts.

    On projects I’ve worked on if we had reference data that was required across all environments if we wanted to make changes to this data then we would just make use of another change script to do that.

    Ahrum suggested that if we had environment specific test data then we’d probably have other environment specific scripts that we could run after the change scripts had been executed.

    One of the more interesting problems when making changes to tables which already have data in is working out what we want to do with the existing data if we change the type of a column.

    We can typically either create a temporary column and copy all the values there first before creating an update script that converts each of the values to the data type of the new column or we can just forget about the data which is a less acceptable options from what I’ve seen.

    The effort involved in doing this often seems to mean that we are more reluctant to make changes to column types after their initial creation which means that we need to choose the type quite accurately early on.

  • A couple of colleagues pointed out that one benefit they had found with taking this approach to working with databases actually helped to expose any problems in the process – there can’t be any under the table changes made to databases if they are being manipulated by change scripts otherwise we end up with different setups in different environments.

    Since we will be recreating and updating databases quite frequently these problems will become obvious quite quickly.

  • Dean pointed out that the change script approach is actually really useful for performance testing – a benefit of this approach that I hadn’t considered previously considered.

    When doing this type of testing we know which version of the database we were using at the time and if there suddenly becomes a performance problem then it should be much easier to track down which change resulted in the problem.

Written by Mark Needham

September 24th, 2009 at 7:35 am

Posted in Book Club

Tagged with

TDD: Copying and pasting tests

with 2 comments

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.

Written by Mark Needham

September 22nd, 2009 at 11:39 pm

Posted in Testing

Tagged with

TDD: Tests that give us a false confidence of coverage

with 4 comments

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.

Written by Mark Needham

September 21st, 2009 at 10:49 pm

Posted in Testing

Tagged with

TDD: Keeping test intent when using test builders

with 4 comments

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.

Written by Mark Needham

September 20th, 2009 at 12:06 pm

Posted in Testing

Tagged with ,

Set Based Concurrent Engineering: A simple example

with 6 comments

One of my favourite ideas that I came across while reading the Poppendieck’s Lean Software Development is set based concurrent engineering which encourages us to keep our options open with regards to the solution to a problem until we absolutely need to decide on an approach after which we probably can’t easily change that decision so we will most likely stick with it.

I like the idea but on the projects I’ve worked on we often seem to take a more point based approach – there will be some discussion up front on the potential solutions to a problem and eventually one of them will be considered to be the best solution and we go and implement that one.

Last week we were doing more work on getting an authenticated user into our system and on this occasion we were trying to work out the best way to get some of the user’s details into our ‘Service’ base class so that we could send down the user’s SAML token with specific requests.

We identified two solutions to do this – inject the user into the Service through a constructor or inject it via a property.

A colleague and I were in favour of the constructor based approach since it results in us creating an object which has all its dependencies ready at creation.

Erik and another colleague favoured the introduction of setter injection in this case since we could just add that to the top level abstract class and avoid the problem that we would have with constructor injection whereby every sub class would need to have that constructor defined.

public abstract class Service
{
	public Service(User user)
	{
 
	}
}
 
public class SomeService : Service
{
	public SomeService(User user) : base(user) { }
	// every sub class of Service would need this constructor
}

We decided to try out both of these solutions concurrently for a time boxed period and then go with the one which looked like it was working out better.

While pursuing the constructor based approach we actually ran into a problem trying to resolve the services – it seems to be a bug in Unity Container version 1.2 which doesn’t allow you to use parameters in the constructor of a type which is being intercepted by a ‘VirtualMethodInterceptor’ which we are making use of for caching some service requests.

We spent a bit of time trying to debug this to work out what was going on but the code eventually fails when trying to emit some IL code.

I tried the latest version of the container out separately and it seemed to work correctly for the same situation so I think the bug has been fixed now.

By this time the time box was up and my other colleague had got further with the setter based approach than we had managed to get so we decided to go with that approach instead.

I think it was still an interesting approach to take as we learnt a bit more about how the container works and we got to see the trade off that we would be making if we used constructor injection between keeping our objects ‘pure’ and creating a lot of boiler plate code to create the constructor for each new service class – we went through about 30 classes putting the new constructor in!

Written by Mark Needham

September 19th, 2009 at 2:24 am