How do you stop yourself from refactoring working but awful code?

Question

I have this problem. I can't stop myself from refactoring existing code that works but is, in my opinion (and perhaps objectively), badly designed or contains other "code smells". This can have a significant negative effect on my immediate productivity. But ultimately will be a big maintenance boon.

If you also suffer from this "affliction", how do you restrain yourself? Or at least manage the refactoring to avoid having to alter large chunks of existing code in order to make it maintainable for the long term.

Answer 1

Decide based on the "bottom line benefit" instead listening to your "inner voice". If that code will be reused very often, then it might make sense to refactor it. However I usually try to ignore my hate for "code smell" and really focus on the benefit/time-wasted ratio of the refactoring in question..

Answer 2

The answer is you don't. Just because code works doesn't mean it will always work. Bad code is bad code.

We refactor to make the code more readable, easier to maintain, and to ensure reliability.

As long as you keep the logic of the old code, it should still work, plus have the added bonus of being overall better code.

I always try to leave code better than I found it.

Answer 3

I won't say I 'suffer' from this affliction but... I hear you brother!

I do think we need to leave the code in a better state than when we left it. So your pursuit is noble. Let's call it 'refactoritis'.

I'm assuming that we all agree on the benefits of refactoring and that it sort of depends on the code as to how necessary it is... To the crux of the question then...

One way I restrain myself, is that I try to feel safe in the knowledge that it's ripe for fixing. And I know how to fix it. And rather than completely restrain myself, I just do one step, like 'Extract Method'. And leave the next 'round' of fixing for later (perhaps we should call this 'second helpings' or 'dessert' if you're sure it's the last step). Then stick a big TODO on it, so you can find it again. And clarify what still needs to be done.

Thanks for the interesting question. It makes me wonder whether we need a 'Refactorors Anonymous' group where we can sit around in a circle and share our problems and war stories.

Answer 4

I agree, it's tempting but if you focus on it you might not get any of your real work done!

Two suggestions:

1. Mark the code so that you can go back to it later to clean up (use the TODO comment or something similar)
2. Add a bug to your bug tracking system indicating smelly code. Perhaps you can then get time scheduled to fix a group of them.

Answer 5

I usually don't restrain myself. If I find a bad piece in the code I'm working on, I correct it directly.

I'm working on a software that has been maintained for about 10 years now and will have to work at least ten more years. In such a situation, the longer a code smell stays in the code, the more often my fellow developers and I will stumble over it and waste time trying to figure it out or invent workarounds for it. This costs more money than just doing the work right now.

An exception are large design problems that take days to refactor. If those do not obstruct my current project in a significant way, I add it to our maintenance todo list to work on the refactoring as a planned task in the near future.

Answer 6

I just wanted to add that if the project in question is supported by unit-tests then refactoring is going to have considerably less risk attached, and would have a degree less productivity cost (because you're going to know exactly when you're ging wrong).

There's still a cost/benefit to be done, but in principle I'd agree with Longhorn that you shouldn't be stopping yourself.

Answer 7

First, you have to realise and accept that the only reason that the bad code matters is if it needs to be read and/or modified. It only needs to be read and/or modified if you're looking for a bug or adding functionality. So only touch the code if you're bug-fixing or enhancing. So, if you come across some massively nested if-then-else statements, or a badly named variable or method, leave it alone unless you need to actually understand or change that code to finish the task you're actually working on. If you do have to change the code then refactor it enough to make the code comprehensible and the change easier to make, but no more.

Answer 8

If I'm working on a task, I don't let anything get in the way of it - it is too easy to get carried away. Anything that looks like it needs work then it probably does, but maybe not right away - make a note/add it to your bug tracker.

Answer 9

I create an issue in our issue tracker (or you could use a post-it note) about fixing that code, and then forget about the problem.

This allows everyone to know that there is code that is bad that could cause some problems, but also allows me to keep working on my current task without too many distractions.

Answer 10

Whoa, lots of you are kinda OCD if you know what I mean. I think to CYA, if you are only working the code base for the short term, then don't bother refactoring. But if you'll be owning this code in the long term, then go for it! But you must back up your new code with unit tests, as well as add unit tests to the legacy code you'll be affecting if tests don't already exists.

Answer 11

I suffer from refactoritis as well, and I deal with it by directing it at code that is mine that others have maintained or other people's code that I have to maintain. And by deal with it, I mean "check version control every hour on the hour to see if anyone has touched my code and if they did, do an immediate diff."

Answer 12

Are you habitually refactoring code written by others, or code that you wrote six months ago while you were in a different kind of 'zone' ? If your answer is the latter, its very good that you don't paint or make sculptures for a living ... you'd be arrested for breaking into people's homes to 'finish' your works long after they were purchased.

I am a BIG fan of Doxygen , which allows me to stick a simple:

