Mark Needham

It strikes me that one of the most important skills to develop in software development is knowing when to keep going with an approach to a problem and when we should stop and try something else.

This situation doesn’t always happen because if we have two people available and realise before we start on the task that there is some doubt as to which solution is the most appropriate then we can adopt a set based approach whereby we try out multiple potential solutions in parallel.

However I’ve found that there are times when we might think that one solution is much better than the alternatives and we will start working on that and discard the alternatives for the moment.

Frequently the approach we choose will solve our problem but on other occasions we can end up getting a bit bogged down when it doesn’t work out as expected.

At this stage I often find that my instinct is to try and solve these problems and while I think this is certainly a valid approach it’s also very easy to end up shaving the yak.

This seems to happen much more frequently when working alone and even when we’re getting feedback which suggests that what we’re trying isn’t working we can be reluctant to back track.

I think there is some pride involved as I frequently find myself really wanting to make the current approach work as I believe that a better developer than myself would be able to do so.

Sometimes that is the case but more often than not when I work through the problem with a colleague their suggestion is to try another approach rather than continuing to struggle with the current one.

It doesn’t make a lot of sense to spend all our time going down one avenue when our goal is solve a particular problem and the solution is a means to that end.

When discussing this with Nick he pointed out that we also need to recognise when we should persevere with the approach.

One way to avoid too much yak shaving is to set a timebox by when we should either have a better idea of whether we are going to be able to solve the problem with this solution or if we need to consider an alternative.

If there’s some light at the end of the tunnel with an approach after this period then I would be inclined to keep on going but it’s sometimes difficult to tell how close we actually are and how much is wishful thinking!

I still sometimes find myself struggling to decide whether to keep going or change direction so it’d be interesting to know if anyone has ideas around doing so more effectively.

Something which I’ve noticed over the last few projects that I’ve worked on is that at the beginning when I don’t know very much at all about the code base, domain and so on is that pairing with someone to TDD something seems to make it significantly easier for me to follow what’s going on than other approaches I’ve seen.

I thought that it was probably because I’m more used to that approach than any other but in Michael Feathers’ description of TDD in ‘Working Effectively With Legacy Code‘ he points out the following:

One of the most valuable things about TDD is that it lets us concentrate on one thing at a time. We are either writing code or refactoring, we are never doing both at once.

It also temporarily removes us from the feeling of drowning in all the new information which is typical at the start of projects. In a way it reminds me of the ‘Retreat Into Competence‘ pattern from ‘Apprenticeship Patterns‘ as it gives us a chance to regain composure and take in all the new information.

We can probably make a much more useful contribution if we only need to understand one small piece of the system rather than having to understand everything which can often be the case with what Feathers coins the edit & pray approach to development.

It’s still necessary to spend some time trawling around the code base working out how everything fits together but I’m now more aware that it’s also really useful to take some time to just focus on one much smaller class or piece of functionality.

Perhaps also something to keep in mind the next time I’m pairing with someone who’s new to a project that I’ve been working on for a while.

I’ve had quite a few discussions with various different colleagues about coding consistency over the last year or so and Pat Kua and Frank Trindade have both written posts suggesting that we should look to have coding standards on projects in order to avoid the type of pain that having an inconsistent approach can lead to.

From what I’ve noticed there seem to be two reasons that we end up with inconsistent code on projects:

1. People’s personal preferences for how certain bits of code should be written vary.
2. The team is trying to incrementally move towards an improved solution to a problem encountered but isn’t completely there yet.

I think the first of these is the area which Frank and Pat are addressing in their posts.

However I think there is some overlap between the two. For example someone may rewrite a bit of code in their own style and while they would suggest that what they are doing is improving the code base their team mates may think that what they are doing is merely creating more inconsistency.

Potential coding inconsistency falls into three categories as I see it:

Personal Preferences

A simple example of this that we had on the last project I worked on was around how we should write simple conditional statements when used as guard clauses.

These were the 3 different ways that people preferred and we had all the different styles across the code base!

public String SomeMethod()
{
	if(SomeCondition()) return "hello";
}

public String SomeMethod()
{
	if(SomeCondition()) 
		return "hello";
}

public String SomeMethod()
{
	if(SomeCondition()) 
	{
		return "hello";
	}
}

We eventually agreed to use the last one because it was the least controversial choice and although people who preferred the other two approaches thought it was too verbose they were fine with using that as a compromise.

