Search code examples
c#clone

Cloning an object doesn't work properly in C#


I'm having trouble with copying an object.

I want to copy object1 and then make changes to object2 without changing object1.

// I'm cloning lower.Rebar object here
upper.Rebar = lower.Rebar.Clone();

// When I make changes to upper.Rebar, lower.Rebar gets changed too.
upper.Rebar.Polyline.Points.RemoveAll(p => p.Y < breaklevel);

// Here is how I'm cloning the lower.Rebar object
public ColumnElevationRebar Clone()
{
    var obj = new ColumnElevationRebar();

    obj.CanBeContinued = CanBeContinued;
    obj.CanBeSpliced = CanBeContinued;
    obj.ConnectionType = ConnectionType;
    obj.SpliceLength = SpliceLength;
    obj.Polyline = Polyline;

    return obj;
}

Definition of the class

public class ColumnElevationRebar
    {
        public ColumnElevationRebar Clone()
        {
            var obj = new ColumnElevationRebar();

            obj.CanBeContinued = CanBeContinued;
            obj.CanBeSpliced = CanBeContinued;
            obj.ConnectionType = ConnectionType;
            obj.SpliceLength = SpliceLength;
            obj.Polyline = Polyline;

            return obj;
        }

        private double splicelength;
        public double SpliceLength
        {
            get { return splicelength; }
            set { splicelength = Arithmetics.Ceiling(value, 100); }
        }

        public RebarConnectionType ConnectionType { get; set; }

        public bool CanBeSpliced;
        public bool CanBeContinued;

        public bool IsSelected;

        public Polyline Polyline { get; set; }

        public double Length
        {
            get { return Arithmetics.Ceiling(Polyline.Length, 100); }
        }
    }


public class Polyline
{
    public Polyline Clone()
    {
        var obj = new Polyline {Points = this.Points};
        return obj;
    }

    public Polyline()
    {
        Points = new List<_2DPoint>();
    }

    public List<_2DPoint> Points { get; set; }
}

Solution

  • You are making only a shallow copy of the object. You should clone all non-value type objects, if you need a deep copy. Based on the names, I'm assuming only Polyline needs cloning:

    public ColumnElevationRebar Clone()
    {
        var obj = new ColumnElevationRebar();
    
        obj.CanBeContinued = CanBeContinued;
        obj.CanBeSpliced = CanBeContinued;
        obj.ConnectionType = ConnectionType;
        obj.SpliceLength = SpliceLength;
        obj.Polyline = Polyline.Clone();
    
        return obj;
    }
    

    Just define the Clone method in the Polyline object (I don't know the class).

    Edit: You are making the same mistake in your Polyline class, you also need to clone the list:

    public Polyline Clone()
    {
        var obj = new Polyline {Points = new List<_2DPoint>(this.Points)};
        return obj;
    }