Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Refactor the following two C++ methods to move out duplicate code

I have the following two methods that (as you can see) are similar in most of its statements except for one (see below for details)

unsigned int CSWX::getLineParameters(const SURFACE & surface, vector<double> & params)
{
    VARIANT varParams;

    surface->getPlaneParams(varParams); // this is the line of code that is different

    SafeDoubleArray sdParams(varParams);

    for( int i = 0 ;  i < sdParams.getSize() ; ++i )
    {
        params.push_back(sdParams[i]);
    }

    if( params.size() > 0 ) return 0;
    return 1;
}

unsigned int CSWX::getPlaneParameters(const CURVE & curve, vector<double> & params)
{
    VARIANT varParams;

    curve->get_LineParams(varParams); // this is the line of code that is different

    SafeDoubleArray sdParams(varParams);

    for( int i = 0 ;  i < sdParams.getSize() ; ++i )
    {
        params.push_back(sdParams[i]);
    }

    if( params.size() > 0 ) return 0;
    return 1;
}

Is there any technique that I can use to move the common lines of code of the two methods out to a separate method, that could be called from the two variations - OR - possibly combine the two methods to a single method?

The following are the restrictions:

  1. The classes SURFACE and CURVE are from 3rd party libraries and hence unmodifiable. (If it helps they are both derived from IDispatch)
  2. There are even more similar classes (e.g. FACE) that could fit into this "template" (not C++ template, just the flow of lines of code)

I know the following could (possibly?) be implemented as solutions but am really hoping there is a better solution:

  1. I could add a 3rd parameter to the 2 methods - e.g. an enum - that identifies the 1st parameter (e.g. enum::input_type_surface, enum::input_type_curve)
  2. I could pass in an IDispatch and try dynamic_cast<> and test which cast is NON_NULL and do an if-else to call the right method (e.g. getPlaneParams() vs. get_LineParams())

The following is not a restriction but would be a requirement because of my teammates resistance:

  1. Not implement a new class that inherits from SURFACE/CURVE etc. (They would much prefer to solve it using the enum solution I stated above)
like image 483
ossandcad Avatar asked Mar 29 '10 17:03

ossandcad


2 Answers

A couple ideas come to mind, but here's what I think would be best:

namespace detail
{
    void getParameters(const SURFACE& surface, VARIANT& varParams)
    {
        surface->getPlaneParams(varParams);
    }

    void getParameters(const CURVE& curve, VARIANT& varParams)
    {
        curve->get_LineParams(varParams);
    }
}

template <typename T>
unsigned int getParameters(const T& curve, vector<double> & params)
{
    VARIANT varParams;
    detail::getParameters(curve, varParams);

    SafeDoubleArray sdParams(varParams);
    for( int i = 0 ;  i < sdParams.getSize() ; ++i )
    {
        params.push_back(sdParams[i]);
    }

    return params.size() != 0;
}

What you do is delegate the task of getting parameters to some other function that is overloaded. Just add functions like that for each different type you have. (Note, I simplified your return statement.)

like image 177
GManNickG Avatar answered Nov 01 '22 23:11

GManNickG


Extract Method. Everything after the lines you have marked as different is identical - so extract it as a method which is called from both of your original methods.

like image 32
Carl Manaster Avatar answered Nov 02 '22 00:11

Carl Manaster