Iterators for pointer vector

Question

I read another post that answered the question about iterators for pointer vectors. I tried using the same concept in my code, but getting some compilation errors. The sample code on which I based my code is:

vector<c*> cvect; cvect.push_back(new sc); vector<c*>::iterator citer; for(citer=cvect.begin(); citer != cvect.end(); citer++) { (*citer)->func(); } 

I want to use a similar concept to create a deep copy constructor for a class that has two data elements that are vector pointers to objects. My code is similar to this:

 class MyContainer { vector<MyStuff*> vecOne; vector<MyStuff*> vecTwo; public: MyContainer(const MyContainer& other); }; MyContainer::MyContainer(const MyContainer& other) { // copy vector one vector<MyStuff*>::iterator vec1_itr; for (vec1_itr = other.vecOne.begin(); vec1_itr != other.vecOne.end(); vec1_itr++) { vecOne.push_back(new MyStuff(vec1_itr)); } // copy vector two vector<MyStuff*>::iterator vec2_itr; for (vec2_itr = other.vecTwo.begin(); vec2_itr != other.vecTwo.end(); vec2_itr++) { vecTwo.push_back(new MyStuff(vec2_itr)); } } 

I get some compilation errors, such as:

/path/MyContainer.cpp:38: error: no match for ' operator= ' in ' vec1_Itr = other->MyContainer::vecOne. std::vector<_Tp, _Alloc>::begin [with _Tp = MyStuff*, _Alloc = std::allocator<MyStuff*>]() vec1_Itr = other->MyContainer::vecOne. std::vector<_Tp, _Alloc>::begin [with _Tp = MyStuff*, _Alloc = std::allocator<MyStuff*>]() '

candidates: __gnu_cxx::__normal_iterator<MyStuff*, std::vector<MyStuff, std::allocator<MyStuff> > >& __gnu_cxx::__normal_iterator<MyStuff*, std::vector<MyStuff, std::allocator<MyStuff> > >::operator=(const __gnu_cxx::__normal_iterator<MyStuff*, std::vector<MyStuff, std::allocator<MyStuff> > >&)

I also get an error for operator!= ... And another set of the same errors for another vector.

Answer 1

You forgot to dereference iterators. Try instead:

 vecOne.push_back(new MyStuff( **vec1_itr )); 

Change 0:

Yes, there must be double dereferencing (fixed above). And that should be const_terator , because you are dealing with a const containing an object:

 vector<MyStuff*>::const_iterator vec1_itr; 

Answer 2

Either do not accept the parameter as const , or declare vec1_itr as const_iterator . The problem here is that vecOne.begin() returns a const_iterator , because the container is const . If you want to change the container, you need to remove the const qualifier.

On the side of the note, if holding a container of pointers means you need to manage pointers in the container, and you have two such containers, then you should move the container to its own class. Try to avoid managing multiple resources in a class.

Answer 3

!! MEMORY LEAK ALERT !!

Your code, as it is, is leaking.

Any exception thrown inside the copy constructor ( std::bad_alloc ?) std::bad_alloc cause a memory leak since the memory passed in vector will never be cleared (the destructor will not be called since the object was never built in the first place).

You could, of course, add the required try/catch , although I warn you that the code will become awkward soon (you need a few).

This is a direct result of a violation of resource management rule 1 :

An object must manage a resource of not more than one , in which case it should not do anything else.

This means that if your object is a business object (with internal application logic), it should not directly access resource management, but instead use existing managers.

In your case, you have two solutions:

• Recommended: since you are not using polymorphism here, do not use pointers. std::vector<MyStuff> great
• If you need polymorphism, but he did not include it in this toy example, use boost::ptr_vector<MyStuff>

Bonus Point: Two of these are defined by reasonable copy constructors, assignment operators, and destructors, so you don't have to rewrite them yourself.

EDIT:

As @David noted, if you need polymorphism, you cannot use the copy construct and therefore need to:

• a clone or equivalent
• pointers and dynamic memory allocation

boost::ptr_vector provide everything you need for this (with automatic use of the clone method when copying).