How do I avoid cascading refactorings?

I’ve got a project. In this project I wished to refactor it to add a feature, and I refactored the project to add the feature.

The problem is that when I was done, it turned out that I needed to make a minor interface change to accommodate it. So I made the change. And then the consuming class can’t be implemented with its current interface in terms of the new one, so it needs a new interface as well. Now it’s three months later, and I’ve had to fix innumerable virtually unrelated problems, and I’m looking at solving issues that were roadmapped for a year from now or simply listed as won’t fix due to difficulty before the thing will compile again.

How can I avoid this kind of cascading refactorings in the future? Is it just a symptom of my previous classes depending too tightly on each other?

Brief edit: In this case, the refactor was the feature, since the refactor increased the extensibility of a particular piece of code and decreased some coupling. This meant that external developers could do more, which was the feature I wanted to deliver. So the original refactor itself should not have been a functional change.

Bigger edit that I promised five days ago:

Before I began this refactor, I had a system where I had an interface, but in the implementation, I simply dynamic_cast through all the possible implementations that I shipped. This obviously meant that you couldn’t just inherit from the interface, for one thing, and secondly, that it would be impossible for anybody without implementation access to implement this interface. So I decided that I wanted to fix this issue and open up the interface for public consumption so that anybody could implement it and that implementing the interface was the entire contract required- obviously an improvement.

When I was finding and killing-with-fire all the places that I had done this, I found one place that proved to be a particular problem. It depended upon implementation details of all the various deriving classes and duplicated functionality that was already implemented but better somewhere else. It could have been implemented in terms of the public interface instead and re-used the existing implementation of that functionality. I discovered that it required a particular piece of context to function correctly. Roughly speaking, the calling previous implementation looked kinda like

for(auto&& a : as) {
     f(a);
}

However, to get this context, I needed to change it into something more like

std::vector<Context> contexts;
for(auto&& a : as)
    contexts.push_back(g(a));
do_thing_now_we_have_contexts();
for(auto&& con : contexts)
    f(con);

This means that for all operations that used to be a part of f, some of them need to be made a part of the new function g that operates without a context, and some of them need to be made of a part of the now-deferred f. But not all methods f call need or want this context- some of them need a distinct context that they obtain through separate means. So for everything that f ends up calling (which is, roughly speaking, pretty much everything), I had to determine what, if any, context they needed, where they should get it from, and how to split them from old f into new f and new g.

And that’s how I ended up where I am now. The only reason that I kept going is because I needed this refactoring for other reasons anyway.

Last time I tried to start a refactoring with unforeseen consequences, and I could not stabilize the build and / or the tests after one day, I gave up and reverted the codebase to the point before the refactoring.

Then, I started to analyze what went wrong and developed a better plan how to do the refactoring in smaller steps. So my advice for avoiding cascading refactorings is just: know when to stop, don’t let things run out of your control!

Sometimes you have to bite the bullet and throw away a full day of work – definitely easier than to throw away three months of work. The day you loose is not completely in vain, at least you have learned how not to approach the problem. And to my experience, there are always possibilities to make smaller steps in refactoring.

Side note: you seem to be in a situation where you have to decide if you are willing to sacrifice full three months of work and start over again with a new (and hopefully more successful) refactoring plan. I can imagine that is not an easy decision, but ask yourself, how high is the risk you need another three months not just to stabilize the build, but also to fix all unforeseen bugs you probably introduced during your rewrite you did the last three months? I wrote “rewrite”, because I guess that is what you really did, not a “refactoring”. It is not unlikely that you can solve your current problem quicker by going back to the last revision where your project compiles and start with a real refactoring (opposed to “rewrite”) again.

Is it just a symptom of my previous classes depending too tightly on each other?

Sure. One change causing a myriad of other changes is pretty much the definition of coupling.

How do I avoid cascading refactors?

In the worst sort of codebases, a single change will continue to cascade, eventually causing you to change (almost) everything. Part of any refactor where there is widespread coupling is to isolate the part you’re working on. You need to refactor not just where your new feature is touching this code, but where everything else touches that code.

Usually that means making some adapters to help the old code work with something that looks and acts like the old code, but uses the new implementation/interface. After all, if all you do is change the interface/implementation but leave the coupling you’re not gaining anything. It’s lipstick on a pig.

It sound like your refactoring was too ambitious. A refactoring should be applied in small steps, each of which can be completed in (say) 30 minutes – or, in a worst-case scenario, at most a day – and leaves the project buildable and all tests still passing.

If you keep each individual change minimal, it really shouldn’t be possible for a refactoring to break your build for a long time. The worst case is probably changing the parameters to a method in a widely used interface, e.g. to add a new parameter. But the consequential changes from this are mechanical: adding (and ignoring) the parameter in each implementation, and adding a default value in each call. Even if there are hundreds of references, it shouldn’t take even a day to perform such a refactoring.

How can I avoid this kind of cascading refactor in the future?

Wishful Thinking Design

The goal is excellent OO design and implementation for the new feature. Avoiding refactoring is also a goal.

Start from scratch and make a design for the new feature that is what you wish you had. Take the time to do it well.

Note however that the key here is “add a feature”. New stuff tends to let us largely ignore the current structure of the code base. Our wishful thinking design is independent. But we then need two more things:

Refactor only enough to make a necessary seam to inject/implement the new feature’s code.
- Resistance to refactoring should not drive the new design.
Write a client facing class with an API that keeps the new feature & existing codez blissfully ignorant of each other.
- It transliterates to get objects, data, and results back and forth. Least knowledge principle be damned. We’re not going to do anything worse than what existing code already does.

Heuristics, Lessons Learned, etc.

