views:

108

answers:

3

As a part of maintaining large piece of legacy code, we need to change part of the design mainly to make it more testable (unit testing). One of the issues we need to resolve is the existing interface between components. The interface between two components is a class that contains static methods only.

Simplified example:

class ABInterface {

    static methodA();
    static methodB();
    ...
    static methodZ();
};

The interface is used by component A so that different methods can use ABInterface::methodA() in order to prepare some input data and then invoke appropriate functions within component B.

Now we are trying to redesign this interface for various reasons:

  • Extending our unit test coverage - we need to resolve this dependency between the components and stubs/mocks are to be introduced

  • The interface between these components diverged from the original design (ie. a lots of newer functions, used for the inter-component i/f are created outside this interface class).

  • The code is old, changed a lot over the time and needs to be refactored.

The change should not be disruptive for the rest of the system. We try to limit leaving many test-required artifacts in the production code. Performance is very important and should be no (or very minimal) degradation after the redesign. Code is OO in C++.

I am looking for some ideas what approach to take. Any suggestions on how to do this efficiently?

A: 

If methodsA-Z were non-static and virtual you could do this easily, correct? Thus if static methodA-Z were to call non-static, virtual methodA-Z you would be able to override behavior. Knowing this you then need a way to change the instance that contains the non-static versions for testing.

Alternatively, you could take the two classes you want to refactor and make them use a wrapper to this static-only class. Then the two can diverge as much as needed.

That's about all my ideas without looking at the real problem.

Noah Roberts
Yes, if the methods were non-static - it would be easy. Do you suggest adding new set of virtual function (to this existing class) that are called from the static ones and than derive from this class a new class that provide difernet implementaiton and could be instantiated and used when testing?I am not sure I understand the alternative. Maybe I wasn;t clear in my original questions - components are not just 2 classes but rather libraries/modules
ratkok
A: 

Thanks for the answers and comments.

After reviewing "Dependency-Breaking Techniques" chapter from Working Effectively with Legacy Code book, combination of the following two techniques actually seem to be the solution for our problem:

  • Instance Delegator - to wrap/replace static methods from the utility class with new virtual methods.
  • Static Setter - to enable instantiation of a different utility class (either production code or stub code).

Combining these two would enable us to decouple the component under test from the rest of the product code.

Our only concern now is the performance hit (due to use of virtual functions).

ratkok
+1  A: 

The simplest answer is to wrap the old library that has the static interface in a facade, then refactor the code to call the new facade instead of the old library. This new wedge should permit substitution of the library for unit testing purposes. Test it out on a single method first, just to see how to implement it.

What really bothers me is when a question is tainted with "performance concerns". We've all written performance critical code, and nobody ever deliberately suggests writing poorly performant code. I find these "concerns" usually come from the naysayers who decry every change, and really don't have a clue as to why a certain change would or wouldn't perform well. Remember, the only valid proof of performance comes from testing. Run your performance test and establish a baseline. Make your changes. Run your performance tests again in the new code. Demonstrate the actual impact. Only then can you make a decision as to the actual impact of a change. You should never allow quibbles over cycles to dominate your designs until demonstrated otherwise.

It sounds like someone important on your project is an old C programmer* who long ago heard some loudmouth say something like "the only way to make C++ run fast is to use static methods." The problem is that he still believes it. 20 years ago the loudmouth may have been right, but compilers and optimizers have vastly improved over those 2 decades. So give your change a try.

The chances are good that if you're using a modern compiler the optimizer is going to remove the extra dereferences in the object code anyway, meaning you'll have added no run-time impact at all.

And if performance is all-critical yet you don't have performance tests, or if you're not using a modern compiler, or if you don't have all the release-build optimizations tweaked (using Profile Guided Optimization, for example) then you have much bigger engineering problems that you need to take care of long before worrying about the performance of an extra layer of abstraction.

  • Note: I am also an old C programmer who also used to say stupid things like that 20 years ago. The difference is that I have learned that some optimizations are far more critical than others, and that the new compilers are amazingly good at figuring most stuff out on their own. My attempts to "optimize" things prematurely usually ended in expensive-to-maintain code that usually doesn't outperform the stock compiler settings anyway.
John Deters
Thanks for the suggestion. I do agree with you wrt performance inpact, but to some degree though. Performance impacts cannot be generalized, it really depends on some other things as well. But, I agree - modern compilers will do the optimization and remove any noticable degradation for the huge majority of the cases.
ratkok