C question about iterating over std::vector-CodePudding

My question ("why it doesn't work?") concerns the small sample below.

When I run this (after g testThis.cc -o testThis) I get:

printing 101
printing 102
printing 103
                                 printing 100
                                 printing 100
                                 printing -1021296524

It should not be a problem of myHolder dropping out of scope, so why make a local copy of the vector itself?

It should stay cozy in the class itself.

#include <vector>
#include <iostream>

class stdHolder {

public:

  stdHolder();

  std::vector<int> getSV() const {return _myVector;}

private:

  std::vector<int> _myVector;

};

stdHolder::stdHolder() {

  _myVector.push_back(1);
  _myVector.push_back(2);
  _myVector.push_back(3);

}

int main() {

  stdHolder myHolder;

  // the following works
  std::vector<int> localSV = myHolder.getSV();
  for (std::vector<int>::iterator it = localSV.begin(); it != localSV.end(); it   ) {
    std::cout << "printing " << *it   100 << std::endl;
  }

  //return 0; // comment this line to see my problem

  // the following loops forever
  for (std::vector<int>::iterator it = myHolder.getSV().begin(); it != myHolder.getSV().end(); it   ) {
    std::cout << "                                 printing " << *it   100 << std::endl;
  }

  return 0;

CodePudding user response：

My question ("why it doesn't work?")

Because myHolder.getSV().begin() and myHolder.getSV().end() work on different vectors since you're calling the member function stdHolder::getSV() twice and for each invocation there will a different vector returned(as you're returning a vector by value).

That is, the iterator it is initialized with myHolder.getSV().begin() but then you're comparing it(pun intended) with the result of myHolder.getSV().end() which is an iterator to a completely different vector returned by the call myHolder.getSV().

One way to solve this is by returning the vector by const reference. That way the first loop will copy from the returned value while the second loop will work correctly because it would be getting iterators off the same vector data member. Note that if you use this then for the second loop you would have to make it a const_iterator.

See the comments in the below modified program:

class stdHolder {

public:

  stdHolder();
//-----------------------v---------------------------------->return by const lvalue reference
  const std::vector<int> &getSV() const {return _myVector;}

private:

  std::vector<int> _myVector;

};

stdHolder::stdHolder() {

  _myVector.push_back(1);
  _myVector.push_back(2);
  _myVector.push_back(3);

}

int main() {

  stdHolder myHolder;

  //still works 
  std::vector<int> localSV = myHolder.getSV();
  for (std::vector<int>::iterator it = localSV.begin(); it != localSV.end(); it   ) {
    std::cout << "printing " << *it   100 << std::endl;
  }

  //this works too now
  //---------------------vvvvvvvvvvvvvv--------------------------------------->const_iterator used instead of iterator
  for (std::vector<int>::const_iterator it = myHolder.getSV().begin(); it != myHolder.getSV().end(); it   ) {
    std::cout << "                                 printing " << *it   100 << std::endl;
  }

  return 0;
}

Other option is to just add begin and end member function for your stdHolder and then you'll be able to use range-based for loop.