Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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.

like image 303
javaLover Avatar asked Dec 25 '22 03:12

javaLover


2 Answers

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();
}
like image 145
JVApen Avatar answered Jan 06 '23 01:01

JVApen


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
}

And if you can make decision at compile time and want to employ templates:

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
like image 32
mvidelgauz Avatar answered Jan 06 '23 00:01

mvidelgauz