/**
 * Function to rule the world
 * @todo make this actually work
 */

Then, I tell Doxygen to generate to-do lists based on my comments. This helps to ensure functions that smell like feet don't get forgotten.

I look back on stuff that I did even two months ago and say "Ick, what the heck was I thinking ... I should do it this way ... ", which results in never actually getting a consumable out the door.

If its other people's code that you consistently re-factor, try to ask yourself if doing so actually improves the code. If you are able to re-factor a function, its pretty obvious that you understand the function .. so unless you get some (more than trivial) benefit from re-writing it, slap a comment above it and come back later.

After you are used to all of the code and have given some thought to how to make things more coherent .. then dive in. Otherwise, you'll end up re-factoring your previous re-factoring (what a vicious cycle that took years to perfect!).

So, unless its inline, or used all over the place .. try to avoid messing with it unless you really need to. Later, come back and hit your TODO list in one swoop .. when doing so is your only immediate goal.

Answer 13

What worked for me was getting fired from a job for it. :\

That said, my basic problem was twofold:

1. I was refactoring code that no one had asked me to work on.

2. I found it too hard to put tests in place, so I did it without them. Which, naturally, broke some things.

What I learned was:

1. Don't work on code you don't need to work on, and that no one has asked you to work on. It's easy to remember this now, given the consequences in the past.

2. When you need to refactor code you are supposed to work on, have tests in place, but they don't have to be automated.

After reading too much TDD stuff, I tended to think of automated tests as the only kind of test there is. In reality, even a bunch of Debug.Print statements can be a decent test to figure out if functionality is staying consistent.

If you have to refactor, and you can't do automated tests, you must do some kind of test, whether it's printing text, or a UI script, or whatever. Any effective test is better than no test at all.

Answer 14

I have trouble stomaching smelly code, but will usually put off cleanup and refactoring until I have to actually make changes, or if I have some down-time.

Until then, I'll just note ideas for changes in comments.

Answer 15

I used to refactor code whenever I came across something I didn't like, but not anymore. I found from experience that it's just not worth it. There are better things you could spend your time on.

The main reason I don't refactor anymore is the risk of introducing a new bug. The worse the old code is, the higher the chance that you're going to make a mistake. For example, the old code might have an unintended side effect. That could be considered a coding error, and you'd remove it in your rewrite. However, code that calls into your rewritten function may be relying on that side effect.

I work on a lot of really ugly legacy code that's mostly uncommented and for which there are no specs. The code just does what it does, and no one can really say why. They just know it seems to work. As horrible as it is, this code does it's job, so it's best to just leave it alone.

That said, I do believe bad code should be rewritten. I just don't think that programmers should do this kind of thing on their own. It should be treated as a project, with defined scope, timelines, test plans, etc., just like implementation of a new feature.

Answer 16

I do it - on some fairly awful code. The system provides a check; the code changes have to be reviewed before final checkin to the main codeline - and that provides a stop.

But I cry internally whenever that happens. A lot the code I work still doesn't use prototypes for the C function definitions. Most, if not all, have a prototype in a header somewhere, but many of the actual definitions haven't been changed since the late 80s.

Answer 17

I generally don't touch any legacy code UNLESS I am asked to build on it extensively. If the code is so bad that adding one feature cascades into bugs all over the place, you have a real reason to re-engineer it.

In addition, there is a very, very real risk you will introduce bugs that had been fixed in the legacy mess. That will make you look unprofessional.

The only good way, IMO, is to slowly refactor the old code outside of production. Then, when you think the functionality is the same, go through the old resolved bugs and make sure no old bugs show up again.

Roll it out to QA and brag to management about it, preferably with some cool new feature or two. If the new code performs much faster, that's a definite selling point.

Generally you will have two issues: complete lack of time, and management pushing against allocating time to refactor. If none of those are an issue, consider yourself very lucky.

Edit: Joel Spolsky has the best answer, I believe.

Answer 18

I try to refactor on a piecemeal basis. That is, when I need to add new functionality, I refactor it in such a way that makes it easier to add that functionality in first. This makes refactoring a slower process, but it tends to make for an easier time adding changes.

Answer 19

Martin Fowler's take on the issue:

Its essence is applying a series of small behavior-preserving transformations, each of which "too small to be worth doing". However the cumulative effect of each of these transformations is quite significant. By doing them in small steps you reduce the risk of introducing errors. You also avoid having the system broken while you are carrying out the restructuring - which allows you to gradually refactor a system over an extended period of time. - Martin Fowler

So when you are changing huge chunks of code at a time, I wouldn't really call that a true refactoring. More like totally rewriting particular code segments. The changes should be gradual and "almost not worth doing". After time you'll notice their cumulative effect.

