Fever040
Fever040

Reputation: 3

Add classes to list dynamically with enum values

I'm trying to make a method that creates a list of objects. The objects are added with enumerator values.

I currently have 2 enumerators: CardType, CardColor. Assigning the CardType value works as intented but when I use the exact same code to asign the CardColor it always returns the same value(Red).

I've included the switch statement I have already tried in comment form.

Class with main method:

 public class CardStack
        {
            Card CardObject;
            List<Card> cardList;
            public CardStack()
            {
                cardList = new List<Card>();
                CardObject = new Card();
            }
            public List<Card> getCardList()
            {     
                for(int i = 1; i <= Enum.GetNames(typeof(CardType)).Length; i++)
                {
                    CardObject.Type = addCardType(i);
                    for(int n =0; n <= 3; n++)
                    {

                        CardObject.Color = addCardColor(n);
                        cardList.Add(CardObject);
                    }

                    CardObject = new Card();
                }
                return cardList;
            }

            public CardColor addCardColor(int colorNumber)
            {
                //Debug.WriteLine(colorNumber.ToString());
                //switch (colorNumber)
                //{

                //    case 0: return CardColor.Green;
                //    case 1: return CardColor.Blue;
                //    case 2: return CardColor.Yellow;
                //    case 3: return CardColor.Red;                
                //    default:
                //        return CardColor.Yellow;
                //}
                return (CardColor)colorNumber;
            }
            public CardType addCardType(int typeNumber)
            {
                return (CardType)typeNumber;
            }

        }

Enum CardColor:

public enum CardColor
{
    Green,
    Blue,
    Yellow,
    Red
}

Enum CardType:

public enum CardType
{
   One = 1,
   Two = 2,
   Three = 3,
   Four = 4,
   Five = 5,
   Six = 6,
   Give_2,
   Take_1,
   Which
}

My debug output(shortened):

 Type: One; Color: Red;
[0:] Type: One; Color: Red;
[0:] Type: One; Color: Red;
[0:] Type: One; Color: Red;
[0:] Type: Two; Color: Red;
[0:] Type: Two; Color: Red;
[0:] Type: Two; Color: Red;
[0:] Type: Two; Color: Red;
[0:] Type: Three; Color: Red;
[0:] Type: Three; Color: Red;
[0:] Type: Three; Color: Red;
[0:] Type: Three; Color: Red;

Upvotes: 0

Views: 576

Answers (3)

Fildor
Fildor

Reputation: 16059

for(int i = 1; i <= Enum.GetNames(typeof(CardType)).Length; i++)
{
    CardObject.Type = addCardType(i);         // Sets Card Type
    for(int n =0; n <= 3; n++)
    {
        CardObject.Color = addCardColor(n);   //1. Sets Color on THE SAME CardObject
        cardList.Add(CardObject);             //2. Adds THE SAME CardObject 4 times
    }
    CardObject = new Card();                  //3. create new Object => all of the 
                                              // above added references point to
                                              // the same CardObject, which has 
                                              // been set to "Red".
}

Solution: Move Line marked with //3. inside the inner for loop, right after //2.

Unrelated:

  • I'd recommend to create a parameterized Constructor, so you can simply do cardList.Add(new Card(type, color)).
  • Since Type and Color of a Card won't change, they should be read-only. In fact Card should be immutable.
  • Just use foreach to iterate the enum values.

Improved version:

public IEnumerable<Card> GetCardList()
{
    foreach (CardType type in Enum.GetValues(typeof(CardType)))
    {
        foreach (CardColor color in Enum.GetValues(typeof(CardColor)))
        {
            yield return new Card(type,color);
        }
    }
}

(see yield contextual keyword)

Where Card:

public class Card
{
    public readonly CardType Type;
    public readonly CardColor Color;

    public Card( CardType type, CardColor color )
    {
        this.Type = type;
        this.Color = color;
    }
}

Upvotes: 2

Anu Viswan
Anu Viswan

Reputation: 18155

In your current code, in the inner loop, you are modifying the properties of the same instance of Card (CardObject) while adding it to cardList. This would mean, all the reference to have the same value as the Color in the last loop, which in your case would be Red.

What you need to do is to reinitialize the CardObject within the Inner loop. For example,

for(int i = 1; i <= Enum.GetNames(typeof(CardType)).Length; i++)
{
    CardObject.Type = addCardType(i);
    for(int n =0; n <= 3; n++)
    {
         CardObject = new Card();   // Change here
         CardObject.Color = addCardColor(n);
         cardList.Add(CardObject);
    }
}

This would ensure you are dealing with a new instance of Card every time in the inner loop.

Meanwhile, another approach to accomplish the same using Linq would be

var list = Enum.GetValues(typeof(CardType))
               .Cast<CardType>()
               .SelectMany(type => Enum.GetValues(typeof(CardColor))
                                       .Cast<CardColor>()
                                       .Select(color => 
                                        new Card {Type = type, Color = color}));

Upvotes: 0

keuleJ
keuleJ

Reputation: 3486

Use foreach to iterate over the enum-values. Make a new CardObject in each interation (inner loop!) and add them to the list.

public class CardStack
{
    private readonly List<Card> _cardList;
    public CardStack()
    {
        _cardList = getCardList();

    }
    private static List<Card> getCardList()
    {
        var localList = new List<Card>();
        foreach (CardType type in Enum.GetValues(typeof(CardType)))
        {
            foreach (CardColor color in Enum.GetValues(typeof(CardColor)))
            {
                var cardObject = new Card(){Type = type, Color = color};
                localList.Add(cardObject);
            }
        }

        return localList;
    }
}
}

Upvotes: 0

Related Questions