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?
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.
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();
}
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
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With