I've heard a definition for "legacy code" is code without unit tests. If you are working on code without unit tests than I would suggest writing tests before making significant changes. If you have unit tests in place than you can refactor without fear of breaking that awful code because you have your unit tests to back you up and ensure everything is still functional.

Answer 20

Stop caring.

I mean, cmon, we're all engineers here, and engineers just love to put out THE MOST ABSOLUTEST PERFECT CODE EVER. And we're going to keep trying. And we're not going to be happy about our work, unless we know that everything we've touched is TMAPCE. Besides, we just know that this way is so much better...

So, the only real way to not keep trying to refactor another little bit at a time, is to not care about your work.
(As I side note, this did happen to me once. It was very sad... till I moved to a different job.)

Answer 21

Perfectly valid question.

I use to find myself starting to refactor code automatically as I ran into it. When I'm 5 minutes or so into the operation (checking things out etc) I suddenly get this feeling that what I'm doing is going to take longer than I expect. At this point I ask myself, is it worth the effort going down this rabbit hole? To be honest, it sometimes is, but most of the time it's not, and after a while you realize you want to rewrite the whole system, just while you're at it!

This got me into the habbit of asking myself continuously: Is the code I'm writing now going to help me accomplish this task in a reasonable amount of time? Am I "wasting" the company's time by doing this refactoring while there are outstanding items which are much higher on the priority list?

I've seen programmers that don't even realize they're doing it. They will work for days on end to get the code in a state where they feel they can now start implementing new features or perform bug fixes. They can't distinguish between refactoring and spending time on the assigned issue.

What I do is just comment what I want to do later on, and simply work with the existing system, no matter how ugly it is! This allows me to break less things and actually getting through my list of things to do much quicker. (We don't do TDD... yet)

Once you find the time, come back and refactor.

Answer 22

Before you start refactoring you need to have comprehensive unit tests for it, and nobody likes to write unit tests :)

Answer 23

By reminding myself about cost-benefit ratios and all that.

Answer 24

Refactor as your last task of the day.

• Figure out exactly what you want to refactor and how you would do it.
• Work out all the real work you need to get done today.
• Finish the real work.
• Refactor.

You will be tempted to refactor first before you start the new work "because that will save even more time", but it's actually very easy to refactor new code on the same day you wrote it.

Answer 25

I read the old Uncle Remus story about the Tarbaby. Or I bring out one of those signs from the stores where it says "You Touch It, You Own It".

Answer 26

It's easy really.

Everything we modify, we have to come up with a written test plan that is understood and reviewed by a QA engineer. Not modifying stuff drastically simplifies actually getting code released. Therefore, unless you're touching that piece of code for some other reason anyway, any kind of refactoring is pretty much a no-no.

Answer 27

Oftentimes refactoring can reduce or eliminate the need to do the real work assigned. If you're fixing a bug then refactoring is a good place - assuming there's tests in place. To a lesser extent adding new features can be achieved with a simple refactoring of the code to make it a bit cleaner or more efficient and then you see that the feature was already there and how to get the user to enable it.

The short answer, however, is that you don't refactor code that works unless there's a pressing need. A good checklist is:

• Am I going to be working with this code daily for an extended period AND am I the ONLY person who will be.
• Is the code so bad that it can't be properly understood. Usually I find I'm just suffering from a case of not invented here syndrome (the thing where someone else's perfectly good solution isn't how I would have done it so I want to do it again for no real gain except pride).
• Is the architecture not capable of supporting said new work (feature, bug fix, etc).
• Is the code so incestuous that changing a few small things has a massive ripple effect of follow-on bugs.

If you answer yes to any one of those then it might be time to refactor.

Answer 28

"Refactoring existing code that works" - This is obviously a minority opinion here, but this is a complete waste of time. It is good you are attempting to restrain yourself.

If you think the code is so bad, find a non-trivial bug in it before you refactor it. Then add an automated test for that bug. Only then allow yourself to refactor.

The attitude that after refactoring working code "I've left the code better" is often programmer hubris. In many cases you don't really know you left the code better, you just believe you did. If you didn't even fix a bug or add functionality why take the risk of being wrong?

Answer 29

When you're working on a piece of code, or you have to use a broken interface, refactoring might be the better answer, so try to constrain refactoritis to that.

