Page 1 of 1

Object.MemberwiseClone() instead of Copy()

Posted: Tue Jun 19, 2007 9:46 am
by fcarlier
I noticed that the Table Type objects expose a Copy() method. It creates a new object, and field-by-field copies the variables.

However, every object already exposes the MemberwiseClone() method, which does exactly this.

Hence, my proposal would be to replace

Code: Select all

Public TableType Copy()
{
  TableType copy = new TableType();
  copy.A = A;
  copy.B = B;
  return copy;
}
by

Code: Select all

Public TableType Copy()
{
  return (TableType)this.MemberwiseClone();
}
.

I would also propose to add a Clone() method (which does the same) and implement the IClonable interface (a standard .NET interface)

Posted: Tue Jun 19, 2007 10:10 am
by jordansparks
Are you kidding me? Tell me at least that it's a fairly new method that MS snuck in when I wasn't looking. Yes, of course we should move to that and simply get rid of the whole Copy thing altogether. Except there are a few rare situations where I think it's doing just a bit more, and I would worry about introducing a bug. Like in ClaimForm, where it also copies the array of attached items.

Using Clone()

Posted: Tue Jun 19, 2007 10:36 am
by grahamde
I noticed this problem a while back and I agree that we should be using this basic object level functionality.

In response to Dr. Sparks, we can still use complicated copy functionality by overriding the Clone() function in the specific situations that require it.
For instance:

public override bool Clone(){
ClaimForm cf=this.MemberwiseClone();
for(int i=0;i<x;i++){
cf.attachments=this.attachments.Clone();
}
return cf;
}

Posted: Wed Jun 20, 2007 12:18 am
by fcarlier
jordansparks wrote:Are you kidding me? Tell me at least that it's a fairly new method that MS snuck in when I wasn't looking.
To your defense, it is a protected member of the object class, so it isn't obvious at all this method exists. For "deep" copies, grahamde's solution is perfect.