Using an STL algorithm on vector of pointers of an object (C++)

Question

I need to count the number of times an object of an array of pointers has the same name(member variable) as the parameter given to a member function. I have tried different approaches, but none of them worked. My code does not even compile. The error is: "error C2514: 'MyComparator' : class has no constructors" and here is the code of the class I use for the comparision and the function used for counting concurencies.

 //definition of the vector
 vector<Class1*> files;
 ...
 int Class2::countNames(string name)
    {
       return count_if(files.begin(), files.end(), bind2nd(MyComparator(),name));
    }
 ...
class MyComparator{
public:
 bool operator()(const CFileType*& ob1, const string &str)
 {
   return ob1->getName()==str;
 }
};

I am struggling with this for hours and I want to do it using STL. The catch here is that i have an vector of pointers, if I had a normal vector it would not need a predicate function, in my case I have to give a parameter to it and I think bind2nd() is the right way. Any help would be greatly appreciated!

Answer 1

Under C++11, you could use a lambda expression, which is much easier than the archaic bind2nd. For example:

#include <string>
#include <vector>
#include <algorithm>
#include <iostream>

class Class1 {
public:
    Class1(const std::string& name) : Name(name) {
    }
    const std::string& getName() const {
        return Name;
    }
private:
    std::string Name;
};

size_t CountMatchingNames(const std::vector<Class1*>& v, const std::string& name) {
    return std::count_if(
        v.begin(),
        v.end(),
        [&name](Class1* c1) { return name == c1->getName(); }
    );
}

int main() {

    Class1 rob("rob");
    Class1 bob("bob");
    Class1 mitch("mitch");

    std::vector<Class1*> v;
    v.push_back(&bob);
    v.push_back(&mitch);
    v.push_back(&rob);
    v.push_back(&mitch);
    v.push_back(&bob);
    v.push_back(&bob);

    std::cout << "rob count:\t" << CountMatchingNames(v, "rob") << std::endl;
    std::cout << "bob count:\t" << CountMatchingNames(v, "bob") << std::endl;
    std::cout << "mitch count:\t" << CountMatchingNames(v, "mitch") << std::endl;

    return EXIT_SUCCESS;

}

This prints:

rob count:      1
bob count:      3
mitch count:    2

Answer 2

First off, you need to make sure that your comparison class is known when you use it: moving the definition of MyComparator in front of the definition of countNames() would be my first step. That said, bind2nd() wants to know the function objects it is dealing with a bit better than you what you offer: it generally wants to know the result_type, the first_argument_type, and the second_argument_type. You can either get these by from std::binary_function<bool, CFileType const*, std::string const&> or by explicitly defining them. Although I don't think it is required, you might want to make the function call operator const. On the other hand, if you are defining a function object rather than a function (for which you could get the necessary typedefs using std::ptr_fun(); personally I think the name is intended to somehow make these guys appealing because it certainly isn't much fun when you have to use them), you can just go the entire way and not meddle with binders in the first place:

class MyComparator {
public:
    MyComparator(std::string const& value): value_(value) {}
    bool operator()(CFileType const* obj) const {
        return obj->getName() == this->value_;
    }
private:
    std::string value_;
};
...
std::count_if(files.begin(), files.end(), MyComparator(name));

You can get roughly the same effect using binders when defining a function and binding it. Typeically, I get it wrong when I don't try out the code but it would look something like this:

bool myComparator(CFileType const* obj, std::string name) {
    return obj->getName() == name;
}
...
std::count_if(files.begin(), files.end(),
              std::bind2nd(std::ptr_fun(myComparator), name));

If you feel you want to pass the name argument by reference rather than copying it all the time, you won't be able to use std::bind2nd(), at least not unless you use a C++ 2011 compiler: it would create a type which has to consecutive const keywords which the compiler won't like. You can use e.g. boost::bind() which doesn't have this problem:

std::count_if(files.begin(), files.end(), boost::bind(myComparator, _1, name));

... and change the declaration of the name parameter to be std::string const& name.

Answer 3

You have a few problems here:

• It looks like you're defining MyComparator after using it. Types need to be defined before use.
• The vector contains Class1*, but the comparator works with CFileType*.
• The deprecated bind1st and bind2nd are a bit tricky to use, and require various types to be defined by the function type. Assuming you can't use the new std::bind (or boost::bind), the easiest way to fix that is for MyComparator to inherit from std::binary_function<Class1*, string, bool>
• The operator() needs to be declared const.

If your compiler supports C++11, then you could write this more simply using either std::bind or a lambda:

count_if(files.begin(), files.end(), bind(MyComparator(), placeholders::_1, name));

or

count_if(files.begin(), files.end(), [&name](Class1 const * p){return p->getName()==name;});

Answer 4

In principle your idea works, however:

1. the comparer class has to be defined before it is used;
2. it has to inherit from binary_function to include necessary typedefs;
3. its operator() needs to be declared const.

With these corrections, the following example works for me:

#include <vector>
#include <functional>
#include <string>
#include <algorithm>
#include <iostream>

using namespace std;

struct Class1 {
  string getName() const { return "aaa"; }
};

//...
class MyComparator: public binary_function<const Class1*, string, bool> {
public:
 bool operator()(const Class1* ob1, const string &str) const
 {
   return ob1->getName()==str;
 }
};

vector<Class1*> files;
//...
 int countNames(string name)
    {
       return count_if(files.begin(), files.end(), bind2nd(MyComparator(),name));
    }

int main() {
  files.push_back(new Class1);
  files.push_back(new Class1);
  cout << countNames("aaa") << ' ' << countNames("bbb") << endl;
}

Note, however, that having a vector of pointers easily leads to memory leaks (like in my example). Consider using Boost.PointerContainer or (with C++11) a container of unique_ptrs.