javaLover javaLover - 4 months ago 12
C++ Question

Group two functions that differ in only 1 line of code

I have two performance-critical functions like this:

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
}


How to group 2 functions together to increase maintainability?

My poor solutions:



Solution 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();
}


It would be ugly.
It also impractical if
do2
want some local variables from
do1
.

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


It costs a conditional checking for every call.

Solution 3. (draft)

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() { }


Overkill and unnecessary complexity?

Edit 1:
The priority of criteria for how good an approach is, are sorted as followed:-

1. Best performance

2. Less duplicate of code

3. Less total line of code

4. Easier to read for expert & beginner

Edit 2: edit the 3rd solution. Thank mvidelgauz and Wolf.

Answer

Your solution 2 is actually not yet that bad. If this code is within a header, it is implicitly considered as inline code. (I've written it explicitly) If you call it with either true or false, the compiler can remove the if-statement, though it depends on a range of factors to know if it will do so. (Size of the whole body after inlining, visibility of the constant, tuning ...)

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

The solution 3 is actually what you want to achieve, as templates require a new function instantiation for each different call, so it would remove the dead code. However it can be written very similar to how you wrote solution 2.

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

If you have C++17, you no longer have to depend on the compiler to remove dead code as you can enforce it to skip certain code via constexpr-if. This construction will guarantee that the code in the if-statement is removed as it ain't even have to compile.

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