将仅 1 行代码不同的两个函数分组

Question

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

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。

Answer 1

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

Answer 2

假设您制作了以下结构：

A class 定义了两个受保护的方法 do1 和 do2 以及一个 public 抽象方法 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 classes，它们都包装一个 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;
    }
};

使用廉价和昂贵的对象作为 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 设计的，而不是它不是最好的性能，但正如您刚刚得出的结论，您将不得不考虑哪个是您的优先事项。

Answer 3

从技术上讲，我更喜欢中所示的解决方案，但由于无法自我解释的文字 true 或必须实际调用函数时，添加的参数看起来很糟糕false。因此，我将组合三个函数：一个提供检查标志作为参数的内部 (private) 函数，以及两个提供关于该函数的明显名称的外部函数，我更喜欢 checkedInsert , uncheckedInsert。然后，这些 public 函数将调用 4 参数实现。

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

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

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

Answer 4

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

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-statement 中的代码被删除，因为它甚至不必编译。

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

Answer 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

将仅 1 行代码不同的两个函数分组

Group two functions that differ in only 1 line of code

c++

maintainability

performance

function

code-duplication

我糟糕的解决方案：