Does vector's copy constructor performs a deep copy?

[caut_baia]

Here's my curiosity

#include <vector>

class A {
protected:
   int somedata;
public:
   A () : somedata (1) {}
   A (const A& cpy) : somedata(cpy.somedata) {}

   virtual ~A () {}
 };

class B : public A  {
   A* someobj;
   std::vector <A*> vec;
public:
   typedef std::vector<A*>::iterator It;
   It i;

   B() : someobj(NULL) {}

   B (int x) : A(), someobj(new A()),vec (std::vector<A*>(x,new A())) ,i(vec.begin())   {}

   //B (const B& cpy) : A(cpy), someobj(new A(*cpy.someobj)), vec(cpy.vec) {} //will the default copy constructor do or must i do something like the following:

   B (const B& cpy) : A(cpy), someobj(new A(*cpy.someobj))  {
      std::vector<A*>::const_iterator o=cpy.vec.begin();
      for (i=vec.begin();i!=vec.end();++i,++o)  {
         (*i)=new A(**o);
       }
    }
       
   ~B ()  {
      for (i=vec.begin();i!=vec.end();++i)  {
         delete (*i);
       }
      delete someobj;
    }
 };

Thank you

[Narue]

No, std::vector does not perform a deep copy of pointer items. You must do it manually or use a smart pointer.

[caut_baia]

Ok so is the second implementation of the copy constructor ok?Or can you suggest a good solution?Thank you

[caut_baia]

Anybody ..

[arkoenig]

You haven't actually said what you want it to do, so it's hard to say whether it's ok.

Copying a vector copies its elements. However, if those elements are pointers, copying a pointer doesn't copy the object that the pointer points to. This is true whether or not the pointer is part of a vector, so it doesn't really have anything to do with vectors.

[mike_2000_17]

Yes, your implementation of the deep-copy constructor seems totally fine (as long as the objects a pointing to objects of class A and not of a derived class, like I said in a previous thread).

