将仅在 1 行代码中不同的两个函数分组答案

【问题标题】：Group two functions that differ in only 1 line of code将仅在 1 行代码中不同的两个函数分组
【发布时间】：2016-08-07 08:21:25
【问题描述】：

我有两个这样的性能关键函数：

insertExpensive(Holder* holder, Element* element, int index){
    //............ do some complex thing 1 
    holder->ensureRange(index);//a little expensive
    //............ do some complex thing 2
}
insertCheap(Holder* holder, Element* element, int index){
    //............ do some complex thing 1
    //............ do some complex thing 2
}

如何将 2 个函数组合在一起以提高可维护性？

我的糟糕解决方案：

解决方案 1。

insertExpensive(Holder* holder, Element* element, int index){
    do1();
    holder->ensureRange(index);//a little expensive
    do2();
}
insertCheap(Holder* holder, Element* element, int index){
    do1();
    do2();
}

会很丑。如果do2 想要来自do1 的一些局部变量也是不切实际的。

解决方案 2。

insert(Holder* holder, Element* element, int index, bool check){
    //............ do some complex thing 1 
    if(check)holder->ensureRange(index);//a little expensive
    //............ do some complex thing 2
}

每次调用都要进行条件检查。

解决方案 3.（草案）

template<bool check> insert(Holder* holder, Element* element, int index){
    //............ do some complex thing 1       (Edit2 from do1());
    bar<check>();
    //............ do some complex thing 2       (Edit2 from do2());
}
template <>
inline void base_template<true>::bar() {  holder->ensureRange(index); }
template <>
inline void base_template<false>::bar() {  }

矫枉过正和不必要的复杂性？

编辑 1： 方法好坏的标准优先级排序如下：-
1. 最佳性能
2. 代码重复少
3. 减少总代码行
4. 更易于专家和初学者阅读

编辑 2： 编辑第三个解决方案。感谢 mvidelgauz 和 Wolf。

【问题讨论】：

如何定义好的解决方案？
你可以创建两个包含 Expensive 和 Cheap 对象的类，当你需要添加一个便宜的对象时，你调用一个便宜的对象的 Insert 方法，当你想插入一个昂贵的对象时，你在 Expensive 对象上调用相同的方法。
谢谢。我忘了在这方面考虑，但是这些方法是如何实现的？恕我直言，在这两种方法中都会出现相同的症状：代码重复，现在也更难跟踪，因为它们出现在 2 个不同的类中。
我发现 API 很重要，这就是我拒绝添加额外参数的原因，但代码重复当然是不好的。看看我的答案，寻找可能的解决方案。
这个更适合Programmers SE site

标签： c++ performance function code-duplication maintainability

【解决方案1】：

您的解决方案 2 实际上还没有那么糟糕。如果此代码在标头内，则隐式将其视为内联代码。（我已经明确地写过了）如果你用 true 或 false 调用它，编译器可以删除 if 语句，尽管它取决于一系列因素来知道它是否会这样做。（内联后的整体大小，常量的可见性，调整......）

inline void insert(Holder* holder,Element* element,int index, bool check){
    do1();
    if (check)
        holder->ensureRange(index);//a little expensive
    do2();
}

解决方案 3 实际上是您想要实现的，因为模板需要为每个不同的调用进行新的函数实例化，因此它会删除死代码。但是，它的编写方式与您编写解决方案 2 的方式非常相似。

template <bool check>
inline void insert(Holder* holder,Element* element,int index){
    do1();
    if (check)
        holder->ensureRange(index);//a little expensive
    do2();
}

如果您有 C++17，则不再需要依赖编译器来删除死代码，因为您可以强制它通过 constexpr-if 跳过某些代码。这种构造将保证 if 语句中的代码被删除，因为它甚至不必编译。

template <bool check>
inline void insert(Holder* holder,Element* element,int index){
    do1();
    if constexpr (check)
        holder->ensureRange(index);//a little expensive
    do2();
}

【讨论】：

