5

I have implemented a deep clone method using a copy constructor in C#. To verify that it works, I am testing it by comparing the results of serializing an object and it's clone. The serialization is done in terms of a generic object T. I have also tried it in terms of a concrete object and get the same results.

I have a method for serializing the objects into byte arrays

private byte[] ObjectToBytes(T obj)
{
    BinaryFormatter formatter = new BinaryFormatter();
    using (MemoryStream stream = new MemoryStream())
    {
        formatter.Serialize(stream, obj);
        stream.Seek(0, SeekOrigin.Begin);
        return stream.ToArray();
    }
}

The following code works fine.

T original = this.GetNewThing();
T clone = original.DeepClone();

// serialize after cloning
byte[] originalBytes = ObjectToBytes(original);
byte[] cloneBytes = ObjectToBytes(clone);

bool equal = true;
for (int i = 0; i < originalBytes.Length; i++)
{
    if(originalBytes[i] != cloneBytes[i]
    {
        equal = false;
        break;
    }
}
equal == true; // True!

However, when I switch the order of when the objects are serialized, the byte arrays are no longer equal.

// serialize before cloning
T original = this.GetNewThing();
byte[] originalBytes = ObjectToBytes(original);

T clone = original.DeepClone();
byte[] cloneBytes = ObjectToBytes(clone);

bool equal = true;
for (int i = 0; i < originalBytes.Length; i++)
{
    if(originalBytes[i] != cloneBytes[i]
    {
        equal = false;
        break;
    }
}
equal == true; // False!

Why does the order of serialization affect this? Does it have something to do with the BinaryFormatter or MemoryStream objects?

EDIT:

Here's what the deep clone method looks like

public MyClass DeepClone()
{
    return new MyClass(this);
}

and the constructor it uses looks like this

protected MyClass(MyClass myClass)
{
    if (myClass == null)
        throw new ArguementNullException("myclass");

    this.number = myClass.Number;
    this.number2 = myClass.Number2;
    this.number3 = myClass.Number3;
}

The object is by no means complex. All the values being copied are values types, so there are no reference types that need to be worried about.

  • 6
    Please post the DeepClone method.. There is probably the issue – Manuel Amstutz May 13 '16 at 21:02
  • Might have to do with the complexity of the objects - check this out: http://stackoverflow.com/questions/5017274/binaryformatter-and-deserialization-complex-objects – Clay May 14 '16 at 20:09
  • Think we might need to see the full `MyClass`, too. Also, it has no state, so why is `ObjectToBytes` a private instance method, and not static? I think it might be helpful if you could format a full example code that could be cut/pasted to, say, LinqPad. – Marc L. May 16 '16 at 15:12
  • Also, your array-equality test doesn't really test equality. It only works if the arrays are the same length. – Marc L. May 16 '16 at 15:29
  • 1
    I just ran this in LinqPad, with missing details filled in with minimal examples that I just made up. The byte-arrays were equal in both cases. The problem is somewhere in something you haven't shown, probably in the class itself, maybe logic within the property accessors that are being called in the `MyClass(MyClass myClass)` constructor. – Marc L. May 16 '16 at 15:34
  • 1
    Check all those properties - it may be that reading a property changes the state. – Luaan May 16 '16 at 15:45
  • The answer from @MarcL. made me realize what was going on! I have a property in the original class that isn't initialized until it is first accessed. Serializing the object does not cause it to instantiate, but the copy constructor does. That's why the byte arrays aren't equal when created in different orders! Thanks so much for yalls help. Sorry I didn't post more specific code, my job does not allow me to. –  May 16 '16 at 16:46
  • Great! glad you found a solution. I would recommend accepting Slugart's answer below, since it is correct: The call to `DeepClone()` was indeed changing the state of the object. – Marc L. May 16 '16 at 16:50

2 Answers2

3

DeepClone, or something it calls in turn, seems to be changing the state of your source object.

Marc L.
  • 3,296
  • 1
  • 32
  • 42
Slugart
  • 4,535
  • 24
  • 32
  • DeepClone changes the source object by accessing a property that is not set until the first time it is accessed. When I clone before creating the byte arrays this is fine, because both the properties get initialized during the deep clone. Creating the byte array before cloning does not cause this property to be initialized, and the serialization doesn't pick up the initialized values (the property is a value type, so instead it picks up the default values). –  May 16 '16 at 16:52
1

You must modify the ObjectToBytes method as follow:

private byte[] ObjectToBytes(Object obj)
    {
       if(obj == null)
          return null;

       BinaryFormatter bf = new BinaryFormatter();
       using(MemoryStream ms = new MemoryStream()) {
           bf.Serialize(ms, obj);

           return ms.ToArray();
       }
    }

Hope it helps

RobertoB
  • 72
  • 5
  • Can you explain how you think this helps? – adv12 May 13 '16 at 21:39
  • @adv12 because it is a bad practice alter stream position before .ToArray. It is a try, but I'd like to see DeepClone implementation. – RobertoB May 13 '16 at 21:50
  • I removed the unnecessary call to `Seek` in my code. It did not fix the issue. It also did not cause any problems when I removed it so I will leave it out. –  May 16 '16 at 15:03
  • 1
    You gonna dispose of that `MemoryStream`? – Marc L. May 16 '16 at 15:19
  • Memory streams don't really matter if they are disposed or not. [The only thing it does](http://referencesource.microsoft.com/#mscorlib/system/io/memorystream.cs,0b83d17ca69bf8ea) is mark the stream as closed and unwriteable – Scott Chamberlain May 18 '16 at 21:08