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<class T>
void Vec_RemoveAll(vector<T>& 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<T>& 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 T
s, 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<T>& 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<T>& vec, const vector<UnaryPredicate> 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.