locked
ICloneable and Deep Copy of a Class RRS feed

  • Question

  • Is this a good way to deep copy a class?

     

    The main class is Graph and contains a List of Plots.

    Plots contains a list of Series.

    Series contains x and y.

     

    Code Snippet

    using System;
    using System.Collections.Generic;
    using System.Text;

    namespace ConsoleApplication
    {
      class Series : ICloneable
      {
        public int x;
        public int y;
       
        public Series()
        {
        }

        public object Clone()
        {
          Series series = new Series();

          series.x = this.x;
          series.y = this.y;

          return series;
        }
      }

      class Plot : ICloneable
      {
        public List<Series> seriesList = new List<Series>();
     
        public Plot()
        {
        }

        public object Clone()
        {
          Plot plot = new Plot();
         
          for (int i = 0; i < seriesList.Count; i++)
          {
            plot.seriesList.Add((Series)seriesList[i].Clone());
          } 
         
          return plot;
        }
      }

      class Graph : ICloneable
      {
        public List<Plot> plotList = new List<Plot>();
     
        public Graph()
        {
        }

        public object Clone()
        {
          Graph graph = new Graph();

          for (int i = 0; i < plotList.Count; i++)
          {
            graph.plotList.Add((Plot)plotList[i].Clone());
          } 

          return graph;
        }
      }

      class Program
      {
        static void Main(string[] args)
        {
          Graph graph1 = new Graph();
         
          graph1.plotList.Add(new Plot());
          graph1.plotList[0].seriesList.Add(new Series());
         
          graph1.plotList[0].seriesList[0].x = 1;
          graph1.plotList[0].seriesList[0].y = 2;
         
          Graph graph2 = (Graph)graph1.Clone();
         
          graph2.plotList[0].seriesList[0].x = 3;
          graph2.plotList[0].seriesList[0].y = 4;

          Console.WriteLine("graph1 X {0}, graph1 Y {1}",

            graph1.plotList[0].seriesList[0].x, graph1.plotList[0].seriesList[0].y);
          Console.WriteLine("graph2 X {0}, graph2 Y {1}",

           graph2.plotList[0].seriesList[0].x, graph2.plotList[0].seriesList[0].y);
        }
      }
    }

     

     

    Tuesday, August 7, 2007 5:11 AM

Answers

  • Looksreasonable.  A couple of things:

    • There's some debate about the usefulness of ICloneable in that "Clone" isn't explicit enough to tell you want it's going to do.  Depending on who you talk to, it's more advisable to create your own interface (if you need to) or your own pattern of deep clone method naming (like DeepClone(), for example) and use that instead of ICloneable.
    • Second, the problem with the way you're doing it now (which will work, don't get me wrong) is that you must keep your Clone method up-to-date whenever you change fields in your class.  This is good for performance but a bit of a maintenance nightmare.  Often I've seen people just serialize the object and deserialize it to make a clone; which will catch any new fields without having to modify your clone method.  A drawback of that is you must attribute your class as Serializable.  For example:

    Code Snippet

    [Serializable]
    class Series
    {
     public int x;
     public int y;
       
     public Series(int x, int y)
     {
      this.x = x;
      this.y = y;
     }

     public Series DeepClone()
     {
      using(MemoryStream memoryStream = new MemoryStream(10))
      {
       IFormatter formatter = new BinaryFormatter();
       formatter.Serialize(memoryStream, this);
       memoryStream.Seek(0, SeekOrigin.Begin);
       return formatter.Deserialize(memoryStream) as Series;
      }
     }
    }

     

     

     

    Tuesday, August 7, 2007 1:36 PM

All replies

  • Looksreasonable.  A couple of things:

    • There's some debate about the usefulness of ICloneable in that "Clone" isn't explicit enough to tell you want it's going to do.  Depending on who you talk to, it's more advisable to create your own interface (if you need to) or your own pattern of deep clone method naming (like DeepClone(), for example) and use that instead of ICloneable.
    • Second, the problem with the way you're doing it now (which will work, don't get me wrong) is that you must keep your Clone method up-to-date whenever you change fields in your class.  This is good for performance but a bit of a maintenance nightmare.  Often I've seen people just serialize the object and deserialize it to make a clone; which will catch any new fields without having to modify your clone method.  A drawback of that is you must attribute your class as Serializable.  For example:

    Code Snippet

    [Serializable]
    class Series
    {
     public int x;
     public int y;
       
     public Series(int x, int y)
     {
      this.x = x;
      this.y = y;
     }

     public Series DeepClone()
     {
      using(MemoryStream memoryStream = new MemoryStream(10))
      {
       IFormatter formatter = new BinaryFormatter();
       formatter.Serialize(memoryStream, this);
       memoryStream.Seek(0, SeekOrigin.Begin);
       return formatter.Deserialize(memoryStream) as Series;
      }
     }
    }

     

     

     

    Tuesday, August 7, 2007 1:36 PM
  • That method is alot easier.  The real code has alot more fields and would have been tedious work to use my original method.  The updated example code is included for reference.

     

    Code Snippet

    using System;
    using System.Collections.Generic;
    using System.Text;
    using System.IO;
    using System.Runtime.Serialization;
    using System.Runtime.Serialization.Formatters.Binary;

    namespace ConsoleApplication
    {
      [Serializable]
      class Series : ICloneable
      {
        public int x;
        public int y;
       
        public Series()
        {
        }

        public object Clone()
        {
          using (MemoryStream ms = new MemoryStream())
          {
            IFormatter bf = new BinaryFormatter();
         
            bf.Serialize(ms, this);
            ms.Position = 0;
         
            return bf.Deserialize(ms);
          }
        }
      }

      [Serializable]
      class Plot : ICloneable
      {
        public List<Series> seriesList = new List<Series>();
     
        public Plot()
        {
        }

        public object Clone()
        {
          using (MemoryStream ms = new MemoryStream())
          {
            IFormatter bf = new BinaryFormatter();
         
            bf.Serialize(ms, this);
            ms.Position = 0;
         
            return bf.Deserialize(ms);
          }
        }
      }

      [Serializable]
      class Graph : ICloneable
      {
        public List<Plot> plotList = new List<Plot>();
     
        public Graph()
        {
        }

        public object Clone()
        {
          using (MemoryStream ms = new MemoryStream())
          {
            IFormatter bf = new BinaryFormatter();
         
            bf.Serialize(ms, this);
            ms.Position = 0;
         
            return bf.Deserialize(ms);
          }
        }
      }

      class Program
      {
        static void Main(string[] args)
        {
          Graph graph1 = new Graph();
         
          graph1.plotList.Add(new Plot());
          graph1.plotList.Add(new Plot());
          graph1.plotList[0].seriesList.Add(new Series());
          graph1.plotList[1].seriesList.Add(new Series());
          graph1.plotList[1].seriesList.Add(new Series());
         
          graph1.plotList[1].seriesList[1].x = 1;
          graph1.plotList[1].seriesList[1].y = 2;
         
          Graph graph2 = (Graph)graph1.Clone();
         
          graph2.plotList[1].seriesList[1].x = 3;
          graph2.plotList[1].seriesList[1].y = 4;

          Console.WriteLine("graph1 X {0}, graph1 Y {1}",

           graph1.plotList[1].seriesList[1].x, graph1.plotList[1].seriesList[1].y);
          Console.WriteLine("graph2 X {0}, graph2 Y {1}",

           graph2.plotList[1].seriesList[1].x, graph2.plotList[1].seriesList[1].y);
        }
      }
    }

     

     

     

     

    Wednesday, August 8, 2007 3:20 AM