Iterators with classes.

[TotallyLearning]

Alright, so the past few days, I've been trying to study vectors, list, and maps.

I'm currently starting with vectors, then am going to experiment with the other two. I'm down to one problem though. I'm given a error saying that I cannot convert my vector containing a class, to my constant iterator.

This is not my exact code, for personal reasons, but it has the general idea.

class A
{
int Something;

public:
  A(int SomethingsValue);
  ~A();
};

class B
{
std::vector<Class A> ClassAList;

public:
    B();
    ~B();
};

A::A(int SomethingsValue)
{
Something = SomethingsValue;
};

A::~A()
{
};

B::B()
{
};

B::~B()
{
for(std::vector<A>::iterator inter = ClassAList.begin(); inter !=     ClassAList.end(); inter++)   //Sorry, right here, the forums auto moved it over
    {
      ClassAList.erase(*inter);
    };
};

I've seen people talking about doing it like this and I've tried multiple other ways, but none of them have worked.

Heres the error with a few name changes:

error C2664: 'std::_Vector_iterator<_Ty,_Alloc> std::vector<_Ty>::erase(std::_Vector_const_iterator<_Ty,_Alloc>)' : cannot convert parameter 1 from 'ClassA' to 'std::_Vector_const_iterator<_Ty,_Alloc>'
1>        with
1>        [
1>            _Ty=ClassA,
1>            _Alloc=std::allocator<ClassA>
1>        ]
1>        No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

Any help, would be much appreciated.

[shacktar]

std::vector<A>::erase() takes a std::vector<A>::iterator so no need to dereference.

Do this instead:

ClassAList.erase(inter);

Also, ClassAList.clear(); would do just as well.

[Disch]

There are a few problems:

1) No need to erase each element one at a time like that. vector's dtor will take care of that automatically. You don't have to worry about cleanup. Or if you want to do it explicitly, ClassAList.clear(); will erase all elements.

2) Calling erase() invalidates the iterator. So even if this code compiled, it would not work because 'inter' would become a bad iterator after the first time the loop is run. This is typically solved by using the return value of erase() as the new iterator.

3) Erasing a vector from the first element to the last is criminally inefficient, since the entire vector would need to be reallocated and moved for each item that is removed. Removing back to front is better (or, as stated earlier, just clear() or have the dtor do it for you automatically)

4) Erase takes an iterator. You're giving it an instance of your class, which is why you're getting a compiler error. Something like this is probably what you were trying to do:

ClassAList.erase(inter); // note: no *



If you want to manually remove each element in a vector, here are your options (listed best to worst)

1 (Best): Do nothing, let vector's dtor do it:

// no code necessary 

2: Clear the vector:

ClassAList.clear();

3: Spin on a loop that pop_back's elements until all elements are removed:

while(!ClassAList.empty())
  ClassAList.pop_back();

4 (worst): erase individual elements:

std::vector<A>::iterator i = ClassAList.begin();
while(ClassAList.empty())
  i = ClassAList.erase(i);

Note that we never do ++i because erase returns the new iterator to use.

[TotallyLearning]

Would a classes dtor, automatically call a vector's dtor?

Also, I knew clear would work perfectly, but it was just for a small attempt.
The pop back one, I never thought of, but also good.


Hmm, I cannot believe, just removing the * fixed it. I tried that earlier and it didn't work, but
now it does.

Thanks.

[Disch]

Would a classes dtor, automatically call a vector's dtor?

Yes. The whole point of dtors is that they're automatically called when the object is destroyed.

Hmm, I cannot believe, just removing the * fixed it. I tried that earlier and it didn't work, but
now it does.

Actually, that doesn't fix it.

The below is BAD CODE AND YOU SHOULD NOT DO IT EVER:

for(std::vector<A>::iterator inter = ClassAList.begin(); inter !=     ClassAList.end(); inter++)
    {
      ClassAList.erase(inter);  // BAD BAD BAD NEVER DO THIS!!!
    };

The reason this is bad is because the call to erase() invalidates 'inter'. Therefore when you do inter++, you not going to the next element in the vector, you're going to some other random area in memory (like a bad pointer).

While the above code may compile without error, running it will do very bad things and possibly will crash your program.

[TotallyLearning]

Indeed, you are right. My bad for not noticing previously, thanks.

I'm sorry, but now I must go to bed.

Also, I should of known the dtor would be automatically called. Since when destroying the class containing the vector, the vector would also be destroyed.

Its so late, I feel stupid for not realizing that.