Refactoring is Sloppy

Every once in a while, I ask someone whether they refactor their code.  The reaction I get is hard to describe.  Often within the span of a few seconds, I see the inward smile that says "Of course, I do" followed by the sad expression  that says "Not as much as I should."  Many times, those two micro-reactions are followed by "Oh my God, I don't want to think about the state of my code right now", or "Are you picking on me?  I'm doing the best that I can." 

I think there is a reason why this happens. We haven't been very good about settling the place of refactoring in software development.  As a result, the practice is usually sloppy -- hit or miss.

The advice that most people get about refactoring is to do it as you go.  And, frankly, if everyone did that, the state of many code bases would be much better.   The reality, though, is that developers often see it as a less important activity.  It doesn't seem like it has any immediate gain, so it is easy to short-change it in the face of a heavy workload.  The natural response to this would seem to be scheduling time to refractor, but traditionally many organizations are reluctant to do that because (again) it is hard to see the payoff.  Most of us have a story about a "refactoring iteration" that went wrong -- at the end the organization was unclear what benefit they received from it and/or upset that they accepted a lull in feature delivery.  Refactoring "stories" don't have a good reputation either -- the ROI is unclear and there's always this nagging feeling that they would not be necessary if we all practiced "refactor as you go."

So, refactoring is sloppy.  What can be done?

Well, there is a good path forward, but it involves doing something counter-intuitive.  We can introduce a "hand off" in development.  Here's how it works.  For every story/feature that you accept in planning imagine how you would implement it if the code was in a state where it would accept the feature easily.  Then, create a task for that refactoring work and make sure it is done before the other tasks for the feature, and (here's the trick) it should not be done by the whomever is going to do a feature addition task on the same story.

Here's the rationale.  Breaking out refactoring at the task level highlights it.  It's easier to focus on refactoring as as team when it is seen as separate activity.  Having an hand-off between people the people who are doing the refactoring and the people who are doing the feature addition forces a degree of scrutiny and communication that is hard to have otherwise.

Like anything else in process, this is medicine.  It's not meant to be "the way that people do things for all time" but rather as a way to raise the visibility of refactoring within a team, or to make deliberate progress toward some longer term refactoring goals. 

Comments

[Gil Broza]

Counter-intuitive advice indeed, but useful. Yes, teams should be refactoring as part of on-going work. They should also exercise for 30 minutes per day, do stretches, and drink less coffee :-) But until they do, they need explicit guidance or methods to address the effects of their actions.

Gil Broza
Author, "The Human Side of Agile"

[Keith Nicholas]

I think something like this is seriously broken. Refactoring needs to be inside your OODA loop of development. I don't think you are learning any good lessons by treating refactoring as something separate from what you normally do, it will highlight refactoring, sure, but it highlights it as a bunch of donkey work before you can do real work.

Personally, what I think is broken here is that people are not feeling responsible and motivated to produce good code. So work on that, highlighting refactoring isn't going to make people more responsible and get them motivated to actually do it. Calling people out when there's bad code, and praising good code will make the difference.

I think in some ways you always need a Gordon Ramsey, "WTF?! What's this crap?? that code is awful, we can't ship this! take some goddamn pride in your work!" (though perhaps toned down depending on what interpersonal relationships are like in a team :) )

Work on getting peoples natural state of being is to produce quality in their micro steps. Reforming things with refactoring as things progress. Refactoring is just a tool in the toolbelt that helps you achieve your own personal goal of creating good software.

[ChrisEdwards357]

Thank you! I think this is a great approach. We often treat refactoring as an implicit behavior that just happens as we code. For the disciplined developer, this is true. But they didn't start that way. They focused on learning to refactor...explicitly. Once it became comfortable it turned into something they just did implicitly.

In development, we have a principle called "explicit over implicit ". Making a concept explicit in your code often makes it easier to reason about. This article applies that principle to refactoring.

Make refactoring explicit.
I like it!

-Chris Edwards

[Colin Jones]

I love it. Betcha the code gets cleaner *before* the handoff than it would be otherwise.

[Michael Feathers]

@Chris Yes, but highlight it only as much as long as you need to.

@Keith True. Caring about quality is important. Highlighting is just one means to that end.

[Mordel]

Thank you, I see this as a way to gain control over a code base I'm struggling to engage with and the costs of implementing features is hidden.

I don't know if I'm going to explicitly structure things this way (as I am the junior of the pair) but I will certainly keep this in my thoughts during estimates.

[MarianSchubert]

We have tried pretty much all the things you mentioned in the first part of your article - refactoring stories, refactoring iterations, doing refactoring before/after/during task or story. All of them with mixed results. Our product is old legacy system written in Perl so you can imagine that there are a lot of opportunities to refactor. We also have quite a few constraints. Biggest one is that our team is pretty small compared to our product and each story touches different part of the system so we need to perform only refactorings we really need. Otherwise we wouldn't get anything done.

Anyway - as you state in the article, big part of the problem is to decide _when_ to do these refactorings. Problem with refactorings which happen before feature-delivering-task is that most of the time we don't really know what's really essential to refactor in order to implement desired functionality easily in later task(s). Each time we do this during planning I feel like fake oracle trying to predict future. :) I have been trying to find a solution to this problem for quite some time. Then I found this thing called "Mikado method" http://mikadomethod.org and for the last few months I have been using it pretty rigorously for most of my work.

