CODEWITHSUNDEEP
c#stack-overflowicloneable
Patrick
Patrick

Reputation: 1783

C# stack overflow

I am trying to find out why I am getting a stack overflow exception. I am creating a simple card game for a school assignment and when I clone the cards to return them I get the stack overflow exception.

So I got this card class:

public class Card : ICloneable
{
    ....

    #region ICloneable Members

    public object Clone()
    {
        return this.Clone(); // <--- here is the error thrown when the first card is to be cloned
    }

    #endregion
}

and I have a class called Hand which then clones the cards:

internal class Hand
{
        internal List<Card> GetCards()
        {
            return m_Cards.CloneList<Card>(); // m_Cards is a List with card objects
        }
}

Last, I got an extension method for the List:

    public static List<T> CloneList<T>(this List<T> listToClone) where T : ICloneable
    {
        return listToClone.Select(item => (T)item.Clone()).ToList();
    }

The error gets thrown in the card class (IClonable method),

An unhandled exception of type 'System.StackOverflowException' occurred in CardLibrary.dll

Upvotes: 1

Views: 1558

Answers (2)

Terry_Brown
Terry_Brown

Reputation: 1408

I've tended to use MemberwiseClone() for the simple data, and then implemented ICloneable throghout the hierarchy of elements that I've needed to clone, so:

public class CRMLazyLoadPrefs : ICloneable
{
    public bool Core { get; set; }
    public bool Events { get; set; }    
    public bool SubCategories { get; set; }
    public OrganisationLazyLoadPrefs { get; set; }

    public object Clone()
    {
        CRMLazyLoadPrefs _prefs = new CRMLazyLoadPrefs();
        // firstly, shallow copy the booleans
        _prefs  = (CRMLazyLoadPrefs)this.MemberwiseClone();
        // then deep copy the other bits
        _prefs.Organisation = (OrganisationLazyLoadPrefs)this.Organisation.Clone();
    }
}

Where OrganisationLazyLoadPrefs also implements ICloneable and so on and so forth throughout the hierarchy.

Hope this helps, Cheers, Terry

Upvotes: 0

Philippe Leybaert
Philippe Leybaert

Reputation: 171864

You're calling yourself:

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

This results in infinite recursion.

Your Clone() method should copy all properties/fields to a new object:

public object Clone()
{
    Card newCard = new Card();

    newCard.X = this.X;
    // ...

    return newCard;
}

or you could use MemberwiseClone()

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

But that gives you less control over the cloning process.

Upvotes: 22

Related Questions