iterators for vector of pointers

I read another post that answered a question regarding iterators for vectors of pointers. I tried to use the same concept in my code but I receive some compilation errors. The code sample I was basing my code on 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 members that are vectors of 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 MyCont开发者_Python百科ainer& 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 like:

/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*>]()'

candidates are: __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 the other vector.

---

You forgot to dereference the iterators. Try this instead:

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

Edit 0:

Yes, should be double dereference (fixed above). And it should be the const_terator instead since you are dealing with const containing object:

vector<MyStuff*>::const_iterator vec1_itr;

---

Either don't take the parameter as const or declare vec1_itr as a const_iterator. The issue here is that vecOne.begin() returns a const_iterator because the container is const. If you want to change the container you'll have to remove the const qualifier.

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

---

!! MEMORY LEAK ALERT !!

Your code, as is, is leaky.

Any exception thrown from within the copy constructor (std::bad_alloc ?) will cause a memory leak because the memory passed into the vector will never be cleaned-up (the destructor won't be called since the object was never constructed in the first place).

You could, of course, add the required try/catch, though I warn you that the code will soon get clunky (you need several).

This is a direct result of violating rule 1 of resources management:

An object should manage at most one resource, in which case it should not be doing anything else.

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

In your case, you have two solutions:

1. Recommended: since you do not use polymorphism here, don't use pointers. std::vector<MyStuff> is perfectly fine
2. If you need polymorphism, but didn't included it in this toy example, then use boost::ptr_vector<MyStuff>

Bonus Point: the two of them define sensible copy constructors, assignment operators and destructors, so that you won't have to rewrite them yourself.

EDIT:

As noted by @David, if you need polymorphism, you cannot use copy construction, and thus need:

• a clone method, or equivalent
• pointers and dynamic memory allocation

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