很好的答案，虽然我认为你混淆了使函数隐式内联的原因：在类定义中确实如此，但仅仅在标题中却没有（你会得到多个定义链接时错误）。
仅供参考：stackoverflow.com/questions/9192077/… 默认情况下，如果函数在标头中实现，则它被视为内联。（或与 inline 关键字一样多）对于模板化函数，情况并非如此，因为它们总是必须在标题中。尽管对于此处的模板示例，最好从其中删除内联。
这个问题是关于成员函数的（如果在类体内定义，则隐式为 inline）。尝试在标头中定义一个独立的函数，例如 void foo() {}，将其包含在两个 cpp 文件中并将生成的对象链接在一起：您可能会收到“多重定义”错误（“可能”，因为标准没有需要诊断 IIRC）：该功能尚未隐式生成 inline。
同意，那是我的错。我最近几乎没有写免费函数

【解决方案2】：

insert(Holder* holder,Element* element,int index, bool ensureRange){
    //............ do some complex thing 1 
    if (ensureRange){
        holder->ensureRange(index);//a little expensive
    }
    //............ do some complex thing 2
}

如果您可以在编译时做出决定并想使用模板：

template<bool check> insert(Holder* holder,Element* element,int index){
    //............ do some complex thing 1;
    if(check)holder->ensureRange(index);
    //............ do some complex thing 2
}


insert<true>(...); //expensive operation
insert<false>(...); //cheap operation

【讨论】：

谢谢，但你只是复制我的第二个解决方案。 :(
你是对的，我的错误是我没有逐行检查你所有的代码。不过，这是我对您的问题 “如何将 2 个功能组合在一起以提高可维护性？” 的回答，我可以回答 “您的第二个选项”
@javaLover 不，我认为，这不仅仅是一个副本，那是你的错：你在第二个示例中复制了 doX 函数。
“每次调用都要进行条件检查”。所以？对性能有可衡量的影响吗？
好点一如既往，n.m。我测量了一个类似的案例，解决后，它节省了 5-10% 的 CPU。不过，我从未测量过这个具体案例，我接受我不太确定是否有任何影响。但是对于这个问题范围，我们假设有。

【解决方案3】：

从技术上讲，我更喜欢mvidelgauz's answer 中所示的解决方案，但是当必须实际调用函数时，添加的参数看起来很糟糕，因为没有自我解释的文字true 或false。所以我会结合三个函数：一个提供检查标志作为参数的内部（private）函数，以及两个提供关于函数的明显名称的外部函数，我更喜欢checkedInsert、uncheckedInsert .然后这些公共函数将调用 4 参数实现。

在下面的代码中，sn -p 减少了大部分参数以使解决方案最明显。它应该被视为类定义的一个片段：

public:
/// performs a fast insert without range checking. Range-check yourself before!
void checkedInsert(Element* element)
{
    insertWithCheckOption(element, true);
}

/// performs a range-checked insert
void uncheckedInsert(Element* element)
{
    insertWithCheckOption(element, false);
}

private:
/// implements insert with range check option
void insertWithCheckOption(Element* element, bool doRangeCheck)
{
    // ... do before code portion ...
    if (doRangeCheck) {
         // do expansive range checking  ...
    }
    // ... do after code portion ...
}

此解决方案同时提供：最好的用户界面和连贯的实现。
顺便说一句：我真的怀疑真正的性能问题是否会由单个条件检查引起。

【讨论】：

“但是当函数必须被实际调用时，添加的参数看起来很糟糕，因为不是自我解释的文字真假” 我完全同意你的看法！但是使用enum（带有very_long_extremely_descriptive_human_readable_values）而不是bool来提高可读性不是更简单吗？我认为这是可读性实践的经典示例，但我想专注于这个问题。许多其他考虑因素仍然可以添加到这个架构问题中。我们要在这里讨论所有这些吗？ )))
我尽量避免使用枚举（好吧，这应该在切换到 C++17 后重新考虑），因为将定义放在某处存在问题。直到现在，我才读到性能对 OP 来说是最高优先级。但是我从很多不好的经验中得出，可读性应该是这个列表中的第一位。所以也许我的回答在“这种特殊情况”中没有用。（？）
@mvidelgauz 另一点：我不喜欢太多参数，第三点：我不喜欢“只是在代码中添加”一些东西：如果有问题，我会尝试思考还有关于改变结构。
" 我不喜欢“只是在代码中添加”一些东西：如果有问题，我也会尝试考虑改变结构" 有时这种习惯导致过度设计，不是吗？
感谢 Wolf，我开始了解您的解决方案 - 注重可读性。虽然答案对范围问题不是那么有用，但对我来说非常有用。 “可读性应该是这个列表中的第一个”是一个很好的引用。 :)