Pat covers a lot of the other types of code that I would classify as being linked to personal preference and I think that it would be beneficial to have coding standards/agreement in the team for the patterns we favour/names we’re going to use in this area.

It’s way easier to just go and write code in your own personal style and I do this way too frequently but it’s much better for the team if we follow a common style.

Improving the code but perhaps not significantly

I’ve written previously about the way that we’ve been making use of a ‘GetBuilderFor‘ class to hang our test builders off and while this worked quite well there is a better way to do this which requires less code being written overall.

We can make use of C#’s object initializer syntax to setup fields with values instead of having to create methods to do that and we don’t need to write the code to make the builder hang off ‘GetBuilderFor’.

As long as we put all the builders in the same namespace we can just type ‘new BuilderNameSpace.’ and then the IDE will show us the choices of builder that we have.

The problem we experienced was that that we already had 30-40 builders written using the ‘GetBuilderFor’ approach and there wasn’t a significant advantage to be gained by going and rewriting all that code to use the new approach.

In the interests of consistency we therefore decided to keep using the ‘GetBuilderFor’ approach even though the next time I come across this problem I’d definitely favour the other approach.

Reg Braithwaite has a really interesting post in which he talks about ‘the narcissism of small code differences‘ and how different authors ‘improve’ the code by adding their own personal touch.

I think he describes the type of code changes that I would classify in this and the previous category although it does make me wonder where the line between refactoring and adding your own personal touch to a piece of code lies.

Improving the code significantly

These are the code changes that may create inconsistency but help us to drive a design improvement which will have a big impact on the way we work.

An example of this that we had on my last project was moving our ‘domain’ model from being defined in WCF messages to being defined independently of other layers in the system.

We wanted to make this change to make the dependency between our code base and the service layer more loosely coupled and we started making that change around about February.

It took maybe a month to move the most awkward parts across since we did it bit by bit in an incremental way rather than stopping everything and making that change.

Along the way the code in these areas was in an inconsistent state and anyone looking at the code base who didn’t know the reason why it was like that would think it was truly insane!

Even now there are still bits of code which are more reminiscent of the old approach than the new and these tend to get changed when people are working in that area.

I think this type of inconsistency is somewhat inevitable if we’re incrementally making improvements to our code base or following the boy scout rule as Uncle Bob puts it.

In Summary

These are my current observations on coding inconsistency but I’m sure there are other factors which can affect this as well.

I think we should look to reduce the inconsistencies in the first two categories as much as possible and ensure that everyone is driving towards the same outcome with respect to the last category.

I spent a bit of time reading some of the Unity code base recently and I decided to try out a variation of Michael Feathers ‘Effect Sketching’ which my colleague Dave Cameron showed me.

‘Effect Sketching’ is a technique Feathers describes in ‘Working Effectively With Legacy Code‘ and the idea is that we sketch a diagram showing the interactions between the fields and methods in a specific class while browsing through the code.

Dave suggested doing a similar thing but with methods and classes instead while stepping through the code with the debugger.

I set up this code to step my way through:

var container = new UnityContainer();
 
container.AddNewExtension<Interception>();
container.RegisterType(typeof (IIDProvider), typeof (HttpContextIDProvider));
container.Configure<Interception>().SetDefaultInterceptorFor(typeof (GetPaymentBreakdownsService), new VirtualMethodInterceptor());
 
object resolve = container.Resolve(typeof (GetPaymentBreakdownsService));

These are some of my observations from this exercise:

• I found it was much easier for me to remember where I was in the call stack then normal.

  I often get quite engrossed in the individual method calls on objects that I forget where the code actually started before it ended up in the current location. Whenever this happened I was able to refer to my sketch to remind myself of where the code had started from.

