Tuesday

How to refactor without reFing it up

Sometimes technical debt is unavoidable. There are lots of good reason why the code you have today is increasing your maintenance costs and slowing feature development. Sometimes you have to accept a hack in order to ship (shipping is a feature). Or if your product has been around a while, it's probably evolved far beyond the original design, and it just needs to be restructured. Or maybe it's not a good reason but it happened anyways, e.g., you trusted a less than stellar developer, forgot to write unit tests, or adopted a framework that sounds great in theory but just doesn't make any sense in practice. In any case, taking an iteration or more (when you can afford it) to deal with technical debt is just par for the course.

The first mistake made by many developers when they start refactoring is to start by cleaning up code. That way is a rabbit hole that will take you through multiple iterations to increasingly madder tea-parties (read: standups) and end with your PM screaming "Off with his head!" because you've made the codebase unworkable for weeks, all in an effort to make things better. It starts so innocently, with a single class. Then you notice that the problem with that class is related to one of its dependencies, so you chase that dependency down and discover that the general architecture is too small (or too large) so you eat the mushroom or drink the drink and soon you're drowning in your own tears. The caterpillar may console you momentarily by giving you a hit off his hookah, but it's just going to get weirder. Better to stay out of the rabbit hole.

The key is limiting the scope and hammering it into everyone involved that "WE DO NOT TOUCH ANYTHING OUT OF SCOPE". I'll say that again, in all caps WE DO NOT TOUCH ANYTHING OUT OF SCOPE. "But, but, the service layer is still a mess, which is why w-" WE DO NOT TOUCH ANYTHING OUT OF SCOPE.

The first step in a successful refactoring is for a senior developer or lead to sit down and figure out the smallest possible refactoring that can be done. Then create specific instructions on how the refactoring should be done. Literally, instructions like "Move line X to class Y and put Z where X was." You don't want to leave it up to the developer to come up with their own way of doing things. Developers get bored easily, and a single developer can try six different ways of doing things before he decides on one he likes.

Once you have detailed instructions and guidelines, cut your developers loose. After the first commit, be prepared to devote at least 10 minutes of your next standup to explaining the instructions again, because it turns out half of them didn't read them. It's not that you have stupid or bad coders on your team, it's that they're smart and think they don't need instructions. It's more fun to get coding and figure it out as you go, right?

Because you made the original refactoring small, once the effort is underway, you need to start planning the next one. Small and iterative works for features and it works for bugs. It will work for refactoring just as well. Don't waterfall refactor.

It goes against every technical design instinct we have, but the better course of action will almost always be to incrementally improves components of a system rather than redesign it from the ground up. You also have to keep the app running. If HEAD is unusable for even a day because of your refactoring, you're trying to change too much at once. Sometimes that means writing some glue code that is ugly and inelegant so that the component you're refactoring can be pretty and elegant. Your inner architecture astronaut will be screaming as he burns up on rentry, but even NASA sometimes needs duct tape.

Here is an example to get you started: Take your JSP/ASP/GSP or PHP templates. Define model classes for them, based on the needs of the UI, not based on whatever model you have in the database or ORM or the DTOs your services return. You should end up with simple inline expressions. e.g., ${model.foo} rather than ${_POST['foo'].replaceAll("\\.","-")} or ${fooList[i].bar.baz.foo}. Move all that ugly logic into your controller. That part is easy. The hard part is resisting the urge to go back your service layer or even your ORM mapper and develop a factory and mapping system that trasforms the output to perfectly match your templates so you can just write Foo.list(params) in your controller and have it be oh-so-elegant. It will be done in 2012. Remember: Worse is better.

1 comment:

Anonymous said...

reminds me of what i just read from Martin Fowler