views:

267

answers:

8

I recently attempted a large refactoring (yes I know smaller increments and unit testing along the way is preferred) that I just had to walk away from. It's not that I couldn't have done it, but I realized that the time to do it right wasn't justified in the business sense. Normally I am one to tackle refactoring with no fear because of good refactoring tools, unit testing, and source control. I've even had people think I refactor too much (which is probably true).

The code under question mixed Windows forms and server components in the same assemblies, was spread across many assemblies, and was overly generous with generics in what felt like the wrong way (e.g. multiple type parameters, 2-3 levels of generic nesting, and long lists of constraints) -- not bad in itself, but the genericness did nothing to cut down on dependencies and seemed to bloat the code rather than reduce it.

Has anyone else had this experience? What were you attempting to refactor? My particular case was C#, but I've had similar experiences with Java and other languages.

+1  A: 

I try to never walk away from a refactoring.

When I've run into circumstances such as the one you're describing, I will, however, reduce my initial goal in the refactoring process.

There is no need to refactor the entire thing in one shot. Often, that's a mistake in the business sense; it can take you away from productive work for too long. However, there are always small opportunities to refactor, even in a large, messy codebase.

I try to think smaller in those cases - obviously, you were working with the code, or you wouldn't have even considered tackling this as a project. In that case, just refactor the part of the code that's bothering you at the moment. Go back later and do another little part.

It's not as rewarding, but in the long run, you still improve the code base, and you never sink all of your time into fixing something that isn't technically broken to begin with.

Reed Copsey
+5  A: 

If you don't encounter this problem once in awhile, you're not being aggressive enough.

There is nothing wrong with getting part of the way through a refactoring operation and realizing that it's not appropriate given the context in which you are performing a fix.

When I hit this problem I try and scale back my refactor to a more appropriate size if possible. By the time you reach this point you often know enough extra information about the problem to fix a smaller portion. I then store away the rest of the change for a later time when it's more appropriate.

JaredPar
"If you don't encounter this problem once in awhile, you're not being aggressive enough." -> Well said.
Jim G.
Two things: "if you don't encounter this...you're not being aggressive enough" and "try and scale back...to a more appropriate size" stand out to me. Being aggressive yet being able to stand back if necessary seems a good mix of qualities.
Kit
+1  A: 

Sure, many times. Usually it's much the same situation as you described; I have some grand ideas about untangling some convoluted code, but after several aggressive changes I found that it wound up being more complicated than the original.

I was just discussing this situation with my coworker today - if you work through a large refactoring and wind up in a only slightly better but different situation, you are probably better off reverting to the original. The overall structure of the code may be a little bit better, but you have increased the risk of introducing defects by making a lot of changes so you have net loss of quality. Unit testing mitigates this to some extent, of course.

Good thing we have version control!

On the other hand, a series of very small refactorings seems to often lead to opportunities to do more large refactorings later, or at least makes the code easier to maintain in the long run.

Ken Liu
A: 

I am refactoring my own personal project to prepare it to be opened. Like you, it seems like it take longer time to refactoring that the time I spent writing it in the first place. (not exactly that feels like). I do not plan to give up though. As its myown project so no time pressure and I really want to open source it. All in all, I understand what you means.

NawaMan
Yeah right, get votedown for answering a discussion. How can my discussion be wrong.
NawaMan
It was probably just a misunderstanding.
Brad Gilbert
+2  A: 

Refactoring (other than very simple things) is not something I'd ever attempt until or unless you have a good automated testing regime in place. That way, you can run your tests and compare the results to your pre-refactored system.

Having to manually test every aspect is a real pain and removes probably all the benefit of refactoring (and then some). With automated tests, the benefits are not outweighed by the costs.

Of course, even with a proper automated testing regime in place, you should still be refactoring in small bits. I prefer to leave code alone when it's working, up until the point where it needs changes made for other reasons. In other words, don't refactor just for some perceived benefit if there's other work to be done elsewhere (especially if that work will have more impact on the bottom line - you are , after all, probably working for a business and cost/benefit analyses should affect every decision you make).

If you have quiet time (and you can't spend that more usefully elsewhere), then by all means just dive in to the code and make it cleaner (however subjective that term is sometimes flung about). But I wouldn't do it without the automated tests since the chances of introducing bugs is high.

However, you'll rarely find yourself in a situation where your time can't be employed somewhere else in a more productive manner. Getting the code to look clean has no business benefit unless that code is likely to be maintained quite heavily in the future (hence there'll be less maintenance cost) and there's usually so many other things that will impress (or placate) your customers which is, when all is said and done, what the business is aiming for.

paxdiablo
+1 Agreed, we just hit a case in our code base where we share a common set of .NET attributes, including CLSCompliant(true). All our products except one have been developed with this in mind, so easy to adhere, however this one product has dozens of compliance violations which have been ignored, and unfortunately no unit tests, so I'm not game changing anything in the code base, even though it should be a fairly simple refactoring, mostly renaming and 1 type change. The hard part with convincing people to write unit tests is that the major benefits come later rather than sooner.
Si
A: 

Sometimes very large refactors are necessary. I have failed at this before, and it can be very discouraging. The strategy that works best for me is to create a branch that breaks DRY by including "the old way" and "the new way" simultaneously. It can be painful and slow, but I haven't ever "failed" doing it this way.

  1. I create a branch in version control.
  2. I try to decouple "the old way" from the system as much as possible.
  3. I create a function, class, or group of functions and/or classes with the same interface as "the old way", that wraps the original items to be refactored.
  4. In the new items, I make thorough unit tests of the items to be refactored first, and run them using "the old way" as a measure of success.
  5. When the unit tests are complete, I refactor the first part that needs refactoring, making calls to the old system wherever parts of the new system aren't yet implemented.
  6. I then test to be sure that my changes didn't break anything.
  7. I then delete any classes/functions that are no longer in use.
  8. I repeat steps 4-7 over and over, slowly replacing more and more of the old system in small chunks, and constantly unit testing to be sure that everything still works.
  9. When the old classes/functions are all removed, the refactoring is done! I then merge my new code into the main branch (obviously this is a process in itself).
Imagist
+1  A: 

Yes, it happens to me all of the time. Refactoring is an important part of my development style and I think it has considerably helped the quality and speed at which I work. But, I do find that I have to abandon a refactoring effort from time to time. Here is my workflow.

  1. Refactor to prepare code for new feature.
  2. Add feature.
  3. Refactor to cleanup new feature.

So basically my workflow is refactor->feature->refactor. And after each step I commit my changes to the source code repository. I always make sure the unit tests pass after each step. I will add unit tests in each step as necessary, but especially during the step where I am adding a feature. I use branching extensively in steps 1 and 2 so that I do not mess up the trunk and so that I can abandon the refactoring work if I see that is getting too unwieldy. And of course I do not always follow this workflow so it is not like I am being pedantic about it or anything.

Brian Gideon
I think branching is a little extreme in this case, but otherwise I wholeheartedly agree with your approach [Refactor->Feature->Refactor].
Jim G.
A: 

I've had this situation a few where we wanted to make some minor tweaks to a simple JavaScript / C# / ASPX / ASCX / CSS file(s) and it just ballooned / exploded / snowballed and we walked away as it just seemed to be rather unproductive given so many other things that could be done. It was developing into a bit of a spike where as a pair we figured, "Ok, let's not do that now..." which made a lot of sense, so we did that. So far, we haven't come back to trying to do it again, but it may bubble back up on the backlog somewhere and eventually get done in one way or another, such as the whole component being removed or abandoned.

JB King