Refactoring of a large C++ function

Question

At work we have a legacy process written in Visual C++ that basically consists of a single 5000 line function. In essence the program is just one big case statement with similar cut-and-pasted code handling a fair amount of the logic for case. Obviously we would like to refactor this code to extract these cases out into separate functions (or objects) and to eliminate any cut-and-pasted code.

My question is - are there any suggestions for going about a refactoring effort of this size? Are there any automated tools that could streamline the process?

Answer 1

My first step would be to take some of the larger cases and first push them out into separate functions. That will reduce the visual clutter for a start and make it easier for you to do the next phase.

Secondly, identify the commonality of the different cases and create generalized functions to call in their stead. Up to a point. If you go too far, you'll have a generalized function that's every bit as bad as your current switch statement :-)

I've never seen a tool that can do even half the job of the spongy thing inside your skull. I'd suggest just using that.

Answer 2

I know Eclipse emphasizes refactoring as a feature. There's a listing of all the useful features on IBM's website, but particularly the "new method from selection" tool seems applicable in your case.

Answer 3

This is kind of a broad question. There are some automated tools but the reality is you're just going to have to study the code and make some decisions. Is there redundancy in the code? If so, think about putting the redundant stuff in their own functions.

Answer 4

Several suggestions:

You can find copy-and-pasted code with a tool like Duplo. A script (in the scripting language of your choice) or an editor with multi-line search-and-replace can help you replace that code with function calls. (This can also be done by hand, but using a script or search-and-replace helps eliminate the possibility of error.)

Refactoring tools can automatically do operations like extracting a portion of a function into a new function. Eclipse CDT, for example, can do this, and it's free, and it can edit code that's maintained in other IDEs. This doesn't always work, but when it does work, it's truly amazing to watch as the IDE splits apart a thousands-line method, extracting just what you want, and correctly identifying every variable that needs to be passed in as a new parameter to your new method... Other refactoring tools are available, such as Refactor! Pro (free version available), but I've not used them.

More generally, Michael Feather's book Working Effectively with Legacy Code is the standard work on doing this kind of thing. Basically, you'll want to set up characterization tests - similar to unit tests, but the goal is to cover (characterize) as much of the current function's behavior as possible, rather than testing for correctness the smallest units possible - then apply refactorings one at a time. (Feathers includes a catalog of refactorings and other techniques that are particularly useful for legacy code.)

Answer 5

The very first step is to develop a good automated regression test if you do not already have one. Then as you pull out each case to a function you can quickly check that you have not broken anything.

Answer 6

1. Don't try to do it all at once.
2. identify a likely refactoring target. Be as narrow as possible.
3. write tests to verify correct functionality of that piece of code.
4. Once all your tests pass, or fail because the original function actually has that bug, refactor that bit.
5. Make sure all your tests still pass.
6. GOTO 2.

Answer 7

Try Visual AssistX at www.wholetomato.com. It integrates directly in with any version of visual studio from VS6 and up. It includes many great development features, but what you are looking for is the refactoring feature. You can see that feature here. It does cost, but I consider it my 'secret weapon' when developing with Visual Studio.

Answer 8

Following on with what Steve Fallows wrote, after you have a unit test, to make certain that the functionality does change create a new function, and have the same unit tests as the original work with it.

At the moment they will all fail.

Then, start to pull out each case statement and put it into it's own function, and call it from the new function, so you will end up with a function with just a switch, and each case just calls a function.

Once all the functionality is moved then you can look at whether you need to refactor any of the other functions, but, start with a unit test to ensure functionality isn't lost.

Each of the new functions should also have their own unit tests, btw.