Thread: Pointer to List Iterator As Function Argument

Your usage of globals is not very good. Consider using return values and more meaningful names. (Also fixed some constness error - getter should be const members.)

Code:

#include <iostream>
#include <list>
#include <string>
using namespace std;

class person
{
private:
    string name;
    int id;
public:
    person(const string& nm, int i) {name = nm; id = i;};
    int getid() const	{ return id; };
    string getname() const    { return name; };
};

list<person>::iterator find_by_id(list<person>& persons, int target_id);

int main()
{
    //lets not use globals, OK!?
    list<person> personlist;

    personlist.push_back(person("Arnold", 0));
    personlist.push_back(person("Barry", 1));
    personlist.push_back(person("Colin", 2));

    list<person>::iterator iter = find_by_id(personlist, 2);
    if (iter != personlist.end()) {
        cout << "name of person with id 2 = " << iter->getname() << endl;
    }
    else {
        cout << "person with id 2 not found";
    }
}

list<person>::iterator find_by_id(list<person>& persons, int target_id)
{
    list<person>::iterator result;
    for (result = persons.begin(); result != persons.end(); ++result) {
        if (result->getid() == target_id) {
            break;
        }
    }
    return result;
}

However, a preferable way is to use the standard find_if algorithm. Unfortunately the condition is quite complicated, so a function object is needed (matching_id), but you can compare the complexity of that object and the function to find by id specifically.

Another reason why that would be preferable is that for the sake of correctness the previous code would require another overload of find_by_id that takes a const list and returns a const_iterator.

Code:

#include <iostream>
#include <list>
#include <string>
#include <algorithm>
using namespace std;

class person
{
private:
    string name;
    int id;
public:
    person(const string& nm, int i) {name = nm; id = i;};
    int getid() const	{ return id; };
    string getname() const    { return name; };
};

//function object is something that overloads operator()
class matching_id
{
    int target_id;
public:
    matching_id(int id): target_id(id) {}
    bool operator()(const person& p) const
    {
        return p.getid() == target_id;
    }
};

int main()
{
    //lets not use globals, OK!?
    list<person> personlist;

    personlist.push_back(person("Arnold", 0));
    personlist.push_back(person("Barry", 1));
    personlist.push_back(person("Colin", 2));

    list<person>::iterator iter = find_if(personlist.begin(), personlist.end(), matching_id(2));
    if (iter != personlist.end()) {
        cout << "name of person with id 2 = " << iter->getname() << endl;
    }
    else {
        cout << "person with id 2 not found";
    }
}

You don't even need a function object if you can use Boost.Lambda and Boost.Bind, but this is very advanced stuff:

Code:

list<person>::iterator iter = find_if(personlist.begin(), personlist.end(), boost::bind(&person::getid, _1) == 2);

3. Is a list a good choice of container here or would a vector be better? If so, how come?

It doesn't matter here. The main reason to use list is when you want to insert and removes items everywhere in the container. If you want random access to container contents then vector might be a better choice.

The idea is that you make a getter method look something like this:

Code:

class Something {
private:
    int data;
public:
    int get_data() const { return data; }
};

That way you're specifying, in effect, that the "this" pointer for get_data() will be const. So you won't be able to modify the class at all (except for any mutable members).

This is good for two reasons. One, this is what you want getters to do. You don't want to let getter methods modify the object. And two, this lets you call the method from a constant object. If you tried to do this, for example,

Code:

void print_data(const Something &something) {
    std::cout << something.get_data() << std::endl;
}

and get_data() wasn't declared const, you'd get a compiler error.

[edit] I'm assuming tabstop's post below is in response to bengreenwood and not to me . . . ? [/edit]

Originally Posted by bengreenwood

So say if there was another member of person, age, which changed over time. If you had a get_age function, that wouldn't be declared as const would it? Because it might change in future, I mean.

You would still declare that member function as const because with respect to that member function, the observable state of the object does not change, even though it may change by the use of other functions.