On clean code and refactoring

Bob Martin (also known as "Uncle Bob") visited us at BEKK today and gave us a short pep talk on "clean code". Using his argument parser written in Ruby as an example, he showed some refactoring tricks and some nice Ruby idioms (passing blocks as arguments, etc.). His main point was that you should never check in bad code and leave the fixing for later, cause "later" never comes. If you make a mess, you should clean it up, and clean it up now.

I've thought along these lines before, and one of the issues I've wondered about is: Should you refactor just after you write the code or should you leave it until the next time you need to change it? This is really just another way of asking "How do you know where your refactoring should end up?"

On one hand, I agree that you should clean up after yourself if you've duplicated something or in some other way made the code worse than it was. But on the other hand, you don't know now what the best structure is since you don't know what changes you want to make next time. You'll have to guess in what form you'll need the code to be the next time you change it. This might lead you to a "local optimum" of code quality, but when you revisit the code you might need to take in a totally different direction, forcing you to abandon the local optimum for a solution that is better in the big picture.

If you leave the code in its current (messy) state and wait until the next time you have to change it, it could be a lot easier to see in which direction you should go. With concrete requirements for how you want to change the code, you can probably see that "hey, this would be a lot easier to change if the code was structured this way...". You would then have to make the necessary refactorings, before making any changes.

Of course, there's always the chance that you'll be in a hurry the next time, and so you won't really have time for refactoring. Your changes will make the design worse, and all of a sudden you have a big ball of mud. Also, working this way feels a bit like always working uphill. For every change you want to make, you have to refactor something first. It's a bit like having to clean up the kitchen, when you really just want to make dinner.

Either way, I guess I prefer cleaning up after I've written some (messy) code to leaving it for later. You can probably make educated guesses in most cases as to where you should take the design. But, just as you should be ready to throw away some messy code, I think you should be prepared to back out some refactorings if they conflict with the way you feel the code should be going.

Anyway, just some food for thought.

Spring dynamic proxy problem

I encountered a strange problem at work today: The following test code failed:

ServiceImpl service = applicationContext.getBean("serviceImpl");

The application context looks like this:

<beans>
<bean id="serviceImpl" class="ServiceImpl"/>
</beans>

ServiceImpl implements the Service interface.

Note that the reference type in the test code is ServiceImpl. I sometimes do this when I need to test drive methods that are not in the implemented interface.

The error was a ClassCastException, which made no sense. Looking at the error, it said that it was unable to cast Proxy-yadayada to ServiceImpl.

It turns out that someone added some AOP stuff to the service classes. Spring then proxies the service implementation in one of two ways: if the class doesn't implement an interface it uses cglib to create a proxy.

But if the class does implement an interface Spring will interject a dynamic proxy which implements that interface between the original interface and implementation, and the above code will fail.

So what's the solution? The only immediate solution I see is to instantiate the service implementation yourself if you need to test drive methods that are not in the interface. Hardly ideal, but easy enough.