How can I create a deep copy of my list collection

Question

Suppose I have the following class:

public class Author
{
    public int ID {get; private set;}
    public string firstName {get; private set;}
    public string lastName {get; private set; }

    public Author(int id, string firstname, string lastname)
    {
        this.ID = ID;
        this.firstName = firstname;
        this.lastName = lastName;
    }

    public static Author Clone(Author clone)
    {
        Author copyAuth = new Author(clone.ID, clone.firstName, clone.lastName);
        return copyAuth;
    }
}

and

public class Book
{
    public string bookTitle {get; private set;}
    private List<Author> authors;
    private List<Author> copyofAuthors;
    public string ISBN {get; private set; }

    public Book(string bookTitle, List<Author> authors, string ISBN)
    {
        copyofAuthors = new List<Author>();
        this.bookTitle = bookTitle;
        this.ISBN = ISBN;
        //How do I create a deep copy of my authors List?

        foreach(Author copy in authors)
        {
            Author addAuthors = Author.Clone(copy);
            copyofAuthors.Add(addAuthors);
        }
    }
}

How would I create a deep copy of my List<Authors> collection? I've read other pages on StackOverFlow that suggest serialization, and suggestions that I'm unfamiliar with and seem confusing.

I followed this link to create my clone method.

Questions 1:

Is the above implementation considered a deep copy? If so, is it okay to do it this way? By that I mean, a foreach loop in the constructor copying the authors to a new list collection.

Question 2:

If I modify anything in the copyofAuthor collection, it no longer references the original collection correct? So the original collection should remain the same?

Update #1 :

    public List<Author> Authors
    {
        get
        {
            return returnAuthors(authors);
        }

    }

    private List<Author> returnAuthors(List<Author> copyList)
    {
        List<Author> getAuthors = new List<Author>();

        foreach(Author copy in copyList){
            getAuthors.Add(Author.Clone(copy));
        }

        return getAuthors;
    }

Have I implemented my getter collection properly such that when it returns the List collection, it's independent of the original collection? So any changes made from the collection returned from the getter will not be reflected in the original collection correct?

Update #2:

With ReadOnlyCollection

    public class Book
    {

        public string bookTitle {get; private set;}
        private ReadOnlyCollection<Author> authors;
        public string ISBN {get; private set; }


        public Book(string bookTitle, ReadOnlyCollection<Author> authors, string ISBN)
        {

            this.bookTitle = bookTitle;
            this.ISBN = ISBN;
            //Is it okay to do this? 
            this.authors = authors;

        }

        public List<Author> Authors
        {
            get
            {   //Create a shallow copy
                return new ReadOnlyCollection<Author>(authors);
            }

        }

    }

Answer 1

It's getting too hard to sort this out in the comments.

• Remove the Clone method from your Author class. It is useless.

In your book class, you have two problems to solve.

• The constructor takes a list of authors, but the caller who passed it in might change it. If we just copy the reference, then the caller can alter accidentally the list held by the book.

• The book gives back a list of authors. If a caller adds something to that list then again, they've mutated the book.

You can solve both problems with an immutable collection. Download the immutable collections library using NuGet if you haven't got it already.

using System.Collections.Immutable;
...
public class Book
{
  public string bookTitle {get; private set;}
  private ImmutableList<Author> authors;
  public IReadOnlyList<Author> Authors { get { return authors; } }
  public string ISBN {get; private set; }

  public Book(string bookTitle, IEnumerable<Author> authors, string ISBN)
  {
    this.authors = ImmutableList<Author>.Empty.AddRange(authors);
    this.bookTitle = bookTitle;
    this.ISBN = ISBN;
  }
}

There. Now you make a copy of the sequence of authors, so if the caller changes that sequence, no worries, you have a copy. And you hand out an IReadOnlyList that is implemented by an immutable collection, so no one can change it.

Couple more things. You ask "is this right?"

public class Book
{
    private ReadOnlyCollection<Author> authors;
    public Book(ReadOnlyCollection<Author> authors)
    {
        //Is it okay to do this? 
        this.authors = authors;
    }

    public List<Author> Authors
    {
        get
        {   //Create a shallow copy
            return new ReadOnlyCollection<Author>(authors);
        }
    }

(Extraneous stuff removed).

No, that is not quite right, for a couple reasons. First, the read only collection is just a wrapper around a mutable collection. You are still in the situation where the caller controls the underlying collection, and therefore can change it.

Second, the typing doesn't quite work out; you can't convert a ReadOnlyCollection to a List.

This is confusing I know. There is a subtle distinction here. A read only collection is just that: you can only read it. It doesn't mean that someone else cannot write it! Such a collection is still mutable, it is just not mutable by you. An immutable collection is truly immutable; no one can change it.

Next: you are doing very well by making the author and the book both immutable. But what if you want to change it? As you note, to change an immutable book means making a new book. But you already have an old book; how can you do that efficiently? The common pattern is:

public class Book
{
  public string Title {get; private set;}
  private ImmutableList<Author> authors;
  public IReadOnlyList<Author> Authors { get { return authors; } }
  public string ISBN {get; private set; }

  public Book(string title, IEnumerable<Author> authors, string ISBN) : this(
    title, 
    ImmutableList<Author>.Empty.AddRange(authors),
    ISBN) {}

  public Book(string title, ImmutableList<Authors> authors, string ISBN) 
  {
    this.Title = title;
    this.Authors = authors;
    this.ISBN = ISBN;
  }
  public Book WithTitle(string newTitle)
  {
    return new Book(newTitle, authors, ISBN); 
  }
  public Book WithISBN(string newISBN)
  {
    return new Book(Title, authors, newISBN);
  }
  public Book WithAuthor(Author author)
  {
    return new Book(Title, authors.Add(author), ISBN);
  }
  public static readonly Empty = new Book("", ImmutableList<Author>.Empty, "");
}

Now you can do this:

Book tlotr = Book.Empty.WithAuthor("JRRT").WithTitle("The Lord Of The Rings");

And so on.

Answer 2

If you have many properties it ll be boring to write this:

new Author(clone.ID, clone.firstName, clone.lastName...);

Check this little exemple of how u can implement ICloneable interface :

This exemple comes from : http://csharp.2000things.com/2010/11/07/143-an-example-of-implementing-icloneable-for-deep-copies/

public class Person : ICloneable
{
    public string LastName { get; set; }
    public string FirstName { get; set; }
    public Address PersonAddress { get; set; }

    public object Clone()
    {
        Person newPerson = (Person)this.MemberwiseClone();
        newPerson.PersonAddress = (Address)this.PersonAddress.Clone();

        return newPerson;
    }
}

public class Address : ICloneable
{
    public int HouseNumber { get; set; }
    public string StreetName { get; set; }

    public object Clone()
    {
        return this.MemberwiseClone();
    }
}

Person herClone = (Person)emilyBronte.Clone();

Answer 3

Question 1: Yes this performs a deep copy. I recommend using the ICloneable interrface instead of this static function as it is standard.

Question 2: Yes the original collection will not change as you are using a new collection object.

Question 3: If you return the collection in the getter then someone can transform it. Either you return a new copy of the collection each time someone wants to get it or you wrap it in a container that would have a private readonly collection and does not expose the collection (but can be enumerable).

In your case not sure if you want the Author to be independent though. Given that they already can only be set privately, it might be useful to share updates on them. The only thing you need to keep independent is the collection. So maybe no need ot clone the Authors.