Follow

Keep Up to Date with the Most Important News

By pressing the Subscribe button, you confirm that you have read and are agreeing to our Privacy Policy and Terms of Use
Contact

Is there a simple way of refactoring this code?

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_;
}

>Solution :

MEDevel.com: Open-source for Healthcare and Education

Collecting and validating open-source software for healthcare, education, enterprise, development, medical imaging, medical records, and digital pathology.

Visit Medevel

Is there a simple way of refactoring this code?

As far as I understood your algorithm/ intention, using std::erase_if () 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.

Add a comment

Leave a Reply

Keep Up to Date with the Most Important News

By pressing the Subscribe button, you confirm that you have read and are agreeing to our Privacy Policy and Terms of Use

Discover more from Dev solutions

Subscribe now to keep reading and get access to the full archive.

Continue reading