4

So I needed a method to deep clone. I wanted one list of cards to equal another list of cards, but then I also wanted to modify one of the clones.

I made a method to copy the list like this:

    public List<Card> Copy(List<Card> cards)
    {
        List<Card> clone = new List<Card>();
        foreach (var card in cards)
        {
            clone.Add(card);
        }
        return clone;
    }

and use it like this:

        _cards = new List<Card>();
        _thrownCards = new List<Card>();

        _cards = Copy(_thrownCards);
        _thrownCards.Clear();

I'm not experienced in C#, but somehow my gut feelings tells me my copy method could be made simpler. Isn't there any other way you could deep copy a list? I tried using MemberWiseClone, but that just created references to the same object, not cloning it(maybe I misinterpreted the MemberWiseClone method).

Do anyone have any tip how simply clone a list object?

Jesper
  • 2,044
  • 3
  • 21
  • 50
  • 2
    You can clone the list with `cards.ToList()` but if `Card` is a reference type you'll need to clone each list element as well to get a deep copy. – Lee Jun 02 '17 at 13:18
  • 1
    Your method is not a deep cloning, because all `Card` instances are not cloned but just copied to another list (assuming they are not structs). – Evk Jun 02 '17 at 13:18
  • I just want to point out that since your Copy function creates and returns a `new List()` you don't actually need to initialize _cards first. You could use your function like this: `_thrownCards = new List(); ...; _cards = Copy(_thrownCards); _thrownCards.Clear();` Also, what you are actually doing in your Copy function is a shallow clone. Here's a [Stackoverflow question](https://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy) about the difference between the two. – ashbygeek Jun 02 '17 at 13:28

4 Answers4

7

That's not a real deep-copy because the Card instances are still the same, only the list is different. You could have this much simpler:

List<Card> cloneList = cards.ToList();

You need to "copy" all properties of the Card instances as well:

public List<Card> Copy(List<Card> cards)
{
    List<Card> cloneList = new List<Card>();
    foreach (var card in cards)
    {
        Card clone = new Card();
        clone.Property1 = card.Property1;
        // ... other properties
        cloneList.Add(clone);
    }
    return cloneList;
}

You could also provide a factory method that creates a clone of a given Card instance:

public class Card
{
    // ...

    public Card GetDeepCopy()
    {
        Card deepCopy = new Card(); 
        deepCopy.Property1 = this.Property1;
        // ...
        return deepCopy;
    }
}

Then you have encapsulated this logic in one place where you can even access private members(fields, properties, constructors). Change the line in the Copy method above to:

cloneList.Add(card.GetDeepCopy()); 
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
  • 1
    Would you recommend putting the copy method on `Card` itself? `cloneList.Add(card.DeepCopy())` – Frank Modica Jun 02 '17 at 13:23
  • @TimSchmelter Sorry, got an error and misinterpreted my own structure. This worked fine, so I marked it as solution! – Jesper Jun 02 '17 at 13:34
1

To have deep copy, you would need have something like that:

public List<Card> Copy(List<Card> cards)
{
    List<Card> clone = new List<Card>();
    foreach (var card in cards)
    {
        clone.Add(new Card 
        {
          property1 = card.property1;
          property2 = card.property2; // and so on
        });
    }
    return clone;
}

Of course if property1 and property2 are also a refference type objects, then you would have to go deeper.

pitersmx
  • 935
  • 8
  • 27
0

How about something like

List<Card> _cards = _thrownCards.ConvertAll(Card => new Card(<card parameters here>));
Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325
Matt
  • 1,596
  • 2
  • 18
  • 32
0

Unlike shallow copy when we copy references only

 public List<Card> Copy(List<Card> cards) {
   return cards.ToList();
 }

In case of deep copy we have to clone each item:

 public List<Card> Copy(List<Card> cards) {
   return cards
     .Select(card => new Card() {
        //TODO: put the right assignment here  
        Property1 = card.Property1,
        ...
        PropertyN = card.PropertyN,
     }) 
     .ToList();
 }
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215