Refactoring has been as simple as adding an default parameter to an existing method call; or a single call to a static class method.

Extension methods on existing classes can help keep the new design’s quality with absolute minimal risk.

“Structure” is everything. Structure is the realization of the Single Responsibility Principle; design that facilitates functionality. Code will stay short and simple all the way up the class hierarchy. Time for new design is made up during testing, re-work, and avoiding hacking through the legacy code jungle.

Wishful thinking classes focus on the task at hand. Generally, forget about extending an existing class – you’re just inducing the refactor cascade again & having to deal with the “heavier” class’ overhead.

Purge any remnants of this new functionality from existing code. Here, complete and well encapsulated new feature functionality is more important than avoiding refactoring.

From (the wonderful) book Working Effectively with Legacy Code by Michael Feathers:

When you break dependencies in legacy code, you often have to suspend your sense of aesthetics a bit. Some dependencies break cleanly; others end up looking less than ideal from a design point of view. They are like the incision points in surgery: there might be a scar left in your code after your work, but everything beneath it can get better.

If later you can cover code around the point where you broke the dependencies, you can heal that scar, too.

It sounds like (especially from the discussions in comments) you’ve boxed yourself in with self-imposed rules that mean that this “minor” change is the same amount of work as a complete rewrite of the software.

The solution has to be “don’t do that, then”. This is what happens in real projects. Plenty of old APIs have ugly interfaces or abandoned (always null) parameters as a result, or functions named DoThisThing2() which does the same as DoThisThing() with an entirely different parameter list. Other common tricks include stashing information in globals or tagged pointers in order to smuggle it past a large chunk of framework. (For example, I have a project where half the audio buffers contain only a 4-byte magic value because that was much easier than changing how a library invoked its audio codecs.)

It’s hard to give specific advice without specific code.

Automated tests. You don’t need to be a TDD zealot, nor do you need 100% coverage, but automated tests are what allow you to make changes confidently. In addition, it sounds like you have a design with very high coupling; you should read about the SOLID principles, which are formulated specifically to address this kind of issue in software design.

I’d also recommend these books.

Working Effectively with Legacy Code, Feathers
Refactoring, Fowler
Growing Object-Oriented Software, Guided by Tests, Freeman and Pryce
Clean Code, Martin

Is it just a symptom of my previous classes depending too tightly on each other?

Most probably yes. Although you can get similar effects with a rather nice and clean code base when the requirements change enough

How can I avoid this kind of cascading refactorings in the future?

Apart from stopping to work on legacy code, you can’t I’m afraid. But what you can is using a method that avoids the effect of not having a working code base for days, weeks or even months.

That method is named “Mikado Method” and it works like this:

write down the objective you want to achieve on a piece of paper
make the simplest change that takes you into that direction.
check if it works using the compiler and your test suite. If it does continue with step 7. otherwise continue with step 4.
on your paper note the things that need to change to make your current change work. Draw arrows, from your current task, to the new ones.
Revert your changes This is the important step. It is counter intuitive and physically hurts in the beginning, but since you just tried a simple thing, it isn’t actually that bad.
pick one of the tasks, that has no outgoing errors (no known dependencies) and return to 2.
commit the change, cross out the task on the paper, pick a task that has no outgoing errors (no known dependencies) and return to 2.

This way you will have a working code base in short intervals. Where you also can merge changes from the rest of the team. And you have a visual representation of what you know you still have to do, this helps to decide if you want to continue with the endevour or if you should stop it.

…I refactored the project to add the feature.

As said @Jules, Refactoring and adding features are two very different things.

Refactoring is about changing the program’s structure without altering its behavior.
Adding a feature, on the other hand, is augmenting its behavior.

…but indeed, sometimes you need to change the inner workings to add your stuff, but I’d rather call it modifying rather than refactoring.

I needed to make a minor interface change to accommodate it

That’s where things get messy. Interfaces are meant as bounderies to isolate the implementation from how it’s used. As soon as you touch interfaces, everything on either side (implementing it or using it) will have to be changed as well. This can spread far as you experienced it.

then the consuming class can’t be implemented with its current interface in terms of the new one, so it needs a new interface as well.

That one interface requires a change sounds fine …that it spreads to another implies changes spread even further. It sounds like some form of input/data requires to flow down the chain. Is that the case?

Your talk is very abstract, so it’s difficult to figure out. An example would be very helpful. Usually, interfaces should be pretty stable and independent from each other, making it possible to modify part of the system without harming the rest …thanks to the interfaces.

…actually, the best way to avoid cascading code modifications are precisely good interfaces. 😉

Refactoring is a structured discipline, distinct from cleaning up code as you see fit. You need to have unit tests written before starting, and each step should consist of a specific transformation that you know should make no changes to functionality. The unit tests should pass after every change.

Of course, during the refactoring process, you will naturally discover changes that should be applied that may cause breakage. In that case, try your best to implement a compatibility shim for the old interface that uses the new framework. In theory, the system should still work as before, and the unit tests should pass. You can mark the compatibility shim as a deprecated interface, and clean that up at a more suitable time.

I think you usually cannot unless you are willing to keep things as they are. However, when situations like yours, I think the better thing is to inform the team and let them know why there should be some refactoring done in order to continue more healthy development. I wouldn’t just go and fix things by myself. I would talk about it during Scrum meetings (assuming you guys have them), and approach it systematically with other developers.

Filed under: softwareengineering - @ 20:06

Thẻ: coupling, design, programming-practices, refactoring

Thiết kế website giá rẻ

Danh mục

How do I avoid cascading refactorings?

Wishful Thinking Design

Heuristics, Lessons Learned, etc.