Post subject: Weird STL Iterator Behavior
Joined: 3/31/2005
Posts: 148
Location: Colorado
Hi all, I figure some of the best coders on the net hang out here, so. . . In the pasted code, my iterator orderingIter loses its value between the constructor call and the triangulate call at the bottom in my code. The vector itself is still OK in the traingulate call, but *orderingIter is 0 before and after it is incremented. The only time the string "ordering" occurs in my code is in the constructor and triangulate methods. My first thought was that I was aliasing the iterator, but I couldn't see where that might be happening. I compiled with -Wall, and didn't see anything relating to orderingIter. Thanks for the help! (btw, I'm petafile on IRC, so hit me up) http://rafb.net/p/Q4IHfu55.html
Do not try to bend the spoon, that's impossible. Instead only try to realize the truth. What Truth? There is nospoon. Then you will see it is not the spoon that changes, it is only yourself
Post subject: Re: Weird STL Iterator Behavior
Editor, Active player (296)
Joined: 3/8/2004
Posts: 7469
Location: Arzareth
You did not provide the whole code, so I cannot tell what's wrong. I'll guess though: You're making a copy of the "dag" object. When you make a copy, the iterator continues to point to the original vector's element, not the copy. If the original vector is deallocated after copying, you will have an iterator pointing nowhere. E.g.
dag GetDag()
{
    dag gruu;
    return gruu;
}

void meow()
{
    dag tmp = GetDag(); // this does copying, original gets destructed
    tmp.triangulate(); // works on an obsolete iterator
}
To fix, follow either one of these solutions: 1) Instead of a storing an iterator possessing a pointer, store an index 2) Implement the copy constructor and the assign operator.
Joined: 3/31/2005
Posts: 148
Location: Colorado
Thanks, but I'm pretty sure I don't copy the dag object. My main function is here: http://rafb.net/p/NHbFWb31.html
Do not try to bend the spoon, that's impossible. Instead only try to realize the truth. What Truth? There is nospoon. Then you will see it is not the spoon that changes, it is only yourself
Editor, Active player (296)
Joined: 3/8/2004
Posts: 7469
Location: Arzareth
Thereisnospoon wrote:
Thanks, but I'm pretty sure I don't copy the dag object. My main function is here: http://rafb.net/p/NHbFWb31.html
Oh yes you do.
dag d; d = dag(points);
The first line constructs a dag object. (Scope=program, destructor will be called at program termination) The second line constructs a second, temporary dag object, and then calls the operator=() for the first dag object, with the second dag object as its parameter; then the second dag object is destructed. d = dag(points); is equivalent in function to the following code:
 { char temporarybuffer[sizeof(dag)];
new(temporarybuffer) dag(points); // construct a temporary

d.operator= ( (const dag*)temporarybuffer ); // assign the temporary to d by the means of the assign operator (=)

((dag*)temporarybuffer)->~dag(); // destruct the temporary
} // end of scope for the temporary
Banned User, Former player
Joined: 3/10/2004
Posts: 7698
Location: Finland
Having an iterator as member variable of a class which can be copied/assigned is indeed dangerous, unless you implement the copy constructor and assignment operator properly. That's because the automatic copy constructor/assignment operator the compiler generates will copy the iterator verbatim (there's no way it can know that you want it to point to the copied member data container). Since it's a vector iterator, the solution is rather simple, as Bisqwit stated: Use an index rather than an iterator. If you really need an iterator, and your class must be copyable/assignable, then there's no way around it: You'll have to write a proper copy constructor and assignment operator which set the iterator of the copy appropriately. If your class does not need to be copyable/assignable, then it's STRONGLY suggested that you disallow both operations (by declaring them private). It will catch such errors at compile time.
Joined: 3/31/2005
Posts: 148
Location: Colorado
Thanks for the help. A nice unsigned int worked much better than the iterator. Fixed, and on to new bugs:)
Do not try to bend the spoon, that's impossible. Instead only try to realize the truth. What Truth? There is nospoon. Then you will see it is not the spoon that changes, it is only yourself
Editor, Active player (296)
Joined: 3/8/2004
Posts: 7469
Location: Arzareth
Thereisnospoon wrote:
Thanks for the help. A nice unsigned int worked much better than the iterator. Fixed, and on to new bugs:)
Rather than unsigned int, I recommend size_t, or if you aren't afraid of verbosity, std::vector<int>::size_type. This helps your program's portability. It won't crash on a 64-bit platform if your vector contains an excess of 4294967295 elements. I've learned this the hard way, by having many programs crash when they have this kind of code:
std::string k = "kupo";
unsigned pos = k.find("q");
if(pos != k.npos) k[pos] = 'u';
Works just fine on 32-bit, crashes on 64-bit. When you change the unsigned into std::string::size_type, it works as intended.
Joined: 4/7/2008
Posts: 117
Or typedef it in your class and get the best of both:
typedef std::vector<int>::size_type index_type;
Or something. :)
Banned User, Former player
Joined: 3/10/2004
Posts: 7698
Location: Finland
Personally I have always used size_t instead of whatever_container::size_type because it's easier to write. However, technically speaking doing that is not guaranteed to work because, AFAIK, the standard doesn't guarantee that the two types will always have the same size. In practice I think 100% of compilers in any desktop or server system will have an identical type for both. There might be some really obscure embedded systems for which they aren't.