Why simple code is more cost effective

A while ago, I was performing a refactor in our codebase, pulling some functionality into a core domain object and then factoring it into some smaller, more focused objects.

My main drivers for making this change were:

1) Law of demeter

2) Hard to see all the paths of execution through the method

3) Nested if { ... } statements

Thinking about 2), a method which has an execution path which is difficult to understand immediately and intuitively (for normal beings that is) is likely to hide a bug. Infact this particular piece of code was doing just that which is why I was changing it.

So more complex logic leads to buggy code which leads to a higher cost of maintennance. The latter has a direct impact on the bottom line of your project, not to mention potential success of the software in its usability or even core function.

The refactor followed three steps.

1) Move the method into the Target class. This involved extracting a parameter which was really the knowledge of the client of Traget. I was a little concerned about Target knowing too much which leads to 2). I did this first by moving the method and then running all tests for client class, then I duplicated the test methods on the Target class and ran them (required a small refactor which made them easier to read as a side effect)

2) As i was doing this move, i realised that a scenario had not been tested, a consequence of 2) above, i.e. the path of execution was complex and so a test had been missed (if TDD had been followed this would be unlikely)

3) Finally, I wanted to get rid of the if statements. In order to do this, I used Replace method with method object to pull out the logic inside the if blocks.

Summary :

By decomposing the flow of logic into smaller more focused units, it makes the individual parts easier to understand and so it is more obvious if an error in the flow exists. It also helps to write test cases in a more modular way. Rather than 20 test methods in a test case you break into several test cases covering specific functionality.

One question arises, is refactor 3) a refactor too far ?

How do I decide this ?

The best way is to get someone else to look at the code and see if it reads well to them. If i was writing a book, I would get someone to proof read it so that I could confirm that my language was expressed in a clear way. This is the kind of review which Pair programming provides as standard, but even then it might be useful to show it to people outside the pair as you can both become entangled.

Share

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.

Share

What does “Refactor” mean ?

I was reading Kent Becks’ Smalltalk Patterns1 and he mentioned quite early on that it is important to factor the design correctly in terms of small, cohesive classes. It wasn’t until I read this that I had made the connection between “Refactor” and “Factor” and hence the mathematical definition of a factor.

Suddenly I thought I saw what he was talking about. Code can be broken down into factors which is when it becomes more flexible or supple (in the language of domain driven design).

In mathematical terms, a factor is a number which can be combined with another number to produce a given result. So the factors of 10 are {1, 2, 5 and 10}. Those of 12 are {1, 2, 3, 4, 6 and 12}.

Let’s say you represent a piece of code as say 12, perhaps it is one big class doing several things. Ok, you can have one big class, maybe its easy to find everything there because its all in one place. However, you could also break it into two classes. Perhaps one is equivalent to say 3 and the other is 4. So 3 * 4 still equals 12 right?

What happens if you factor the 4 into {2, 2}. Now you have {2, 2, 3, 4} You can now make quite a few different “products” of these factors, not just 12, e.g. 2 * 3 = 6. Further, you can make combinations of these intermediate products aswell, e.g. 2 * 2 * 4 = 16

I thought this was an interesting way to look at factoring (or re-factoring), and possibly a useful analogy to explain it. I had not heard it being talked about in this way before.

1 Kindly lent to me by Pat

Share

Initialisation inline or in Constructor ?

private Map<String, String> nodeValueMap = new HashMap<String, String>();
private Map<String, NodeHandler> nodeHandlers = new HashMap<String, NodeHandler>();			

Or

public MyClass() {
    this.nodeValueMap = new HashMap<String, String>();
    this.nodeHandlers = new HashMap<String, NodeHandler>();			
}

Well, I think I might have a rule of thumb… If you don’t actually have a constructor, then you should use inline instantiation. As soon as you need to do some more complex logic in the constructor (in the case above I might want to initialise the map with some objects) then move all the initialisers into the constructor ?

i.e. don’t have multiple patterns going on.

As an aside, a quick discussion with Alistair (of how big is my potato fame) reminded me of the fact that you can actually do an instance initialisation like:

{
    this.nodeValueMap = new HashMap<String, String>();
    this.nodeHandlers = new HashMap<String, NodeHandler>();			
}

Although what the case for doing so is im not sure, might be nice if you are making a STRUCT kind of object.

Also I didn’t realise that you can declare a class inside a method:

public void doSomething() {
    public class InnerClasss{
        public void sayHello() { System.out.println("hello");}
    }

    InnerClass inst = new InnerClass();
    inst.sayHello();
}

Alistair had used this for testing some reflection code, when he needed some test classes to work with.

Share

Referential transparency

Just learned this term.

from wikkipedia

basically if a function can be evaluated and then replaced by that value in the algorithm, it is referentially transparent

e.g. the function plusOne(int x) { return x + 1; } is referentially transparent as it will always return the same value for each value of x.

e.g. the function today() is not transparent because if you replaced it on one day with its value of “25th July 2008” and then ran the program on a different day the result of the progam would be different.

easy eh?

Share