0

I'm trying to create a copy of an object using its constructor but when I modify something of the copy, the original object is modified too. I'll be grateful if you can help me, this is my code:

public class XMLStructure
{
 public XMLStructure(XMLStructure xmlCopy )
  {
     this.Action = xmlCopy.Action;
     this.Name = xmlCopy.Name;
  }
  public String Name { get; set; }
  public ActionXML Action { get; set; }
}
camilo93.cc
  • 147
  • 1
  • 2
  • 8
  • Well the string is indeed copied, but the `ActionXML` will still reference the original – Ofir Winegarten Jul 21 '17 at 14:58
  • The string is immutable. So you are effectivelly "kind of" copying its value. But ActionXml is a reference type. You are just copying the pointer to the value. You need to do a `deep copy` of your object. – fharreau Jul 21 '17 at 15:02
  • @OfirWinegarten: The string isn't copied. Strings are reference types just the same. But strings are immutable, making the matter of "when it's modified" moot -- "changing a string" means creating a brand new string and assigning that. Changing an `ActionXML`, on the other hand, is probably done directly through its properties. – Jeroen Mostert Jul 21 '17 at 15:02
  • 2
    [That](https://stackoverflow.com/questions/26063027/why-is-the-original-object-changed-after-a-copy-without-using-ref-arguments) wasn't a duplicate Servy – Tim Schmelter Jul 21 '17 at 15:03
  • 1
    Tempted to hammer as https://stackoverflow.com/questions/78536/deep-cloning-objects – DavidG Jul 21 '17 at 15:05
  • 1
    @servy: the topic of this question is: why does this copy-constructor doesn't work as expected? Answer: Because it is half-finished. The question of the related question was: why is my original instance changed if i pass a copy to a method that changes it? Answer: because it is not a copy at all but the same reference. – Tim Schmelter Jul 21 '17 at 15:15
  • @TimSchmelter That's correct, and the reason this person's copy constructor doesn't work as expected is because they're passing around an instance and changing it, and don't understand why C# behaves as it does when that happens. The answer to both questions is the same, because they're doing the same thing, and have the same problem, as a result of the same misunderstanding, even if the reason they're doing the thing is different. – Servy Jul 21 '17 at 15:18
  • @Servy: the misunderstanding is different. Here OP knows that he needs to create a new instance to avoid that issue, he even uses a copy-constructor and doesn't simply assign a `XMLStructure`-instance to a second variable as OP of the other answer did. This is more a real programming issue while the other is just a fundamental lack of understanding of how .NET works. Only the symptoms are same. – Tim Schmelter Jul 21 '17 at 15:26
  • @TimSchmelter The OP just copied a reference type from one variable to another and expected it to create a new instance of it and is surprised that mutating one is observable through the other. It's *the exact same* problem, with the exact same misunderstanding, as the other question. I don't even know what you mean by "more real" but just because you think the OP made *the exact same mistake* in "real" code (whatever that means) doesn't change the question, or the answer. – Servy Jul 21 '17 at 15:28
  • @Servy: that's why i've said it's half-finished. He used the right approach but the implementation has a bug. If there was no copy-constructor but just a variable1 assignment to variable2 in the code which OP hasn't shown, then these were duplicates. – Tim Schmelter Jul 21 '17 at 15:30
  • @TimSchmelter So you understand that it's the exact same problems as the duplicate, with the exact same resolution, and the exact same explanation, but don't think it's a duplicate because there is some irrelivant context that has nothing at all to do with the question or answer. That's...now how duplicates work. Adding in some irrelivant information to a question doesn't make it not a duplicate of a question without that irrelevant information. – Servy Jul 21 '17 at 15:34
  • 2
    There is much advice here on the need to clone the Action property but no one has suggested using String.Copy() for the name property to ensure the Name property of the new cloned object is a genuine independent copy of the string. I appreciate this is an edge case due to immutable .Net strings but feel a mention of String.Copy() adds to the discussion. – camelCase Jul 21 '17 at 15:38
  • @camelCase: I'd rather go the opposite direction and mention the option of making `ActionXML` an immutable type, just like `String` is. Immutability needs more love. – Jeroen Mostert Jul 21 '17 at 17:41

3 Answers3

7

ActionXML is reference type, you will also need to create a copy of ActionXML.

Here is a link to a web page explaining reference types vs value types.

jProg2015
  • 1,098
  • 10
  • 40
1

You need to "deep clone" the object to avoid the problem you have observed. The accepted method for doing this in .Net has evolved over the years. Today the simplest option is to serialise an object out to a JSON string and then hydrate a new object from that JSON string.

var json = JsonConvert.SerializeObject(xmlSourceObject );
var clonedXmlObject = JsonConvert.DeserializeObject<XMLStructure>(json);

The more traditional .Net solution is to implement the ICloneable interface.

camelCase
  • 1,549
  • 2
  • 16
  • 28
1

you need to do same (add constructor, that allows to copy) for ActionXML and any other reference type variable in that class.

Hiran
  • 1,102
  • 11
  • 18