Copy of class object being changed when original is changed?

Question

I seem to be having the issue of value passed by reference.  I'm a newb and learning.  From all my reading, what I have done should be working.

First the code:

Class FOO
{
private string _field;
private savedFoo;
public string Field
{
get{ return _field;
}
set{ _field = value;
}
public void BeginEdit
{
if (savedFoo == null)
{
savedFoo = new FOO();
savedFoo._field = this.Field;
}
void Main()
{
FOO myFoo = new Foo();
myFoo.BeginEdit();
myFoo.Field = "Something";
}

The issue is when I change myFoo.Field, the savedFoo._field also changes.  I want to stop this from happening.  From what I understand in my research is this is the prefered way to deep clone.  Am I right or just not understanding things correctly.  If someone could show me the right way (if I'm wrong), and an explanation would also be nice as I'm learning.

---

Always Learning new Things

Answer 1

• As it is your code shouldn't even compile (you don't have a type listed for "savedFoo", and you're missing some brakets).
• Please try to have your code indented properly; it's much harder to read without any indentation.
• How are you comparing the value of "myFoo" in "Main" with the value of "savedFoo"?  What's your basis for assuming they're not different?

Answer 2

What you're doing here seems to work fine for me. You are deep copying the foo object when you assign.

Here was the code I used:

public class FOO {
	private string _field;
	private FOO savedFoo;
	public string Field {
		get { return _field; }
		set { _field = value; }
	}
	public void BeginEdit() {
		if(savedFoo == null) {
			savedFoo = new FOO();
			savedFoo.Field = this.Field;
		}
	}
	public FOO GetSavedFoo {
		get { return savedFoo; }
	}
}
 
public void TestFOO() {
	FOO myFoo = new FOO();
	myFoo.Field = "Initial Value";
	myFoo.BeginEdit();
	Console.WriteLine("myFoo:    " + myFoo.Field);
	Console.WriteLine("savedFoo: " + myFoo.GetSavedFoo.Field);
	myFoo.Field = "New Value";
	Console.WriteLine("myFoo:    " + myFoo.Field);
	Console.WriteLine("savedFoo: " + myFoo.GetSavedFoo.Field);
 
}

The program's output was:

myFoo:  Initial Value

savedFoo: Initial Value

myFoo: New Value

savedFoo: Initial Value

======

The reason this works is because strings are assigned by value, just like other primitive types (like int, double, and struct types, like datetime). Every time you do = with a string, you get a new copy of the string.

Now, if you were using something more complex instead of string (Like another class) for the type of Field, what you're doing here would look very different. Lets look at a modification:

public class FOO {
	private Koda _field;
	private FOO savedFoo;
	public Koda Field {
		get { return _field; }
		set { _field = value; }
	}
	public void BeginEdit() {
		if(savedFoo == null) {
			savedFoo = new FOO();
			savedFoo.Field = this.Field;
		}
	}
	public FOO GetSavedFoo {
		get { return savedFoo; }
	}
}
 
public class Koda {
	public string Thing { get; set; }
}
 
public void TestFOO() {
	FOO myFoo = new FOO();
	Koda k = new Koda();
	k.Thing = "Initial Value";
	myFoo.Field = k;
	myFoo.BeginEdit();
	Console.WriteLine("myFoo:    " + myFoo.Field.Thing);
	Console.WriteLine("savedFoo: " + myFoo.GetSavedFoo.Field.Thing);
	k.Thing = "New Value";
	Console.WriteLine("myFoo:    " + myFoo.Field.Thing);
	Console.WriteLine("savedFoo: " + myFoo.GetSavedFoo.Field.Thing);
 
}

The output of this program is much different:

myFoo:  Initial Value

savedFoo: Initial Value

myFoo: New Value

savedFoo: New Value

The reason for this is because the Koda object is being passed/assigned by reference. Both copies of the FOO object are sharing the same Koda object, not copies. So they return the same values.

Answer 3

Sorry for the bad example.

---

Always Learning new Things

Answer 4

Then using a string was a bad example.

Here is my actual class code that I have at the moment.

public class PersonObject : BaseObject
    {
        // Struct used to transfer state to data manager
        public struct POObject
        {
            public int Id;
            public int PersonTypeId;
            public string FirstName;
            public string AltFirstName;
            public string LastName;
            public string Suffix;
            public DateTime BirthDate;
            public bool EmailPromotions;
            public Collection<PhNbrObject> PhoneNumbers;
            public Collection<EmailObject> EmailAddresses;
        }

        private PersonObject savedObject;

        // statuses
        private bool isEditing;
        private bool isDirty;
        private bool isNew;
        private bool isDeleted;

        // members
        private int _id;
        private int _typeId;
        private string _firstname;
        private string _altfirstname;
        private string _lastname;
        private string _suffix;
        private DateTime _birthdate;
        private bool _emailpromo;
        private Collection<PhNbrObject> _phoneNumbers;
        private Collection<EmailObject> _emailaddress;

        // copy of class to hold state before changes
        public PersonObject()
        {
            isNew = true;
            isDirty = false;
            isEditing = false;
            isDeleted = false;

            _phoneNumbers = new Collection<PhNbrObject>();
            _emailaddress = new Collection<EmailObject>();

            savedObject = new PersonObject();

            RuleBroken("Person Type Id not set", true);
            RuleBroken("First Name not set", true);
            RuleBroken("Last Name not set", true);
            RuleBroken("Email Promo not set", true);
        }

        #region Public Interface

            public bool IsNew
            {
                get
                {
                    return isNew;
                }
            }

            public bool IsEditing
            {
                get
                {
                    return isEditing;
                }
            }

            public bool IsDirty
            {
                get
                {
                    return isDirty;
                }
            }

            public bool IsDeleted
            {
                get
                {
                    return isDeleted;
                }
            }

        
            public int Id
            {
                get
                {
                    return _id;
                }
            }

            public int TypeId
            {
                get
                {
                    return _typeId;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _typeId = value;
                    RuleBroken("Person Type Id not set", _typeId <= 0);
                    isDirty = true;
                }
            }

            public string FirstName
            {
                get
                {
                    return _firstname;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _firstname = value;
                    RuleBroken("First Name not set", string.IsNullOrEmpty(_firstname));
                    isDirty = true;
                }
            }

            public string AltFirstName
            {
                get
                {
                    return _altfirstname;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _altfirstname = value;
                    isDirty = true;
                }
            }

            public string LastName
            {
                get
                {
                    return _lastname;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _lastname = value;
                    RuleBroken("Last Name not set", string.IsNullOrEmpty(LastName));
                    isDirty = true;
                }
            }

            public string Suffix
            {
                get
                {
                    return _suffix;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _suffix = value;
                    isDirty = true;
                }
            }

            public DateTime birthDate
            {
                get
                {
                    return _birthdate;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _birthdate = value;
                    isDirty = true;
                }
            }

            public bool EmailPromo
            {
                get
                {
                    return _emailpromo;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _emailpromo = value;
                    RuleBroken("Email Promo not set", string.IsNullOrEmpty(_emailpromo.ToString()));
                    isDirty = true;
                }
            }

            public Collection<PhNbrObject> PhoneNumbers
            {
                get
                {
                    return _phoneNumbers;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _phoneNumbers = value;
                    isDirty = true;
                }
            }

            public Collection<EmailObject> EmailAddress
            {
                get
                {
                    return _emailaddress;
                }
                set
                {
                    if (!isEditing)
                        throw new InvalidOperationException("Property can only be set while editing.");

                    _emailaddress = value;
                    isDirty = true;
                }
            }


            public void BeginEdit()
            {
                if (savedObject == null)
                    savedObject = new PersonObject();

                SaveState();

                foreach (PhNbrObject pno in PhoneNumbers)
                {
                    pno.BeginEdit();
                }

                foreach (EmailObject eo in EmailAddress)
                {
                    eo.BeginEdit();
                }

                isEditing = true;

            }

            /// <summary>
            /// Saves or deletes object if appropriate.
            /// Clients should immediately call BeginEdit
            /// if they wish to continue editing.
            /// </summary>            
            public void ApplyEdit()
            {
                if (isDeleted && !isNew)
                {
                    Delete(Id);
                    isNew = true;
                    isDeleted = false;
                }
                else
                {
                    if (isDirty || isNew)
                    {
                        Save();
                        isNew = false;
                    }
                }

                foreach (PhNbrObject pno in PhoneNumbers)
                {
                    pno.ApplyEdit();
                }

                foreach (EmailObject eo in EmailAddress)
                {
                    eo.ApplyEdit();
                }

                isEditing = false;
                isDirty = false;
            }

            /// <summary>
            /// Cancels all changes since the last ApplyEdit
            /// or since the object was marked for deletion.
            /// Terminates editing.
            /// </summary>
            public void CancelEdit()
            {
                isDirty = false;
                isDeleted = false;
                isEditing = false;

                RetrieveState();

                foreach (PhNbrObject pno in PhoneNumbers)
                {
                    pno.CancelEdit();
                }

                foreach (EmailObject eo in EmailAddress)
                {
                    eo.CancelEdit();
                }
            }

            /// <summary>
            /// Gets the current state of object
            /// </summary>
            /// <returns></returns>
            public POObject GetState()
            {
                POObject poo = new POObject();

                poo.Id = this.Id;
                poo.PersonTypeId = this.TypeId;
                poo.FirstName = this.FirstName;
                poo.AltFirstName = this.AltFirstName;
                poo.LastName = this.LastName;
                poo.Suffix = this.Suffix;
                poo.BirthDate = this.birthDate;
                poo.EmailPromotions = this.EmailPromo;
                poo.PhoneNumbers = this.PhoneNumbers;
                poo.EmailAddresses = this.EmailAddress;

                return poo;
            }

            /// <summary>
            /// Loads a Person from the database. 
            /// </summary>
            /// <param name="id"></param>
            public void Load(int id)
            {
                PersonObjectManager pom = new PersonObjectManager();
                POObject poo = new POObject();

                try
                {
                    poo = pom.Load(id);

                    foreach (PhNbrObject pno in poo.PhoneNumbers)
                    {
                        pno.BeginEdit();
                    }

                }
                catch (Exception ex)
                {
                    PublicFunctions.DisplayError(ex);
                }

                this.SetState(poo);                  
                isNew = false;
            }

            /// <summary>
            /// Marks person for deletion.
            ///   Can be undone by cancelling edit.
            /// </summary>
            public void Delete()
            {
                isDeleted = true;
                isDirty = true;
            }            

        #endregion

        #region Implementation

            /// <summary>
            /// Sets the current state of instance
            /// </summary>
            /// <param name="poObject"></param>
            private void SetState(POObject poObject)
            {
                RuleBroken("Id not set", poObject.Id <= 0);
                RuleBroken("Person Type Id not set", poObject.PersonTypeId <= 0);
                RuleBroken("First Name not set", string.IsNullOrEmpty(poObject.FirstName));
                RuleBroken("Last Name not set", string.IsNullOrEmpty(poObject.LastName));
                RuleBroken("Email Promo not set", string.IsNullOrEmpty(poObject.EmailPromotions.ToString()));

                try
                {
                    this._id = poObject.Id;
                    this._typeId = poObject.PersonTypeId;
                    this._firstname = poObject.FirstName;
                    this._altfirstname = poObject.AltFirstName;
                    this._lastname = poObject.LastName;
                    this._suffix = poObject.Suffix;
                    this._birthdate = poObject.BirthDate;
                    this._emailpromo = poObject.EmailPromotions;
                    this._phoneNumbers = poObject.PhoneNumbers;
                    this._emailaddress = poObject.EmailAddresses;
                }
                catch (Exception ex)
                {
                    PublicFunctions.DisplayError(ex);
                }
            }

            /// <summary>
            /// Returns the state of the saved object
            /// </summary>
            private void RetrieveState()
            {
                this._id = savedObject.Id;
                this._typeId = savedObject.TypeId;
                this._firstname = savedObject.FirstName;
                this._altfirstname = savedObject.AltFirstName;
                this._lastname = savedObject.LastName;
                this._suffix = savedObject.Suffix;
                this._birthdate = savedObject.birthDate;
                this._emailpromo = savedObject.EmailPromo;
                this._phoneNumbers = savedObject.PhoneNumbers;
                this._emailaddress = savedObject.EmailAddress;
            }

            /// <summary>
            /// Saves current state of object
            /// </summary>
            private void SaveState()
            {
                savedObject._id = this._id;
                savedObject._typeId = this._typeId;
                savedObject._firstname = this._firstname;
                savedObject._altfirstname = this._altfirstname;
                savedObject._lastname = this._lastname;
                savedObject._suffix = this._suffix;
                savedObject._birthdate = this._birthdate;
                savedObject._emailpromo = this._emailpromo;
                savedObject._phoneNumbers = this._phoneNumbers;
                savedObject._emailaddress = this._emailaddress;
            }

            /// <summary>
            /// Sends the data to the database
            /// </summary>
            private void Save()
            {
                PersonObjectManager pom = new PersonObjectManager();

                try
                {
                    int i = pom.Save(this);

                    if (i > 0)
                        _id = i;
                }
                catch (Exception ex)
                {
                    PublicFunctions.DisplayError(ex);
                }
            }

            /// <summary>
            /// Deletes object from database
            /// </summary>
            /// <param name="id"></param>
            private void Delete(int id)
            {
                PersonObjectManager pom = new PersonObjectManager();

                try
                {
                    foreach (PhNbrObject pno in savedObject.PhoneNumbers)
                    {
                        pno.Delete();
                        pno.ApplyEdit();
                    }

                    pom.Delete(this.Id);
                }
                catch (Exception ex)
                {
                    PublicFunctions.DisplayError(ex);
                }
            }

        #endregion
    }

I first noticed the problem with the phone number collection.

---

Always Learning new Things

Answer 5

@barrakoda

"The reason this works is because strings are assigned by value, just like other primitive types (like int, double, and struct types, like datetime). Every time you do = with a string, you get a new copy of the string."

This is flat wrong.  Strings are not assigned by value, they are reference types.  A variable of type "string" is merely a reference to a string object that exists elsewhere, not the actual string itself.  (That would be impossible, because a value type must always be of a fixed size, and a string isn't of a fixed size.)  When you assign one string to another you do *not* get a new copy.  Both strings are references that are pointing to the same string object that is in memory somewhere.

Strings are, however, immutable.  Once you create a string you can never change it.  This is a trait that is common among value types, and having mutable value types is considered a bad idea, but it is possible and it does happen.

It's worth noting that because strings are immutable it is irrelevant to you whether two string variables point to the same string or to two different strings that have the same value.  Since the string can't change there is no way to ever observe any side effects of the fact that there is a shared object.

(Note that using unsafe code you can in fact modify the contents of a string to demonstrate these statements.  You should never actually do that in a real program though, that would be very mean.)

Answer 6

@Nightman

That's really a lot of code to sort through, and that makes the problem somewhat harder to demonstrate or explain.

What you are seeing is that a copy operation isn't just "shallow" or "deep".  In fact, you have shades of grey with "completely shallow" on on end, "perfectly deep" on the other, and lots of space inbetween.  You created a copy that is in that half way space, when you thought that you had one that was "perfectly deep".

Cloning isn't a trivial operation.  That's why you won't see any library methods that just clone any arbitrary object for you with an option for "shallow" or "deep".

What is happening is that you have an object that is a reference type, and it has references to other objects, and that has references to other objects.  You created a new object at the top level and copied all of the parameters from the old object to the new one, but you copied all of those parameters through a shallow copy, not a deep copy.

Even if you had used a deep copy to copy all of the members of your top level object, in particular the collection of phone numbers, that object would need to do a deep copy of all of the items it references, rather than a shallow copy.

That's the problem with just saying "shallow" or "deep"; the real question is not *if* the copy is shallow or deep, the question is "how deep does the copy go".  You may want a copy that goes to the deepest level possible, you may want one at the shallowest level possible, or you may *want* one that falls in the "inbetween" space.

Answer 7

@servy42

I'm just keepin' it simple man, relax. The guy's new, doesn't need to be super complex.

Answer 8

@severy42

Thank you for the explanation, that helped clarify shallow and deep cloning in a better light.

With the understand that I do have and the reading of other post. It was my understanding that by instantiating the saved object as a new object I would be able to make changes to my original object and the saved object would only be effected when I called my SaveState method and changed the properties that way.

Is there a way to get the savedObject seperated and not linked by a reference?

---

Always Learning new Things

Answer 9

@servy42

I'm just keepin' it simple man, relax. The guy's new, doesn't need to be super complex.

Simple isn't an excuse to mean "wrong".  There isn't even a grain of truth in what you said.  It is a frequently mis-understood topic and repeating those blatantly false misconceptions will only harm a new programmers understanding of the topic, not help it.

Answer 10

@severy42

Thank you for the explanation, that helped clarify shallow and deep cloning in a better light.

With the understand that I do have and the reading of other post. It was my understanding that by instantiating the saved object as a new object I would be able to make changes to my original object and the saved object would only be effected when I called my SaveState method and changed the properties that way.

Is there a way to get the savedObject seperated and not linked by a reference?

You are creating a new object, and you are copying all of the fields, not just referencing the old one.  That's not the problem.  The problem is that for reference types (i.e. classes, which is most objects in .NET) the variable is not the object itself.  The collection "_phoneNumbers" is not an actual collection.  The content of that field are simply a reference to where the actual collection exists (somewhere, out there).  When you just assign "_phoneNumbers" from the old type to the new type you're just copying the value of that reference.  You need to make a brand new collection and then copy all of the items in that collection over.  However the contents of those collection aren't actual instances of "PhNbrObject".  They're just references to where "PhNbrObject"s exist somewhere else.  You need to create new instances of "PhNbrObject" for each item in the collection.  If a "PhNbrObject" has any instance fields that are reference types (unless they're immutable) then those also need to be made using a deep copy, rather than a shallow copy, and so on (to infinity) until every field that needs to be copied is either a value type, is immutable, or you can trust won't be mutated (unless it's desirable for the instance to be shared).

The same thing needs to happen with the collection of emails, all the way down to the lowest level.

This is why deep copies are a pain.  Oftentimes it's desirable to either utilize immutable objects, or to work in situations where shared instances are not a problem.  I don't really have the time to digest the entire purpose of the application and re-design it for you though.

Answer 11

@severy42

I don't really have the time to digest the entire purpose of the application and re-design it for you though.

I'm not asking for that, I don't learn that way. 

---

Always Learning new Things

Answer 12

"I'm not asking for that, I don't learn that way. "

I know you didn't; I didn't mean to imply you did.  I meant that I couldn't even go over a re-design at a high level; without code.

Answer 13

So what your saying is that what I'm after is possible, just difficult to achieve and maintain.  Even with something as simple as my attempt at a short example with the FOO class.  

That would explain the seemingly lack of good examples existing on how to achieve this task, it might be time to switch to another language for this.

---

Always Learning new Things

Answer 14

So what your saying is that what I'm after is possible, just difficult to achieve and maintain.  Even with something as simple as my attempt at a short example with the FOO class.  

That would explain the seemingly lack of good examples existing on how to achieve this task, it might be time to switch to another language for this.

Making a perfectly deep copy of non-trivial objects is indeed difficult to achieve and maintain.

The solution is more likely that you just shouldn't be making a deep copy.  You should see if there is a way of solving your problem without needing to make an entirely new copy of everything.

Another option would be to rely more on immutable types, because as I have discussed you can make shallow copies of immutable types instead of deep copies knowing that it's just fine.  This (usually) isn't that bad if you've designed your programs with that in mind from the start.  Part of your problem is that it seems (from me just skimming the code) that this is a rather central aspect of your program, so altering it might not be trivial.

If you could provide a shorter example, or just describe in a paragraph or two what's going on I may be able to help you design a better approach.

Answer 15

My issue with immutable types is once they are created in the constructor you can't change their values. I would like the values in the saved object to change only when the set state method is called.

I'm not sure how to describe what's going on other than I'm trying to learn c# and creating a multi-layer application.  This class is my data object layer.  My front end form creates an instance of this class and provides a nice UI to alter the fields of the class.  There is a data layer that this class calls upon to save and retrieve the data from a database.  The full intent of the application is to create a general customer database application.

---

Always Learning new Things