When to stop refactoring (Stop, Look and Listen before you cross the road)

I have been working with Clive on a fair amount of refactoring the last couple of weeks.

The other day we encountered a classic case of refactoring choice and narrowly averted a potentially lengthy (and confusing to future developers) refactor. This was due to my pairs’ breaking the spell of the refactor path we were following.

Our problem was a circular dependancy. We were introducing a new service to the system to replace a heap of code sprinkled throughout the codebase with a central point of access. We had implemented our shiny new service and were moving around the codebase getting things to use it instead of doing all the work for themselves.

We reached a class and attempted to add a reference to our new module (we are working in C#). Doh! visual studio complains at us “you cant add that reference because you already depend on this module”. Apparently there are ways to circumvent this but I am quite a stickler for dependency management (think it was hammered in to me learning C++), and anyway who wants to mess with visual studio when its angry?

Solution 1

Ok, so lets move our dependancy into a module which is higher up the dependancy tree, problem solved. Well, yes, except that once we moved one, we discover a couple more. Plus we are working on a branch and in a codebase without too much of a safety net (ok lets not get into why we would be doing that), so we dont want to be changing the world.

Our Solution was move the class up into a new module, but leave the old one there, just delegating to the new implementation. Not ideal, but at least it leaves the code in an unambigous state (i.e. there are not 2 classes doing the same thing, infact with the same code.

We had reached a point where we were beginning to feel uncomfotable with this. It was feeling heavyweight and intrusive, and worse, felt like we were creating technical debt. this situation would really need to be tidied up later.

We broke for lunch, in the hope that we may have some inspiration with food. After lunch, I returned to find Clive browsing the codebase:

“Hey Clive, did you get any genius ideas over lunch ?”

“Well, I was just looking through the code, and i was wondering, what was it we were trying to achieve again ?”

It was like smelling salts:

“Of course, that’s great problem solving skills, we’ve been going about this all the wrong way!”

The answer was obvious. Instead of moving half the code all over the place because of this one problem child, we had to change that child to be less greedy for responsibility. It was doing too much.

All we had to do was stop it getting its own resources, and instead, pass them in, injecting through the constructor. This pushed the responsibility for obtaining the resource to the client.

As it happened, the client was also in the common module, but was a couple of static methods. A simple move method refactor moved these methods down into a module which we didnt depend upon and hey presto our dependancy circle was broken.

I think this is a great example of how you can get sucked in to a solution. To begin with we found it hard to see another way out of the problem. Simply stopping, looking at the problem form the beginning again, and listening to the code which was telling us not to cross the road, we were able to walk up the street for a few metres and find a safe zebra crossing.