views:

418

answers:

5

I have a class (foo) that contains a vector.

If i try iterating over the elements in the vector like so:

for(vector<random>::iterator it = foo.getVector().begin();
        it != foo.getVector().end(); ++it) {
  cout << (*it) << endl;

}

The first element is always corrupted and returns garbage data.

However, if do something like:

 vector<random> v = foo.getVector();
 for(vector<random>::iterator it = v.begin();
            it != v.end(); ++it) {
      cout << (*it) << endl;

 }

Everything appears to be working fine. Is there a "gotcha" that I do not know about?

I've also tried doing cout << foo.getVector()[0] << endl; outside of the loop but that appears to be working ok.

Thanks.

Edit:

Here's my header file:

#ifndef HITS
#define HITS

#include <vector>
#include "wrappers.h"

class Hits {

    public:
        Hits();
        std::vector<word_idx_value> getVector() {return speech_hits;}
        const std::vector<word_idx_value> getVector() const {return speech_hits;}
        void add(const word_idx_value&);
        Hits &operator+=(const Hits&);
    private:
        std::vector<word_idx_value> speech_hits;
};

#endif
+7  A: 

getVector() returns a vector by value. The two invocations of getVector (begin() and end()) return different copies of the vector, so you call begin() on one object and end() on another. What you get is two iterators into two different containers. Comparing those two iterators with != yields an undefined value.

+9  A: 
for(vector<random>::iterator it = foo.getVector().begin();

The temporary vector is returned when you do foo.getVector() and it gets destroyed the moment ; is encountered after foo.getVector().begin(); Hence iterator becomes invalid inside the loop.

If you store the value of foo.getVector(); in vector v ( v = foo.getVector();) and then use the vector v, it works fine. It is because the vector v is valid throughout the loop.

aJ
That's correct. But note that even something like std::copy(foo.getVector().begin(), foo.getVector().end(), dest) has undefined behavior (as in my answer), even though both temporary vectors live until std::copy returns.
Yes. As you mentioned it is two copies of vector and iterators are different.
aJ
+2  A: 

getVector() returns vector by value and in the first case you get a temporary variable that gets destroyed once you're inside the loop. In the second case you copy the result into a local variable that is still alive while inside the loop. Possible solution is to return vector by const reference.

sharptooth
A: 

modify the getVector function to return the object reference like this: std::vector<word_idx_value>& getVector() {return speech_hits;}

+1  A: 

You error is in the getVector() method. Return by reference.

class Hits
{
    public:
    std::vector<word_idx_value>&   getVector() {return speech_hits;}
    //                         ^
    //                      Add the & to return by reference.

    // You may also want a const version at some point.
    std::vector<word_idx_value> const&   getVector() const {return speech_hits;}

If you don;t return by reference you are creating a temporary copy. The copy is then destroyed after it has been used. In this case after the begin() has executed the temporary object is destroyed, and thus the iterator returned by begin() is not valid.

Martin York