While I'm young enough (studying for my master) that I'm not normally paid to program (I've been programming for Open Source programs, and sometimes getting money out of that), the few times I've worked on an actual company (for summer jobs, quite time constrained) I found my productivity to be much lower, and I've a definite feeling that it happened because the code, while being readable and so on, was not of the highest quality. I saw a number of bugs I wanted to fix, but gave low priority to all of them, and still the quality of the code I added myself was much lower. Dunno if that applies to anybody else, still it may be interesting to know and to see if somebody had the same experience.

Answer 30

I find that there is an internal struggle between my idealist and my pragmatist. The idealist wishes to refactor, and refactor often, even at the expense of actual progress, while my pragmatist knows that often the code is is "good enough" and the focus should be on moving forward.

Here the problem is the defining the term "good enough", and the definition varies based on your type of project and constraints (time and money for example) placed on you.

I place a lot of weight on fixing things that are going to make supporting an upgrade path for the product difficult. For example, if a database schema is less than ideal, it should be fixed before release to avoid complicated schema evolution tools that have to operate on you clients production data.

Above all, refactoring should not be a dirty word in your work place. Refactoring should be a part of your process and you should not feel like refactoring efforts need to be done on the sly.

Answer 31

I have suffered from this in the past, and I micromanage my 'bad' good habit.

Basically, I do the following:

• I have two separate instances of our source tree at any given time.
• If I have a massive refactor that's not really top priority, I hack away at it in the second branch.
• I'll then keep working away in my first branch, and just maintain the second branch, doing little tests as I think about it to make sure I haven't broken anything.
• I'll then check it in once I'm confident, and I can time it with a slow-down in our production cycle (i.e., don't check in right before milestone!).

In all honesty, that 'major' refactoring never really happens often -- usually if I need to do that, it's already been tasked anyhow. Still, the few times I've done that, it's come in handy. Ideal if you can do a branch for it, and just keep integrating changes.

For the smaller stuff, I'll often keep those changes local in my 'main' branch, and run with them locally for a while. The moment that anything I'm working on touches those files, I then check the whole change in -- our process currently includes a 'buddy check' system before check-ins, so usually it's stuff that'd clear peer review anyhow.

Anyways. Might be more of a brain dump than you cared for, but hopefully it helps.

Answer 32

I prioritize.

When I have some cycles available (ha),

I generally try to re-factor code that:

• would be sensitive for anyone else to work on,
• would actually need to be worked on, meaning it is touched regularly
• that is in poor shape for performance reasons or documentation reasons, in that order.

Where re-factoring adds value, I do it. Trying to line up future new features to overlap with re-factoring makes it easier to do while under the hood.

Reasons not to touch anything we feel like touching?

• Often old code has been hardened for input/output, as clumsy as it may be, it works.
• If it ain't broke, don't make it that way.
• There should always be something new and neat to work on, and code the right way. Old stuff will get re-factored over time as it is extended.

Answer 33

If I'm ever in a situation where some code needs refinement (and I usually only worry about this for my own code) I immediately add comments in the header of the function/file. Even if I have time to work on the refactoring right then, I still take the time to write comments to organize my thoughts first. As I make the improvements, I remove the comments. That way, even if I have to put the refactoring off until later, I have some kind of guidelines to remind of what the hell I was doing.

Answer 34

The following Boy Scout rule applies very good to (bad) code and design:
Leave the campground cleaner than you found it!

Answer 35

The highest-rated answer urging you to go ahead and refactor is good for many cases, but somewhat simplistic too. (I'd probably comment on it, but have no privileges to do so - I hope this works stand-alone too.)

If you work with a large (legacy) system that's been in development for years and years, there are always too many things to refactor at once (unless you have been exceptionally rigorous all those years, which I don't believe :). So, you simply cannot get on all the tangentials you'd like to; that's a fact of life you have to accept. Otherwise you'd always be speding days on end cleaning everything up, when the original change (bugfix or enhancement) could have been done in much less time (tests and some refactoring included!).

So, usually you have to draw a line somewhere; refactor only code that directly concerns the task at hand, and only if it will not take a disproportional amount of time.

As for the bigger overhauls of architecture - which certainly you can't avoid when dealing with aforementioned large codebases. You'll have to select the ones deemed most critical, and task and prioritize them in your process high enough that they will really get done, even when these changes add no external value themselves (i.e. only immediate value for the developers, by making the code more manageable). (Now, if this would not be possible - if decision-makers are not smart enough to see that it's necessary to use time on such improvements, well, then your codebase is simply doomed in the long term. :))

If you are free of any constraints of commercial software development, your mileage may vary. ;)

By the way, good question - I too find myself thinking about where to draw the line quite often.

Answer 36

I stop myself by thinking: "Is this going to actually accomplish anything?" "What are the benefits of taking this action, now, tomorrow, next week, a month or a year down the line?"

If there's little to no benefit-to-time-spent ratio, then I dont' do it.

Answer 37

Use tools to make your refactorings as safe as and as low cost as possible so that refactoring means less time and less risk. Eclipse has wonderful tooling regarding this. Regarding not breaking stuff, automated tests are clearly a prerequisite for refactoring smelly code.

Clearly identify and minimize the boundaries of code which directly affect your current "productive" coding task (implementing a feature or fixing a bug), and refactor only that part. Also consult with your manager about the technical debt the smells impose and the estimated effort the refactorings would take.