Group functions two that differ in only one line of code

I have two critical functions, such as:

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 to improve serviceability?

My bad decisions:

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

That would be ugly. It is also impractical if do2local variables from are required 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 requires a conditional check for each 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 criteria for how good the approach is sorted as follows: -
1. Better performance
2. Less duplicate code
3. Smaller overall line of code
4. Easier to read for experts and beginners

2: . mvidelgauz Wolf.

+4
5

2 . , . ( ). , if-statement, , , . ( , , ...)

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

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

, :

, 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, wrapp a 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
{
};

check Expensive, , , . , oop , , , , , .

+1

, , mvidelgauz answer, , - - true false. : (private) , , , - checkedInsert, uncheckedInsert, 4 .

, . :

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

This solution provides both: the best user interface and consistent implementation.
BTW: I really doubt that real performance problems will ever be caused by a single conditional check.

+1
source

As another alternative, you can simply use the default function argument with a default value that does not match the range check

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
0
source

Source: https://habr.com/ru/post/1650446/


All Articles