How can I make a list that can be accesed by other classes?

Question

I'm trying to make a project using the standard std::list. There is a list of names that have to be accessed by multiple classes. The list has to be inside a class. I'm currently trying with this code:

class MyClass {
private:
    list<string> myList =  {"one","two","three"};

public:
    list<string> GetList()
    {
        return myList;
    }
};

The class that I want to access the list from

class CallingClass {

    MyClass myClass;

public:
    void GetList()
    {
        list<string> calledList = myClass.GetList();
        for(int x=0;x<4;x++) { 
            cout<<calledList[x]<<endl; // I get error in this line: "no match for operator[]"
        }
    }
};

main

int main()
{
    CallingClass call;
    call.GetList();

    return 0;
}

I'm pretty clueless.

Answer 1

for(int x=0;x<4;x++) { 
    cout<<calledList[x]<<endl;
}

There are two problems with this for loop:

• the returned std::list only has 3 elements, but you are trying to access 4 elements, so the last iteration of the loop would go out of bounds, which is undefined behavior.

• Not that it matters, because std::list does not have an operator[] defined to begin with, which is what the compiler is warning you about. This is because std::list is not a random-access container, like std::vector is, so you can't access its elements by index. You have to walk through the std::list using iterators instead.

Try this:

list<string>::iterator iter = calledList.begin();
for(size_t x = 0; x < calledList.size(); x++) { 
    cout << *iter << endl;
    ++iter;
}

Alternatively:

for(list<string>::iterator iter = calledList.begin(); iter != calledList.end(); ++iter) {
    cout << *iter << endl;
}

Which can then be simplified further by using a range-for loop (in C++11 and later), which internally handles the iterators for you:

for(auto &elem : calledList) { 
    cout << elem << endl;
}

Answer 2

This has nothing to do, whatsoever, with what classes wants to access from which other class. It is something more fundamental.

cout<<calledList[x]<<endl;

calledList is a std::list. std::lists do not have an overloaded [] operator. Only std::vectors do. Either replace your lists with vectors, or use some other way to iterate over a list. How about range iteration, using modern C++?

for (auto &l : calledList)
   cout << l << endl;

Furthermore:

for(int x=0;x<4;x++) { 

Even if you could use [] with a list, you have another problem. The loop will end up accessing the first four values in the container. The shown container only has three values. This will result in undefined behavior.

To summarize:

1. std::lists do not have [] operators.

2. Using range iteration, and modern C++, results in safer and more readable code.

Answer 3

You have two other answers that explain your problem. I'm going to point out one more thing:

    const list<string> & GetList()
    {
        return myList;
    }
    ...
    const list<string> & calledList = myClass.GetList();

I made very slight changes. Your version returns a COPY of your list. This is inefficient. instead, you should return a const reference. Notice I added the const keyword and the & character.

And then when I use it, I also made the list a const reference.

Now, if you actually need a COPY, you can still do this:

list<string> calledList = myClass.GetList();

This will end up doing the same thing as your version. But most of the time, you are only going to read the list in some sort of const fashion, so you save a lot of work by returning a const reference and then assigning it to a const reference.