You probably want to also implement the assignment operator. operator = that is. Basically, it is almost the same implementation as your copy-constructor (in fact the idiom "copy-and-swap" or "copy-and-move"(C++0x) is a way to efficiently implement the assignment in terms of the copy-constructor. Or, you can just rewrite the same code again in the assignment operator.

[caut_baia]

So the asignment operator would look something like this ?

B&::B operator= (const B& cpy)  {
   delete someobj; //should i delete this before reasigning?
   someobj=new A(*cpy.someobj);
   std::vector<A*>::const_iterator o=cpy.vec.begin();
   for (std::vector<A*>::iterator i=vec.begin();i!=vec.end();++i,++o)  {
      (*i)=new A(**o);
    }
  }

I really can't get something working.I need to store a vector of objects in binary mode.Each of which contains a set of vectors.I have the copy constructors and assignment operators implemented for each class each of which is derived from a generic templated Window class.Almost like an interface.I can store objects of simple type which have no data allocated dynamically but for those that contain say a vector of pointers to objects heap allocated the program crashes.I'm making some tests using some simple classes which emulate the behavior of the more complex ones that i have.The real program is pretty big so i i'm only posting an example.

#ifndef STORAGE
#define STORAGE
#pragma once

#include <fstream>
#include <assert>
#include <cstdlib>
#include <string>

template <typename Type,template <typename> class Cont>
class Stash  {
   unsigned long Size;
   std::string Filename;
   std::fstream FileStream;
   std::ios::openmode Mode;
   enum {TYPE_SIZE=sizeof(Type)};
   Stash& operator= (const Stash&);
   Stash (const Stash&);

public:

   Stash () {}
   Stash (std::string Filename_,std::ios::openmode Mode_);
   ~Stash () {}

   bool OpenFile (std::string Filename_);
   void Close ();

   void StoreFrom (const Cont<Type>& Package_);
   void RetrieveIn (Cont<Type>& Package_);
 };
//---------------------------------------------------------------------------
template <typename Type,template <typename> class Cont>
Stash<Type,Cont>::Stash (std::string Filename_,std::ios::openmode Mode_)  :
   Filename(Filename_), Mode(Mode_)  {
 }
//---------------------------------------------------------------------------
template <typename Type,template <typename> class Cont>
bool Stash<Type,Cont>::OpenFile (std::string Filename_)  {
   FileStream.open (Filename_.c_str(),Mode);
   return FileStream.is_open();
 }
//---------------------------------------------------------------------------
template <typename Type,template <typename> class Cont>
void Stash<Type,Cont>::Close ()  {
   if (FileStream.is_open) FileStream.close();
 }
//---------------------------------------------------------------------------
template <typename Type,template <typename> class Cont>
void Stash<Type,Cont>::StoreFrom (const Cont<Type>& Package_)  {
   assert (OpenFile(Filename));
   if (FileStream.is_open())  {
      FileStream.seekp(std::ios::beg);
      for (Cont<Type>::const_iterator i=Package_.begin();i!=Package_.end();++i)  {
         assert (FileStream.write((unsigned char*) &(*i),TYPE_SIZE));
       }
    }
 }
//---------------------------------------------------------------------------
template <typename Type,template <typename> class Cont>
void Stash<Type,Cont>::RetrieveIn (Cont<Type>& Package_)  { 
   if (!FileStream.is_open())  {
      assert (OpenFile(Filename)); 
    }
   Type Obj;
   
   FileStream.seekg (0,std::ios::end);
   unsigned long size=FileStream.tellg();
                               
   FileStream.seekg(std::ios::beg);
   
   while (size)  {      
      assert (FileStream.read((unsigned char*) &(Obj),TYPE_SIZE));
      Package_.push_back (Type(Obj));
      size-=TYPE_SIZE;
    }   
 }

#endif

#include <iostream>
#include <vector>
#include <algorithm>
#include "stash.h"

class A  {
   int n;   
public:
   A () : n(0) {}
   A (int x) : n(x) {}
   A (const A& cpy) : n(cpy.n) {}
      
   ~A () { }
   friend std::ostream& operator<<(std::ostream& o,const A& a)  {
      return o << a.n;
    }
 };

class B  {
   std::vector<A*> vec;
   A* obj;
public:
   B () : obj(new A) {}
   B (int x) : vec(std::vector<A*> (x,new A(x))), obj(new A())  {}      

   B (const B& cpy) : vec(cpy.vec), obj(new A(*cpy.obj))  {
      std::vector<A*>::const_iterator o=cpy.vec.begin();      
      for (std::vector<A*>::iterator i=vec.begin();i!=vec.end();++i,++o)  {
         (*i)=new A(**o);
       }
    }

   B& operator= (const B& cpy)  {
      delete obj;
      obj=new A(*cpy.obj);
      vec=cpy.vec;
      std::vector<A*>::const_iterator o=cpy.vec.begin();      
      for (std::vector<A*>::iterator i=vec.begin();i!=vec.end();++i,++o)  {
         (*i)=new A(**o);
       }
      return *this;
    }  

   ~B ()  {
      for (std::vector<A*>::iterator i=vec.begin();i!=vec.end();++i)  {
         delete (*i);
       }
      delete obj;
    }
  
   friend std::ostream& operator<<(std::ostream& o,const B& b)  {
      return o << b.vec.size();
    }
 };

int main ()  {
   std::vector <B*> vec(5,new B(5));
   std::vector <B> temp;
   
   std::ios::openmode mode=std::ios::in|std::ios::out|std::ios::binary;

   Stash <B,std::vector> store ("qwe.txt",mode);   
   for (std::vector <B*>::iterator i=vec.begin();i!=vec.end();++i)  {
      temp.push_back (**i);
    }
   store.StoreFrom (temp);
   std::vector <B> t;
   store.RetrieveIn (t); /crash
   copy (temp.begin(),temp.end(),std::ostream_iterator<B>(std::cout," "));
 }

I'm definitely missing something..

[mike_2000_17]

Your problem is a nice situation in which the copy-and-swap idiom is very applicable:

class B  {
   std::vector<A*> vec;
   A* obj;
public:
   B () : obj(new A) {}
   B (int x) : vec(std::vector<A*> (x,new A(x))), obj(new A())  {}      

   B (const B& cpy) : vec(cpy.vec.size()), obj(new A(*cpy.obj)) { //notice the cpy.vec.size() here. no need to copy the vector, just construct a vector of the same size.
      std::vector<A*>::const_iterator o=cpy.vec.begin();      
      for (std::vector<A*>::iterator i=vec.begin();i!=vec.end();++i,++o)  {
         (*i)=new A(**o);
       }
    }
    
    B& swap(B& rhs) { //define a swap function, in terms of std::swap functions.
      std::swap(n,rhs.n);
      vec.swap(rhs.vec);
      return *this;
    };
   
    B& operator= (const B& cpy) {
      B temp(cpy); //copy the cpy object into temp
      return swap(temp); //swap this and temp. And temp will automatically be deleted.
    }  

   ~B ()  {
      for (std::vector<A*>::iterator i=vec.begin();i!=vec.end();++i)  {
         delete (*i);
       }
      delete obj;
    }
  
   friend std::ostream& operator<<(std::ostream& o,const B& b)  {
      return o << b.vec.size();
    }
 };

PS: It would really really make your life easier if you use smart pointers instead.

[caut_baia]

The swap thing really looks great.Though it still crashes.If i don't delete anything inside the destructor's body though it exits fine.Maybe i'm deleting something twice but i really have a hard time into figuring this out for like two days or so.About smart pointers i know they're really efficient and can save you of a lot of trouble but i'm really kicking my ass into learning this the hard way.Thanks a lot for your time.Your suggestions are very much appreciated as always.

[caut_baia]

ok i replaced every vector with a deque and changed the destructor to this:

~B ()  {
   for (int i=0;i<vec.size();i++)  {
      delete vec[x];
      vec.pop_front();
    }
   delete obj;
 }

Kinda works .. So you need to also pop the object from the vector.I thought exiting the scope and calling vector's destructor should take care of that..

[mike_2000_17]

This won't work:

~B ()  {
   for (int i=0;i<vec.size();i++)  { //size() is changing so this will not iterate through all elements.
      delete vec[x]; //what is x?
      vec.pop_front(); //why do you need to pop anything?
    }
   delete obj;
 }

I think this should work:

~B ()  {
  for (int i=0;i<vec.size();i++) 
     delete vec[i];
  delete obj;
}

If it is still crashing, it is not because of the above destructor. It is something else.

[caut_baia]

You are absolutely right.. i haven't thought about vec.size() constantly changing.I just quickly wrote that so i must have put an x instead of i.Anyway .. i said it worked .. but i doesn't.I bet there is a small error somewhere but i wanted to make sure it isn't from the copy constructors or destructors since i changed the assignment operator's implementation the way you suggested so it goes out of the quiestion.I'll try to figure this is out but i'm completely baffled ..

[mike_2000_17]

You might want to use a debugger to trace the error. I don't know what IDE you are using but most of them are equiped with pretty good ones. Also, valgrind can help to detect and back-trace memory-related crashes.

[caut_baia]

Actually i'm compiling at the command line using borland 5.5 .. edit.exe editor.I do have visual studio 5 installed but shamefully i have never used a debugger before.I'll try anyway.Thanks