I have a function that have very similar repeating code. I like to refactor it but don't want any complex mapping code. The code basically filter out columns in a table. I made this example simple by having the comparison statement having a simple type, but the real comparison can be more complex. I am hoping there may be some template or lambda technique that can do this.

if (rec->field2 != *field2)

vector<MyRecord*>& MyDb::Find(bool* field1, std::string* field2, int* field3)
{
  std::vector<MyRecord*>::iterator iter;
  filterList_.clear();
  std::copy(list_.begin(), list_.end(), back_inserter(filterList_));

  if (field1)
  {
    iter = filterList_.begin();
    while (iter != filterList_.end())
    {
      MyRecord* rec = *iter;
      if (rec->field1 != *field1)
      {
        filterList_.erase(iter);
        continue;
      }
      iter  ;
    }
  }

  if (field2)
  {
    iter = filterList_.begin();
    while (iter != filterList_.end())
    {
      MyRecord* rec = *iter;
 
      if (rec->field2 != *field2)
      {
        filterList_.erase(iter);
        continue;
      }
      iter  ;
    }
  }

  if (field3)
  {
    iter = filterList_.begin();
    while (iter != filterList_.end())
    {
      MyRecord* rec = *iter;
 
      if (rec->field3 != *field3)
      {
        filterList_.erase(iter);
        continue;
      }
      iter  ;
    }
  }
  return filterList_;
}

Just in case someone is curious, this is my final code. Thanks again everyone. A lot easy to understand and maintain.

vector<MyRecord*>& MyDb::Find(bool* field1, std::string* field2, int* field3)
{
  auto compare = [&](MyRecord* rec) {

    bool add = true;
    if (field1 && rec->field1 != *field1) {
       add = false;
    }
  
    if (field2 && rec->field2 != *field2) {
       add = false;
    }

    if (field3 && rec->field3 != *field3) {
       add = false;
    }
    return add;
  };

  filterList_.clear();

  std::copy_if(list_.begin(), list_.end(), back_inserter(filterList_), compare);
  return filterList_;
}

CodePudding user response：

you can use std::copy_if (as you already/would do a copy anyway)

vector<MyRecord*>& MyDb::Find(bool* field1, std::string* field2, int* field3){
  filterList_.clear();
  std::copy_if(list_.begin(), list_.end(), back_inserter(filterList_),[&](MyRecord* rec){
    // whatever condition you want.
    return field3 && rec->field3 != *field3;
  });
  return filterList_;
}

CodePudding user response：

Is there a simple way of refactoring this code?

As far as I understood your algorithm/ intention, using std::erase_if (c 20) you can replace the entire while loops as follows (Demo code):

#include <vector> // std::erase_if

std::vector<MyRecord*> // return by copy as filterList_ is local to function scope
Find(bool* field1 = nullptr, std::string* field2 = nullptr, int* field3 = nullptr)
{
    std::vector<MyRecord*> filterList_{ list_ }; // copy of original
    const auto erased = std::erase_if(filterList_, [=](MyRecord* record) { 
        return record 
            && ((field1 && record->field1 != *field1)
            || (field2 && record->field2 != *field2)
            || (field3 && record->field3 != *field3));
        }
    );
    return filterList_;
}

If no support for C 20, alternatively you can use erase–remove idiom, which is in effect happening under the hood of std::erase_if.