Vector of Enums. Weird iterator behaviour

[mongrol]

Hi folks,
I've got a vector with a couple of enums in it and I can't see to get a sane iterator from it using begin() and end(). Here's my code.

enum weaponMode_t {
	SINGLE,
	BURST
};

class Weapon{
public:	
	std::vector<weaponMode_t> weaponModes;
	std::vector<weaponMode_t>::iterator weaponModesIterator;
	weaponMode_t selectedWeaponMode;
	void setWeaponMode();
};

The vector is populated in the weapon constructor. Edited version below with only 1 case;

Weapon::Weapon(weaponName_t weaponName)
{
	switch (weaponName) {		
		case ASSAULT:
			weaponModes.resize(2);
			weaponModes[0]=SINGLE;
			weaponModes[1]=BURST;
			break;
		default:
			break;
	}
	weaponModesIterator = weaponModes.begin();
	selectedWeaponMode = *weaponModesIterator;
}

I then press a key to iterate around the vector.

void Weapon::setWeaponMode()
{
	Utility tool;
	selectedWeaponMode = *weaponModesIterator;
	weaponModesIterator++;
	if (weaponModesIterator==weaponModes.end()) weaponModesIterator=weaponModes.begin();	
}

What's happening is end() and begin() are megabytes apart and pressing the key advances the iterator in the expected 4 byte blocks. back() works correctly and kinda proves that it's begin() at fault. Am I doing something completely n00b or do enums operate differently in vectors?

I've got similar code iterating around vectors of classes and strings with no problems. Staring at the code for many hours doesn't seem to fix it either. ;)

[guestgulkan]

It is possible that if you get the begin() of the vector in one function ( say for example in a constructor) and save that value, then resize the vector in a later function, then it is possible that the vector may have to deallocate/reallocate memory - which means that the original
begin() value is now INVALID !!

By the way
The vector::resize() function actually takes two parameters - the correct signature is void resize ( size_type sz, T c = T() );.

So vector.resize(2) will add two objects (created using the object's default constructor) to the objects to the vector.

[mongrol]

Thanks for the reply. I only ever call resize once and that's in the constructor. I've worked around the issue by moving the final begin() statement out of the class constructor and call it basically immediatly from outside the class. This works ok but isn't how I expect it to work. Clearly there's details in how constructors work that I'm missing.

[jsmith]

I would strongly suggest that you not keep an iterator as a data member of the class. At a minimum it will save you headaches down the road (see guestgulkan's post) and is very likely the source of your troubles now, because by keeping an iterator as a data member, your Weapon class is semantically non-copyable, but syntactically you can copy it.

If you really, really, really must keep an iterator data member, then you are going to have to write your own copy constructor and assignment operator that takes care of filling out the iterator correctly in the copy.

[mongrol]

Thanks jsmith. I now see that that is exactly what my problem is. Can you tell me how or where I've missed that iterators can't be a data member? I haven't noticed that mentioned in the reference anywhere and I'd like to be aware of such things in the future.

[jsmith]

It's not that they can't be data members, it's just that you have to be extremely careful, so much so that most seasoned programmers will suggest not making them data members. The reason you have to be careful is the
same reason that various sources explain that certain member functions of containers will potentially invalidate iterators.

Consider vector, for example. Behind the scenes, vector is simply an array, and an iterator into the vector is
simply a pointer to an element. If vector ever wants to resize the array, it does so by allocating a second array,
larger than the original, copying all the elements from the original to the second array, and then destroying the first array. The result of this operation is that the address of every element in the vector has now changed. Since iterators are simply pointers to elements, it stands to reason that any iterator you had kept prior to the resize is now pointing to free memory, and not to an element in the vector.

Your problem was that you were copying the container implicitly through your class' implicit copy constructor.
By default, implicit constructors are implemented as member-wise copy. One of your data members was a
vector. How are vectors copied? Well, vector's copy constructor allocates an its own array and copies each
element from the source to the new array. As such, the address of every element in the new vector is different from the address of the corresponding element in the original. But the iterator data member -- it's just a pointer, so the pointer gets copied. So now the copy has an iterator not into its own vector, but rather into the vector that was copied from.

[mongrol]

Thanks a lot Mr Smith. Perfectly understood. I've now removed all my iterator code where it's guilty and used a simpler incrementing index instead. Nice and tidy.

[jsmith]

Another alternative is to always make the "active" weapon mode the first mode in the container. If you only have a handful of weapon modes, leaving the container as a vector is fine, otherwise I'd suggest std::list at the point for performance reasons.

The only reason I suggest this is because the power of std::vector -- operator[] with array index access -- is also its bane, in that since very few other containers provide this type of access (std::deque comes to mind as the only one; map uses operator[] for lookup), your code as is will only work with vector and deque and would have to undergo modification to work with any other non-associative container type.

Just a thought.