• Despite having the drawing I still got a bit lost when the PolicyInjectionBehaviourDescriptor made a call back to the container’s Resolve method which meant that the code went through the same path that I’d just followed:

  32 33 34 35 36 37 38

  public IInterceptionBehavior GetInterceptionBehavior( IInterceptor interceptor, Type interceptedType, Type implementationType, IUnityContainer container) { InjectionPolicy[] policies = container.Resolve<InjectionPolicy[]>();

  I was getting really confused watching various different injection policies being resolved instead of the type I was trying to resolve!

• I’ve previously tried drawing diagrams which just contained the classes in a code base but I’ve found that including the methods that connect them makes it easier for me to understand what’s going on.

  I’ve been drawing these diagrams out by hand but I thought I’d translate some of it into dot notation so that I could create a version using graphviz to show on here.

  

  The nodes in orange represent classes and the dotted line represents where an event was fired.

• I realised that I still need to spend more time reading code so that I know when to dive into an object and when the details are probably not important. At the moment I’m too prone to getting distracted by wanting to see how a specific method works instead of looking at those details later on when I actually need to know.
• I felt as I was reading the code that in a lot of places the option type from functional programming would have come in quite useful. There is often code similar to this bit from LifeTimeStrategy:

  object existing = lifetimePolicy.GetValue();
  if (existing != null)
  {
      context.Existing = existing;
      context.BuildComplete = true;
  }

  Instead of existing returning a null it could return ‘None’ to indicate it hasn’t been resolved yet.

• I’ve read about the ‘yield‘ construct before but I’ve never seen a need to use it yet in any code I’ve written so it was interesting to see it being used quite frequently inside PolicySet:

  public IEnumerable<InjectionPolicy> GetPoliciesFor(MethodImplementationInfo member)
  {
      foreach (InjectionPolicy policy in this)
      {
          if (policy.Matches(member))
          {
              yield return policy;
          }
      }
  }

  From my understanding of this construct it seems like it acts in a similar way to a stream i.e. it’s only evaluated when it’s actually needed.

• In the ‘Fundamentals of Object Oriented Design in Uml‘ Meilir Page Jones suggests that we might want to avoid replicated behaviour in our public APIs since it leads to greater complexity.

  This would therefore seem to suggest that having overloads of methods on an object is something to be avoided. Interestingly in this code base the overloads for ‘UnityContainer’ are actually defined as extension methods which then delegate back to ‘UnityContainer’ and pass in default values for unspecified parameters.

  This seems like quite a neat way of getting around the problem since we keep the API clean while also providing users of the code an easy way to do so.

On the subject of reading code I recently came across an interesting post by designbygravity which describes some approaches for reading code more effectively.

In particular I really liked the section about not hating the code :

You can get sucked into hating the code, merely because it is not yours. Software people tend to be equipped with ample egos, and other people’s code can offend. But realize, their working code is better than your imagined code, because their working code exists right now. So put your ego aside and learn the code in front of you.

It’s so easy to drift into this mindset but it’s rarely helpful so if we can stop ourselves it’s almost certainly beneficial.

In our latest technical book club we discussed chapter 8 – ‘How do I add a feature?’ – of Michael Feather’s ‘Working Effectively With Legacy Code‘.

This chapter covers Test Driven Development and a technique I hadn’t come across before called Programming By Difference.

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

• In the section on TDD Feathers mentions the copy/paste/refactor pattern which I wrote about a few days ago. Cam pointed out that this technique allows us to get to the green bar more quickly after which we can decide how to clean up the code. It seems like perhaps this could be added to Kent Beck’s ‘Green Bar Patterns’ that he describes in ‘Test Driven Development by Example‘.

  I’ve noticed a few times recently that delaying our desire to remove duplication until we can properly see where the duplication lies might be a better strategy than prematurely removing duplication and creating a meaningless abstraction which can be difficult to remove.

• From the examples in the book, programming by difference is used to describe an approach where we create a sub class and override some aspect of the super class. It allows us to quickly add new features although it does increase the complexity of the system since we have to add a new class to achieve this.

  The next step after this is to refactor the code into a state where the new class’ behaviour is moved elsewhere or the new class is shaped into something that makes more sense.

  Feathers also points out that while programming by difference may initially lead to our code being in worse shape it can also allow us to see new abstractions which we can then work towards.

• I like the term ‘normalised hierarchy‘ with respect to the way that we create inheritance hierarchies. Feathers suggests that we should look to create a hierarchy where no class has more than one implementation of a method. i.e. if a class has a concrete implementation of a method then that shouldn’t be overriden.
• Towards the end of the chapter Feathers asks whether it is overkill to create a particular class which is just acting like a properties collection at the time. We discussed this and Cam pointed out that we seem to either end up with ‘complex interactions and simple objects’ or ‘more complicated objects and simple interactions’.

  From my observations it seems that we err on the side of not creating enough objects but as Naresh Jain points out it is possible to go too far the other way and end up creating objects just for the sake of it.

Michael Feathers recently linked to a paper titled ‘Testable Java‘ that he’s currently working on which covers some of the ideas from the book but with examples in Java. Worth a look.