重构以下两个 C++ 方法以移出重复代码答案

【问题标题】：Refactor the following two C++ methods to move out duplicate code重构以下两个 C++ 方法以移出重复代码
【发布时间】：2010-03-29 17:08:44
【问题描述】：

我有以下两种方法（如您所见），除了其中一种（详见下文）外，它们的大部分语句都相似（如您所见）

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;
}

我是否可以使用任何技术将这两种方法的公共代码行移到一个单独的方法中，可以从两种变体中调用 - 或者 - 可能将这两种方法组合成一个方法？

以下是限制：

SURFACE 和 CURVE 类来自第 3 方库，因此不可修改。（如果有帮助，它们都来自 IDispatch）
还有更多类似的类（例如 FACE）可以适合这个“模板”（不是 C++ 模板，只是代码行流）

我知道以下可以（可能？）作为解决方案实施，但我真的希望有更好的解决方案：

我可以为这 2 个方法添加第三个参数 - 例如一个枚举 - 标识第一个参数（例如 enum::input_type_surface、enum::input_type_curve）
我可以传入 IDispatch 并尝试 dynamic_cast 并测试哪个强制转换为 NON_NULL 并执行 if-else 以调用正确的方法（例如 getPlaneParams() 与 get_LineParams()）

以下不是限制，但由于我的队友抵抗，这是一个要求：

不实现继承自 SURFACE/CURVE 等的新类。（他们更愿意使用我上面提到的枚举解决方案来解决它）

【问题讨论】：

你没有清除 params 向量。你打算用来自许多对象的参数填充它吗？也许有更好的方法来重构你的代码，这取决于你在调用 geXXXXParameters 方法之前做了什么。
为什么在 bool 就足够时返回 unsigned int ？
SafeDoubleArray的类型是什么？我怀疑这可以重构更多，但我们首先需要它。我支持@Matthieu 提出bool 的动议。
@nick-d 好建议。我会考虑“清除”向量。如果“清除”向量保持其初始保留大小，则需要阅读（我的生产代码与此处发布的摘录略有不同）。
@matthieu-m 我公司设定的编码标准要求使用unsigned int。我们遇到了各种需要提供良好错误代码的情况，并决定最好使用多值返回（而不是 2 值返回）来提供此功能。

标签： c++ refactoring

【解决方案1】：

我想到了几个想法，但这是我认为最好的：

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;
}

您所做的是将获取参数的任务委托给其他重载的函数。只需为您拥有的每种不同类型添加类似的功能。（注意，我简化了您的退货声明。）

【讨论】：

不错。我喜欢这个。我倾向于这个。很遗憾我自己没有想到这一点。
非常好，特别是如果与 Carl Manaster 提到的 Extract Method 结合以移出一些代码（在调用 detail::getParameters 之后）。当我的模板方法很轻时，我更喜欢它：使依赖图更轻:)

【解决方案2】：

Extract Method。您标记为不同的行之后的所有内容都是相同的 - 因此将其提取为从您的两个原始方法调用的方法。

【讨论】：

True - 尽可能多的通用代码到一个单独的方法。这行得通。但这是否意味着，如果我在“不同的代码行”之前有更多通用代码，那么“提取”会很困难？
基本上，任何重复的代码块都可以提取到它自己的方法中，无论有多少行导致它，或者跟随它。例如，如果提取的方法需要太多参数，它可能会变得混乱，并且替代重构有时可能更合适。但是提取方法是一个很好的起点。
您必须执行两次提取方法（或更多次，具体取决于您有多少不同的代码）。是的，那时可能会变得很困难。

【解决方案3】：

为什么不直接将VARIANT varParams 作为参数传递给函数而不是CURVE 或SURFACE？

unsigned int CSWX::getParameters(VARIANT varParams, vector<double> & params)
{
    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

    return getParameters( varParams, params );
}

您也可以考虑（如果可能）制作这些方法模板并接收output_iterator 作为参数而不是vector。这样您的代码就不会依赖于所使用的集合类型。

【讨论】：

对不起。我不太明白你在暗示什么。 'surface' 和 'curve' 调用的方法是不同的——这就是这两种方法不同的代码行。而 varParams 是一个局部变量，后面就不需要了。如果没有正确提出，也许我需要重申我的问题？您能否提供一些示例代码来帮助我理解？
为清楚起见添加了代码。此解决方案会使其更短，尤其是在更多方法受到影响的情况下。
可能意味着创建一个 SafeDoubleArrayToVectorOfDouble 方法，他们都可以调用它来完成转换工作。

【解决方案4】：

不要传入 SURFACE 或 CURVE，而是传入对基类的引用以及方法函数指针。然后对 surface->getLine_parameters 或 curve->getPlaneParamters 的调用将替换为 (shape->*getParamters)(varParams);

typedef void Baseclass::*ParamGetter(VARIANT varParams);

unsigned int CSWX::getLineParameters(const Baseclass & geometry, ParamGetter getParams
       vector<double> & params)
{
  VARIANT varParams;

  (geometry->*getParams)(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;
}

【讨论】：

【解决方案5】：

哦！请不要使用错误的声明：

return params.size() != 0;

由于 params 是空的，而不是 (1)，它返回 (-1)（所有位都被提升）。

但是你真的可以像这样合并return语句：

return (params.size() != 0) ? 0 : 1;

从抽象类FIGURE 或SHAPE 或更多类派生类SURFACE 和CURVE 似乎是正确的。所以你可以脱离模板并使用虚拟覆盖：

void getParameters(const FIGURE& surface, VARIANT& varParams)

【讨论】：

【解决方案6】：

宏！通常不是最好的解决方案（它是一个宏），也可能不是这个解决方案中最好的，但它会完成这项工作。

macro_GetDakine_Params(func)
    VARIANT varParams; \
    curve->##func(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)
{
    macro_GetDakine_Params(getPlaneParams)
}
unsigned int CSWX::getLineParameters(const CURVE & curve, vector<double> & params)
{
    macro_GetDakine_Params(getLineParams)
}

【讨论】：