【解决方案4】：

假设您制作了以下结构：

定义两个受保护方法do1 和do2 和一个公共抽象方法Insert 的类。

class BaseHolder
{
proteted:

void do1(/* complete with necessary parameters*/)
{

}

void do2(/* complete with necessary parameters*/)
{

}

public:
abstract void Insert(Holder* holder, Element* element, int index);

};

class Expensive : BaseHolder
{
public:
void Insert(Holder* holder, Element* element, int index)
{
    do1();
    holder->ensureRange(index);
    do2();
}
};

class Cheap : BaseHolder
{
public:
void Insert(Holder* holder, Element* element, int index)
{
    do1();
    do2();
}
};

对不起，如果我犯了一些语法错误，但这是我看到解决方案的方式。

另一种可能性是制作自定义的Cheap 和Expensive 类，它们都包装Holder，并在Expensive 构造函数中验证范围：

class Base
{
protected:
Holder* _holder;
public:
Holder* GetHolder(){ return _holder; }
}

class Cheap : Base
{
    public:
    Cheap(Holder* holder)
    {
        _holder = holder;
    }
};

class Expensive : Base
{
    public:
    Expensive(Holder* holder)
    {
         holder->ensureRange(index);
         _holder = holder;
    }
};

一个使用Cheap and Expensive 对象作为Insert 方法的参数。我猜第二个解决方案比第一个更好。

还有一个更像第一个的解决方案，但它使用模板方法设计模式，因为最好的出现在最后：

class BaseHolder
{
proteted:

void do1(/* complete with necessary parameters*/)
{

}

void do2(/* complete with necessary parameters*/)
{

}
virtual void check(Holder* holder, int index){  };
public:
void Insert(Holder* holder, Element* element, int index)
{
    do1();
    check(holder, index);
    do2();    
};

class Expensive : BaseHolder
{
protected:
override void check(Holder* holder, int index)
{ 
    holder->ensureRange(index);
}
};

class Cheap : BaseHolder
{
};

此解决方案仅在Expensive 对象上定义check 方法，它具有最多的代码重用，并且绝对是最干净的方法之一。不幸的是，这是关于 oop 设计的，而不是关于它不是最好的性能，但正如您刚刚得出的结论，您必须考虑哪个是您的优先事项。

【讨论】：

是否有任何理由不将方法设为静态？如果没有这样的原因，那么为什么它们不能只是命名空间下的函数呢？ IE。您能否在答案中添加使用示例？
我不关心语法或无关紧要的细节:)，但你的解决方案与我的第一个解决方案有同样的缺点 - 如果 do2() 想要 do1() 的局部变量，则不切实际。
@javaLover 您可以将这些值作为参数发送，或者将它们作为类保护字段保存，以便do2 也可以使用它们。
@javaLover 谢谢！这些方法将使您的代码更易于维护，但没有什么可以与简单的 if 检查相比性能，所以我猜从这个角度来看，您的第二个选项是最好的
这看起来不错，虽然我怀疑它的性能更好。它引入了很多间接甚至虚拟调用，快速的情况肯定会比任何原始情况都慢一些指令（和缓存未命中）

【解决方案5】：

作为另一种选择，您可以简单地使用默认函数参数，默认值对应于无范围检查

void insert(Holder* holder, Element* element, int index, 
            bool performRangeCheck = false)
{
  /* do some complex thing 1 */

  if(performRangeCheck)
  {
    holder->ensureRange(index);
  }

  /* do some complex thing 2 */
}

// ...

insert(holder, element, 2);       // no range check
insert(holder, element, 2, true); // perform range check

【讨论】：