snippetcppMinor
Filter function implementations using copy_if algorithm vs. for loop
Viewed 0 times
implementationscopy_ifloopfunctionalgorithmfilterusingfor
Problem
I have the following method of some class, which also defines the method
This is a clear case where
or
Both seem much less obvious than the original, so I am inclined to keep the
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
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
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.
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.