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.