tags:

views:

159

answers:

4

When (deep) Cloning a custom object, should I use clone.str1 = String.Copy(obj.str1) or clone.str1 = obj.str1?

I'd prefer the latter - shorter and quicker, but is it "safe"?

I'd point to this thread, but, however, not sure what to use.

+3  A: 

Calling String.Copy will create a separate copy of the string.
Since strings are immutable, there's no need to do that.

SLaks
so, understood, that I can use the a = b. However, is not always safe, by e.g. (from the provided link) : "This may matter if you use the string with unmanaged code which deals with the memory locations directly and can mutate the string."
serhio
Are you really dealing with unmanaged code that mutates a string in-place?
SLaks
Actually, no...
serhio
+1  A: 

When deep cloning, why not just use a BinaryFormatter instead?

See this link.

Here's the code I wrote at that link:

public Automobile Clone()
{
    Automobile result = null;
    using (MemoryStream ms = new MemoryStream())
    {
         BinaryFormatter bf = new BinaryFormatter();
         bf.Serialize(ms, this);
         ms.Position = 0;
         result = (Automobile)bf.Deserialize(ms);
     }
     return result;
}
David Morton
The trick with the binary formatter is handy, especially for existing classes. However, it is *nowhere near* as fast or memory efficient as writing your own deep clone. It shouldn't be your first choice approach.
Rob Levine
problems with cycle references.
serhio
Cyclic references should be just fine. But non-serializable classes will be a problem.
SealedSun
@SealedSun: why do you mean "just fine"? I can't binary serialize a object that has a parent.
serhio
@serhio - I haven't checked, but I'm pretty sure (90%) that is only an issue with xml serialization and not binary serialization in .Net.
Rob Levine
There's no problem with cyclic references in binary serialization... and it's fast... and it requires no maintenance... and, if properly implemented, it's reusable across classes.
David Morton
you both have reason. This is an interesting way to go. Thanks, David. It's a pity that the question is formulated as is, so I can't make your answer as answer, but is was useful.
serhio
no without problems, however, I use VB.NET, so Handles makes me problems, but found a solution http://www.codeproject.com/KB/vb/serializevbclasses.aspx
serhio
A: 

when deep cloning, you need to clone every mutable value. String is mutable on .NET, so yes, you should clone it.

edit: whoops ... seems they are immutable, so no, you don't need to. :)

greetz

back2dos

back2dos
This is completely false.
SLaks
@serhio: no, sry, I was wrong. I assumed String is mutable, since it has an insert method, but actually this returns a new string. this is also why clone returns the string itself.
back2dos
@back2dos, I understood. However, string is immutable only in the managed code ;)
serhio
@serhio: yeah, ok, if some evil soul from the dark hells of low level languages comes along and tampers with your data, then everything's possible and nothing's valid ... :D
back2dos
+3  A: 

Yes - the latter choice (simple assignment) is safe for strings (in managed code), as this code illustrates:

    string s1 = "Initial Value";
    string s2 = s1;

    Console.WriteLine("String1: " + s1);
    Console.WriteLine("String2: " + s2);

    s1 = "New Value";

    Console.WriteLine("String1 - after change: " + s1);
    Console.WriteLine("String2 - after change: " + s2);

Output:

String1: Initial Value
String2: Initial Value
String1 - after change: New Value
String2 - after change: Initial Value

Strings are immutable - so when you change s1, you are really creating a new string and assigning it. The reference of s2 remains pointing to the old instance.

Rob Levine
so, what is wrong with the String.Copy?
serhio
Nothing is wrong with it, but under the hood it uses unsafe code to do a char copy on the string. This means you are doing an unnecessary copy operation, I guess.
Rob Levine