Refactoring: When do you know it's time and when do you do it?

Question

When do you know it's time to refactor/review some piece of code ? And better yet, when do you do it?

Probably like others, I've found myself knowing that something needs a refactor/review but deadlines and management left no time for that. I'd also like to hear how you include code review in the general development process.

Lately I've found myself doing it before working on new features/code. For example, if I have to develop something new or change something in module X of an application, I do a code review on that module. I found it also helps me understand the module better so I can then make the changes more easily.

So when do you know it's time and when do you do it? And most of all how do you include it in the planning of a project?

Answer 1

The standard TDD way is Red-Green-Refactor: make a test that fails, write the code to pass the test, then refactor existing code while still passing tests. Refactoring occurs after tests pass and when you find code that is too complicated or uses bad patterns. Refactoring should be part of your normal, daily development process and not an add-on at the end of a development cycle. It works better, I think, to keep the refactorings small. Doing it as part of your regular activity keeps the poor code from growing too large before the refactoring takes place -- at least ideally.

Answer 2

Refactoring isn't something I set aside time to do separately. I'm constantly doing it as I develop new code, or when I'm maintaining old code. Work it in to your normal routine and always look for things you can improve in your code.

To respond to the specific case you added in your own answer to this question: I think your situation is a perfect example of when it's a good idea to refactor instead of a total rewrite. Make sure you write a good set of test cases for the code in question before you change a line of code. If the code fails some tests it will serve three purposes. First, it will give you specific areas to focus your efforts on. Second it will give you a solid justification (to your boss) for working on the code. Third is the standard unit test safety net that you want to have in place any time you refactor code.

Answer 3

Refactoring is something I do continuously through development, not something I plan for. Whenever the code suggest it could be better structured in some way, I make the appropriate changes.

You can never expect to get the design exactly right. The actual nuances reveal themselves during implementation and by constantly refactoring you always strive to reach a better designed and factored code.

Answer 4

I usually don't do it unless I know I'm going to be duplicating code. So, when writing a new feature, if I find myself saying "hmm I did something like this somewhere else..." I try to see how I can refactor the original to enable as much code reuse as possible.

Answer 5

I refactor as I go and I try to keep it quick and safe - the better that area of the code is tested, the more I can do quickly and safely.

In addition, I mark areas or architectural issues I think need a bigger overhaul, and try to schedule those larger sessions separately - usually, what makes them larger is lack of tests, which means I have to spend a while adding those tests that I need.

Answer 6

To get into a specific problem: There's a project where some bad code was written (even by people who are no longer in the company) during a few months. A full rewrite would be unfeasible and I couldn't explain it to either the client or management.

So I was wondering if refactoring a certain module before doing changes on that module would be acceptable in that situation.

I know it's not the best scenario but the context is a special case (code already broken, can't rewrite it all).

Answer 7

I think the correct answer is: always! While working on new feature if I see a piece of code that I can refactor I just do it. Because I use TDD I don't fear that old functionality will stop working.

Answer 8

I tend to see either 'code smells' like I'm repeating the same code over and over again or I see something that makes me think, "There has to be a better way to do this and I will go find it." It is part of how I write code and think it is somewhat of a good thing to have good code that may take a little longer to complete but that is much more easily scalable, maintainable, or have someone else take it and not have to spend days figuring out what I was doing in the code.

If you're inheriting code, then I tend to think there are 2 schools of thought on what to do with it:

1) Keep your distance. This is where you make the required changes to get the feature in and don't do anymore. If you know the module will be replaced soon or you only work on this once or twice a year, then I can see the logic in not wanting to spend lots of time fixing it.

2) Immerse yourself and fix it now. If what you doing is fairly extensive changes or is a piece of code that you'll be working with regularly, then it may be viewed as part of the maintenance to do some refactoring or documentation or however you'd want to describe where bad code is turned into not so bad code as this will save you time later on.

Answer 9

When your code smells

Answer 10

I will say that I look for code smells as well, but I would like to be more specific. I am using a framework of my design that is growing and evolving with each project. I will tend to heavily refactor and redesign (disciplining myself to keep these separate is something I'm still working on) early in a project and as I get closer to a deadline and closer to solving any issues or code smells I will ease off and focus on my specific implementation. As I do this, I will usually find (or create) a few more things that, while functional, I am not entirely happy with. I will log these and on my next iteration through the project I will address these issues.

When I come back to the code, I will sometimes find that there is a more elegant way to handle the situation and kick myself for not seeing it sooner. Sometimes, I will find there is a better way, but it is not the way I had originally envisioned. Sometimes I find that it is good the way it is and that changing it would be over-designing. Other times I find that in fixing something else, my original issue has disappeared.

Answer 11

1. When I prepare to make a functional change to the code (feature, bug fix, performance, whatever), I first ask myself what it would look like if the code was ideally-structured to make that change.

2. Then I refactor in that direction

3. I now make the functional change.

4. Finally, I refactor again to clean up the ugliness introduced by my change.

I always want to balance my refactoring work against other factors (deadlines, importance of this code, quality of test coverage, etc.).

Answer 12

If you don't need to change that code (it works, no need for extend it), don't do it. Let it be. In other way, if it needs changes in it, write some test a refactor it and include your changes.

Answer 13

I consider myself a novice programmer (I've been doing programming for a living only for 6 months at the moment) and I've noticed that just by looking at the code you should get the feeling if it needs refactoring or not.

As far as I understand some refer to this as "code smell" but I'd say it's more of a unjust feeling that you haven't done your best with code you're eyeing at. You may not be sure what to do or how to improve the code but if you have even the littlest doubt of that the code isn't perfect then it most likely isn't.

Answer 14

You can often make the scope even smaller than a module. Sometimes a single function will be an obvious candidate for refactoring in isolation, even if it's just renaming local variables, removing the need for comments by making the code explain itself, stuff like that.

If you can identify an area that needs to be changed, clean up in that area before and during the change being made. Often I find I need to perform some refactoring to get a enough understanding of the code to make the change at all.

I'd add my voice to everyone else who says to get some sort of testing wrapped around the code, though. Try to at least cover a reasonable set of "normal" cases and get some automation in place (there are frameworks available for just about every language) so that it's easy and fast to run your tests after every small change. I wish I'd thought of/known about testing frameworks when I first started my code cleaning activities...

Answer 15

If I keep visiting the same code over and over again when fixing bugs, I figure it's time to refactor it. If I've joined a new project or become responsible for new code, I also sit down and start refactoring. If I am extending something, then prior to any big changes I refactor out all of the old problems first (before they become too ingrained).

You've finished refactoring when you've reached some targeted level of normalization. If it's just general clean up: 1,2 or 3 is good enough. If you're extending the code, then 4 or 5 are better. If you're really trying to leverage the existing work over a long period, then 6 is the way to go.

Paul.

Answer 16

To answer only part of your question: I'd reiterate a point made by some of the others here already - in general, if you don't have a repeatable set of tests that you can run to make sure that what you changed hasn't broken the code - then you probably shouldn't refactor at all.

You said the code is already broken. You might be tempted to go with initially making as few, small changes to it as possible, in order to get it "working". Trouble is, without a test, how can you say it really is "working"?

Good luck to you!