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

Edited 5 Years Ago by caut_baia: n/a

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

Edited 5 Years Ago by Narue: n/a

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

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.

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.

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..

Edited 5 Years Ago by caut_baia: n/a

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.

Edited 5 Years Ago by mike_2000_17: n/a

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.

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..

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.

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 ..

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.

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

This article has been dead for over six months. Start a new discussion instead.