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.

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:

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:

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.

Even though we are still asking the NavigationModel things, I feel like this is related to Tell-Don’t-Ask. Perhaps we are telling NavigationModel what we would like to know, instead of asking it about its internals. The code is still west-oriented though.