class question

I need to create a clone of a class (the same class as my earlier vector topic)

the call looks like this:

Code:

clone_test=List_Num_1.clone();

in my class the clone() is the following:

Code:

  CNameValueList CNameValueList::clone()
  {
    CNameValueList temp;
    string strValue;
    string strName;
    for (MYVECTORITER the_iterator=list.begin(); the_iterator != list.end(); the_iterator++)
    {
      strValue=(*the_iterator)->getValue();
      strName=(*the_iterator)->getName();
      temp.add(strName,strValue);
    }
    return temp;
  }

Upon returning "temp" it crashes...
Can anyone give me a hint as to why this code doesn't work?

I would suggest that it is because temp does not exist outside of the scope of the member function clone()

You should be allocating memory for the class and returning a pointer to the object.

>CNameValueList CNameValueList::clone()

If you return an object, not a pointer, the program will copy your object from temp to the object you asign the result to. If you don't have a copy constructor or operator =, this will probably fail because it will copy the class bitwise. Use a pointer as stated above.

Code:

CNameValueList CNameValueList::clone()
{
  CNameValueList* temp=new CNameValueList;
  string strValue;
  string strName;

  for (NVPLISTITER the_iterator=list.begin(); the_iterator != list.end(); the_iterator++)
  {
    strValue=(*the_iterator)->getValue();
    strName=(*the_iterator)->getName();
    temp->add(strName,strValue);
  }
  return *temp;
}

This is what I currently have. It copies fine, but 2 errors occur.

1: If I add a new value to my cloned class, it overwrites the the other data.

2: Upon completion of the program it crashes.

thanks (still churning away at it)

Your crash problem stems from what I was telling you in the other thread....if you have a vector of pointers, you must take special care!.....if you copy the vector to another, then the new vector recieves copies of the pointers...therefore, to pointers exist for each object on the heap....and if 1 vector calls delete on the object before going out of scope, then the second vector now refers to data that could cause a crash if its dereferenced or deleted!!!!

To get around this, your clone function must implement a deep copy to create more objects on the heap as opposed to copying references to single objects.....

Also, if you are using a clone function, you might want to keep an eye out for the copy constructor and operator=()...they must either do a deep copy too, or be made private and unusable...

Here is the code from the earlier thread.......I altered it a little to show how you can implment the Clone function

Code:

#include <iostream>
#include <string>
#include <vector>
#include <utility>//for std::pair
using namespace std;


class foobar{
	typedef vector<pair<string, string>* > MYVECTOR;
	typedef MYVECTOR::iterator MYVECTORITER;
	
	foobar(const foobar&);//Make private...rely on foobar::Clone
	const foobar& operator=(const foobar&);//Make private...rely on foobar::Clone
	
	MYVECTOR vec;
public:

	foobar(){}//Need this as made copy constructor private

	void AddPair(const string& lhs,const string& rhs){
		pair<string,string>* temp = new pair<string,string>(lhs,rhs);	
		vec.push_back(temp);
	    ToScreen();
	 }
   
    ~foobar(){//destructor
    	
			for(int i = 0;i < vec.size();++i)
    			delete vec[i];//delete each
    	 

	}
	
	void ToScreen(){//Add a print function
	
		MYVECTORITER the_iterator;
		for (the_iterator = vec.begin();
			the_iterator != vec.end();
			++the_iterator){
	        cout << (*the_iterator)->first << ' ';
	        cout << (*the_iterator)->second << '\n';
	    }
	}
	
	void Clone(const foobar& rhs){
	
		
			for(int i = 0;i < vec.size();++i)
    			delete vec[i];//delete each
		vec.clear();//empty current vector
				
		for (int i = 0;i < rhs.vec.size();++i){//Deep copy
	        pair<string,string>* temp = new pair<string,string>(*rhs.vec[i]);
			vec.push_back(temp);
	    }	
	}

};

int main(){

	foobar f;
	
	f.AddPair("Hello","World");
	
	cout << endl;
	
	f.AddPair("Eat at","Joes");
	
	cout << endl;
	
	f.AddPair("foo","bar");
	
	cout << endl << "From g (cloned from f)" << endl;
	
	foobar g;//make a new foobar
	
	g.Clone(f);//copy f members to g
	
	g.ToScreen();//print out all of g
	
	cout << endl << "From h (cloned from g)" << endl;
	
	foobar h;
	
	h.Clone(g);//Yet another copy
	
	h.ToScreen();


}

I understand about the having to create a new "hard" copy of the list and each value pair but I guess i'm confusing myself.

I need to have it so that the following can happen:
a=b.clone();

So inside my function clone() i need to:
1) create a new CNameValueList object.
2) copy all values from the current object into the new object.
3) return the new object.


so in my CNameValueList::Add(str1,str2) I have :

Code:

CNameValue* temp2=new CNameValue(strName,strValue);
list.push_back(temp2);

and in my CNameValueList::clone() I have :

Code:

CNameValueList CNameValueList::clone()
{
  CNameValueList* temp2=new CNameValueList;
  string strValue;
  string strName;

  for (NVPLISTITER the_iterator=list.begin(); the_iterator != list.end(); the_iterator++)
  {
    strValue=(*the_iterator)->getValue();
    strName=(*the_iterator)->getName();
    temp2->add(strName,strValue);
  }
  return *temp2;
}

Questions:
So have I done the right thing with the function I have created?
The way I see it (which must be missing something cuz it has an error) is that:
1- It creates a new CNameValueList
2- It gets the values from the old list.
3- It pushes(creates) new CNameValue objects onto the new list
4- It returns the newly created list

Is that what you mean?