Return to Answer

• 1

• 87.5k
• 14
• 104
• 322

• 3.4k
• 17
• 23

Before getting into each function, your last question is important -- how do we generalize the type of container? By passing iterators instead of containers, of course! When we first encounter the C++ Standard Library, the ubiquity of iterators can seem confusing -- why not pass containers around instead? The reason is that different programs have many different needs, and passing a particular kind of container can be very restricting. For example, if my function takes a parameter of type const vector<const Foo>&, I cannot pass a const vector<Foo>& or a const vector<const Foo*>&. If instead I take a pair of iterators with the requirement that *it is of type const Foo&, I can use a wider range of container types; vector<Foo>'s and vector<const Foo>'s iterators work directly, and vector<const Foo*> can be made to work with iterator adaptors.

OK, function by function.

1. Simply Remove All Instances of a Value From a Vector ====

   template void Vec_RemoveAll(vector& vec, T val) { vec.erase(remove(vec.begin(), vec.end(), val), vec.end()); }

First I'll include my standard disclaimer about using namespace std;: Don't do it, it can lead to subtle bugs, and you'll get used to seeing std:: everywhere.

Second, pass const T&, not T. What if T is a large type? std::remove takes const T&.

Third, I don't think this is a particularly wise function to write. It's a common C++ idiom; once you learn it it should become automatic.

2. Remove All Instances of a Container of Values From a Vector ====

   template<class Container, class T> void Vec_RemoveAll(vector& vec, const Container& vals) { for (const T& val : vals) vec.erase(remove(vec.begin(), vec.end(), val), vec.end()); }

First point is a question: Is this the performance characteristic you want? Intuitively, I would think that vals would tend to be smaller than vec, so you probably want to iterate through vals in the inner loop, not the outer loop. You could achieve this by using std::remove_if instead and writing a function that determines whether an element is in vals. You could get even better performance if you required vals to be sorted.

Second: for (const T& val : vals) is dangerous. Container isn't required to contains Ts, just things that are comparable to T. You would want to write this as for (const auto& val : vals). For example, consider that vec is a vector<string> and vals is a vector<const char*>. Each of the strings in vals would be copied into a string while iterating using your version.

3. Remove All Elements That Meet Unary Predicate's Condition ====

   template<class T, class UnaryPredicate> void Vec_RemoveAll_If(vector& vec, UnaryPredicate* predicate) { vec.erase(remove_if(vec.begin(), vec.end(), predicate), vec.end()); }

Almost right, except that you should pass UnaryPredicate, not UnaryPredicate*. Your version works fine for function pointers but not function objects. But I raise the same objection as to function 1 -- this is a standard idiom, and hiding it behind a function call is more likely to confuse than enlighten experience C++ programmers who read your code.

4. Remove Elements That Meet Any of the Predicates' Conditions ===

template<class T, class UnaryPredicate> void Vec_RemoveAll_If(vector& vec, const vector predicates) { for (const UnaryPredicate& predicate : predicates) vec.erase(remove_if(vec.begin(), vec.end(), predicate), vec.end()); }

I recommend against writing this function in the first place; it's probably best for any potential user of this function to compose their predicates manually into a single predicate, or in the alternative to write a library to help them do so. That is more generally applicable and removes the requirements that users of your library learn this idiosyncratic API. In the alternative, you can do as you did in function 2, and pass const Container& instead of trying to specify a specific container.