Problem with making a deepcopy using IClonable interface in C#

Question

I'm having a trouble of making a deep copy of an object.

I need to make a deep copy of Graph class object. This is my Graph class and Edge class from which Graph is using objects.

class Graph : ICloneable
    {
        private List<Edge> edges;
        private List<int> vertices;

        public Graph()
        {
            edges = new List<Edge>();
            vertices = new List<int>();
        }
       
        public List<Edge> Edges
        {
            get
            {
                return edges;
            }
        }

        public List<int> Vertices
        {
            get
            {
                return vertices;
            }
        }
    }

    class Edge
    {
        public int vertexV;
        public int vertexU;
        public int weigth;

        public Edge(int vertexV, int vertexU, int weigth)
        {
            this.vertexV = vertexV;
            this.vertexU = vertexU;
            this.weigth = weigth;
        }
    }

So far, I have tried:

 public Graph Clone() { return new Graph(this); }
 object ICloneable.Clone()
 {
       return Clone();
 }

public Graph(Graph other)
{
    this.edges = other.edges;
    this.vertices = other.vertices;
}

public object Clone()
{
     var clone = (Graph)this.MemberwiseClone();
     return clone;
}

But it only created a shallow copy which doesn't do the trick. Of course, IClonable interface was implemented for all examples above. I tried looking on other examples online but with no results. I was using foreach loop the add all the elements from edges and vertices but that solution is extremely slow.

Answer 1

Welcome to the joys of OOP!

Joking aside, you will need to create new List objects when you construct the clone:

public Graph(Graph other)
{
    this.edges = new List<int>(other.edges);
    this.vertices = new List<int>(other.vertices);
}

Your Clone code would then be unchanged:

public Graph Clone() { 
    return new Graph(this); 
}

object ICloneable.Clone()
{
    return Clone();
}

If your Edge class is mutable then you will need to clone those too:

public Graph(Graph other)
{
    this.edges = other.edges.Select(e => new Edge(e.vertexV, e.vertexU, e.weight)).ToList();
    this.vertices = new List<int>(other.vertices);
}

Since your nodes are int, which is a value-type, you might consider making the Graph class immutable. Immutable types never need to be cloned, which can make code much easier to understand.

Answer 2

This should work

public object Clone()
{
    Graph graph = new Graph();
    edges.ForEach(e => graph.edges.Add(new Edge(e.vertexV, e.vertexU, e.weigth)));
    graph.vertices.AddRange(vertices.ToArray());
    return graph;
}