How does it work for me? During planning we only create tasks which deliver (parts of) desired functionality. When I take task like that (let's call it feature task) I don't start coding like usual e.g. with clean/working code in mind. I just use naive approach and _try_ to implement given feature and than I throw all my changes away. During this phase I'm taking notes about problems which make finishing feature hard or impossible. Because I'm writing code that I'm going to throw away I can use some pretty nasty tricks e.g. "do I need this function to return data in some other format? no problem - I just modify it or even return fake static data.". At this point I don't really care that it will break every other place which uses this function. I just take note that this modification has to be done before actual work starts on feature task. Then I throw code which I just wrote away. I review my notes with problems and if there are just few simple ones I start to implement them and push commits with these refactorings. When I'm done I try to implement feature again and if everything goes OK than it's just one (usually pretty simple) commit. And it feels like code has been designed to accept features like this.

When I find bigger problems I write each of them on task card and put them on board so that any of my teammates could help me with them. It's common that these tasks could be done in parallel as most of the time they touch different parts of the codebase. For these bigger problems I apply "Mikado method" again as it's possible that they would also require some pre-refactorings.

When I find really big problems it's easier to talk about them with the team and decide whether to continue or limit scope of the feature so that it could be implemented in reasonable time.

So far I'm really pleased with this method as it provides me a lot of benefits. Few of them are:

1. no more guessing what to refactor
2. goal (feature) oriented
3. commits are simple (as they don't contain multiple unrelated changes)
4. at no point I'm pushing bad code into codebase - just improvements and final implementation of the feature
5. it's easier to zoom out to see where I'm and what needs to get done

PS: Your book "Working Effectively with Legacy Code" helped us quite a few times so thanks for it :)

[Kevinrutherford]

I really like this idea as a short-term measure. Indeed I was just about to embark on such a refactoring task when I picked up this article.

The downside I find, though, is that sometimes this refactoring takes the wrong path. Or gilds the lily and doesn't stop soon enough. I'm guilty of doing both of those on a regular basis, so I need a team that nags me to do the minimum necessary to get the following story on the road.

[Jbrains]

I feel uncomfortable calling this a "hand-off", but I understand what you mean, and it matches the way I like to work.

It matters most to me to be able to ship a working and sufficient version of a feature before all the refactoring has finished. This has more to do with having the "attitude" of Continuous Integration (see James Shore's article) than anything to do with refactoring as such. This creates a delicate balance between a willingness to refactor enough (of the "Clean the Kitchen as you go" variety) and a willingness NOT to let refactoring act as a barrier to shipping a working, sufficient feature.

Unfortunately, this will create more endless arguments about "done" and "done done" and all that nonsense -- I hope, only for a while. :)

[Bill Sorensen]

Typo: refractor s/b refactor. Thanks!

[Davyd McColl]

On the topic of refactoring though: part of the problem may simply be the discomfort that comes with not knowing (really) what a valid refactor is. I've seen some terrible ones like:
* refactoring out a method with a name which doesn't actually describe what it does
* refactoring one line of code into a method which is used exactly once, introducing the performance penalty of another frame on the call stack without any actual benefit.

A good refactor can make code more elegant and perhaps even more efficient. A bad one, well, it makes the next maintainer of your code an unhappy panda... who has to undo your crappy refactor.

[Tom Mullen]

I would echo Marian's experience, specifically

"Problem with refactorings which happen before feature-delivering-task is that most of the time we don't really know what's really essential to refactor in order to implement desired functionality easily in later task(s)."

Often the code is correctly structured/designed before we add a new feature, but if the new feature introduces dependencies that weren't expected before then the best design after adding the new feature may be slightly different to the original.

This is not to say the original code was in need of a refactor, only that including the new feature will change our view of the abstractions at play and their relative importance.

If we are completely familiar with the dependencies that the new feature adds then we can refactor up front, but, if not, we sometimes have to code and test in order to discover and learn them (Marian's throw away code). This is a similar discussion to whether tests can be written up front (TDD).

[Anonymous Coward]

Actually, I've noticed that most efficient programmers, and most teams which do maintain a nice codebase do refactor as they go.

I've also noticed that development standards enforcing small and well defined changes to be committed at a time to source control are actually making it difficult to do continuous refactoring - whenever you pack some refactored code into the same change that also changes or adds a feature or fixes a bug, you actually create a change which is against the development standard. If some reviewer tells you to do two commits instead of one, your motivation to refactor as you go quickly vanishes. And that's how bit rot infects your codebase.

[Rachel Davies]

I've been thinking about programmer tendencies to defer refactoring (see recent blog post). I like your approach of breaking out the preparatory refactoring task as first step in implementing a story, moving the refactor to the front of the XP Red-Green-Refactor cycle. I'm less sure whether it has to be done by a separate developer, where teams are pair programming and switching between pairs ownership is less of an issue.

[Curtis Cooley]

Not only is this good advice to bring refactoring to the forefront, but it's damn good advice in the long run as well.

In emergent or evolutionary design you do the simplest thing that could possibly work. You do not design or add hooks for that feature you just know is coming down the pipe. You end up with a simple code base that may or may not "accept" the next changes.

Taking time to examine the code and architecture to see how easily it will accept the required changes should be an activity performed before each story is implemented. Sometimes some "prefactoring" will be required and sometimes it won't.

I believe Ward referred to this as "making room" for the new code.