HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Filter function implementations using copy_if algorithm vs. for loop

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationscopy_ifloopfunctionalgorithmfilterusingfor

Problem

I have the following method of some class, which also defines the method isAllowed:

auto filter(const auto& in)
{
    auto ret = decltype(in) {};
    for(auto x : in)
      if(isAllowed(x))
        ret.insert(x);
    return ret;
}


This is a clear case where copy_if could be used instead. I see two alternative versions:

auto filter(const auto& in)
{
    auto ret = decltype(in) {};
    copy_if(begin(in), end(in),
            inserter(ret, end(ret)), 
            [this](auto i) {return this->isAllowed(i);});
    return ret;
}


or

auto filter(const auto& in)
{
    auto ret = decltype(in) {};
    copy_if(begin(in), end(in),
            inserter(ret, end(ret)), 
            bind(&A::isAllowed, this, placeholders::_1));
    return ret;
}


Both seem much less obvious than the original, so I am inclined to keep the for loop. Is there a strong argument not to? (And is there a better way?)

Otherwise, I feel itchy because cases like this point to the limited usefulness of the algorithm tools, despite what best practices advise.

There are probably good arguments for choosing copy_if that are not being considered properly (performance?). This question is mainly after those, as the others seem more intuitive and easier to get.

Solution

I'll respectfully disagree with other answers that suggest using anything but the first option. The only thing the other two std-library-heavy approaches accomplish is to obfuscate the code. Never underestimate the importance of keeping it simple. Complexity is the number one enemy of any piece of software.

The first one is not only more explicit about what is happening, but it is also universal. Any high-level programming language has the concept of a for-loop, so an occasion reader coming from another language background can still clearly see what's going on over there. A JavaScript programmer would have a very hard time figuring out what an inserter or a placeholder is. Even a novice C++ programmer with limited knowledge of the library would have to waste time digging through documentation to understand a simple function that does a conditional copy.

Don't get me wrong here, I'm not making a case against the Standard Library,
I'm just pointing out that, like any programming language feature, it can be overused. In the three examples presented by the OP, the second and third clearly are overusing the std lib just for the sake of using library feature XYZ. My opinion.

Context

StackExchange Code Review Q#84016, answer score: 4

Revisions (0)

No revisions yet.