The illustrious Chris Eppstein recently tweeted:
If some code should be refactored, stop what you are doing and refactor it.
I was about to respond, but realized I had more to say than would fit in a tweet. (Waiting for someone to fix that problem!) (Then I got distracted and didn't finish this article for a few days, oops.)
Now, Chris is really smart and probably doesn't mean exactly what he said, but it's easy to misinterpret his advice. I'll agree with him that you should be eager to refactor code when you discover the need. Don't let that technical debt accrue interest longer than necessary! However, you shouldn't be reckless about it.
Please keep this in mind:
Don't do a refactoring in the middle of making another change.
If you are working on a story and in the middle of making a code change when you discover the need to refactor something, make a note of it (I usually create a chore in Pivotal Tracker) and forget about it until you're done with the change in progress. After you complete the change, come back and do the refactoring. Make sure all tests are green before starting to refactor, and are green when done. The refactoring change should be a separate commit in git (separate checkin in SVN, etc).
OK, I'm pragmatic and realize that that approach doesn't work all the time, but it's a good ideal to shoot for. I considered discussing cases where it would be OK to refactor something in the middle of another change, but on second thought I think I'll leave that be for now. You'll learn that for yourself better than by following someone else's advice on the subject.
This is a good warning. However, the trouble comes when you see a potential refactoring that could make the current change easier to implement, which is common. Then you have two choices:
Finish working on the current change using "the bad old way" then convert it when you refactor, or
Stash away your current change, do the refactoring, then reimplement the current change, using the stash as notes.
The decision is usually determined by the amount of work already invested in #1.
The gist of my tweet was to strike while the iron is hot because need is apparent. Akin to TDD (you do the boring part first so it gets done), I think the refactoring should be done before the current code/story is complete. That refactoring is a requirement of that story albeit one that was uncovered during implementation. So when I find myself in this situation, I stash my current work or move to a topic branch if needed, then I refactor, then I go back to my current work.
Like all things, there are exceptions. Big multi-day refactorings might get treated the way you suggest or even added to the product backlog. Or maybe there's a deadline that cannot slip. But there is never a time to refactor like the present.
Anyways, you say what we should do, but not why you think it. Maybe we agree more than we seem to.
I've even pulled all nighters before so that the refactoring gets done and the deadline is met :) Maybe I'm reckless, but I prefer to think that I'm uncompromising.
Mark: Yes, that's the main exception to the rule. I'm glad you didn't include choice 3, do the refactoring in the middle of the other change because really it's just a tiny little refactoring and what could it hurt...
Hey Chris, thanks for stopping by. The main reason why not to refactor in the midst of another change is that refactoring is different than creating features. The point of a refactoring is the improve the design of the code, not its functionality. Doing two fundamentally different things at the same time is asking for trouble. You need good test coverage and green tests before you refactor, and often when building features or fixing bugs you have red tests so you can't refactor reliably. How do you know if you are done with the refactoring if you have all this other stuff going on too? The point is to manage the complexity of your development task.
And, from an agilist perspective, I disagree that a refactoring is a requirement for any feature story. Refactoring stories are chores, not requirements. The product owner doesn't know or care anything about how the code is designed, and wouldn't be able to accept a feature story with those kind of requirements anyway.
Josh, I agree that it's a bad idea to have partial changes in flight while refactoring. That's why I stash my work before starting on the refactoring. I do not think it's worth finishing whatever is in flight before you finish that refactoring. Depending on what refactor is needed, the only think you may keep after you're done is the tests.
Re: agile. I think it depends on who the product owner is to decide how to address this work within the confines of the process. At my company, the product owner is an internal customer who recognizes the value of refactoring and keeping our codebase clean and consistent as a means to maintaining or improving our team velocity. As such, the engineering team can place technical stories on our backlog to carve out time to address bigger changes with no obvious customer impact.
I think we are pretty close to agreeing, Chris. I just prefer to avoid breaking the flow on the work in progress, and find that putting a story in Tracker to remind me to do the refactoring later is usually the best way to avoid adding distractions. Most of the time doing the refactoring can wait, so that's my default behavior.
Like most other programmers, you guys must be way smarter than I am. If I stop in the middle to work on something else I will forget the details of what I was doing in the first place. For me, the "make a note of it and continue" approach is way more successful.