C# index number randomly generated and put into array to not occur again, still get the same result out of list with the number

Question

class Cards
{
    private Random random = new Random();
    private List<Card> playerDeck = new List<Card>();
    private List<Card> computerDeck = new List<Card>();
    private List<int> usedPlayerNrs = new List<int>();
    private List<int> usedComputerNrs = new List<int>();

    public enum Card
    {
        AceClubs = 11,
        AceDiamonds = 11,
        AceHearts = 11,
        AceSpades = 11,
        DeuceClubs = 2,
        DeuceDiamonds = 2,
        DeuceHearts = 2,
        DeuceSpades = 2,
        EightClubs = 8,
        EightDiamonds = 8,
        EightHearts = 8,
        EightSpades = 8,
        FiveClubs = 5,
        FiveDiamonds = 5,
        FiveHearts = 5,
        FiveSpades = 5,
        FourClubs = 4,
        FourDiamonds = 4,
        FourHearts = 4,
        FourSpades = 4,
        JackClubs = 11,
        JackDiamonds = 11,
        JackHearts = 11,
        JackSpades = 11,
        KingClubs = 13,
        KingDiamonds = 13,
        KingHearts = 13,
        KingSpades = 13,
        NineClubs = 9,
        NineDiamonds = 9,
        NineHearts = 9,
        NineSpades = 9,
        QueenClubs = 12,
        QueenDiamonds = 12,
        QueenHearts = 12,
        QueenSpades = 12,
        SevenClubs = 7,
        SevenDiamonds = 7,
        SevenHearts = 7,
        SevenSpades = 7,
        SixClubs = 6,
        SixDiamonds = 6,
        SixHearts = 6,
        SixSpades = 6,
        TenClubs = 10,
        TenDiamonds = 10,
        TenHearts = 10,
        TenSpades = 10,
        ThreeClubs = 3,
        ThreeDiamonds = 3,
        ThreeHearts = 3,
        ThreeSpades = 3
    }

    public void createDecks()
    {
        playerDeck = Enum.GetValues(typeof(Card)).Cast<Card>().ToList();
        computerDeck = Enum.GetValues(typeof(Card)).Cast<Card>().ToList();
        usedPlayerNrs = new List<int>();
        usedComputerNrs = new List<int>();
    }

    public Card getPlayerCard()
    {
        int index = random.Next(playerDeck.Count());
        usedPlayerNrs.Add(index);
        while (usedPlayerNrs.Contains(index))
        {
            index = random.Next(playerDeck.Count());
        }
        Card randomCard = playerDeck[index];
        return randomCard;
    }

    public Card getComputerCard()
    {
        int index = random.Next(computerDeck.Count());
        usedComputerNrs.Add(index);
        while (usedComputerNrs.Contains(index))
        {
            index = random.Next(computerDeck.Count());
        }
        Card randomCard = computerDeck[index];
        return randomCard;
    }

}

I need to get a card from a deck of cards but I am only allowed to get it once. However when I put my randomly generated numbers into an array to exclude that number from occurring again I still get the same cards. Have tried alot already that's why the codes a bit messy at this point.

if you apply a random permutation (that is a number of swap(random,random)) to a List containing all the cards and get the cards in order, you end up with a random card order — Exceptyon, Apr 14 '16 at 12:15
An enum should never have members with the same value. That's fundamentally broken! How would you know from the value what card it was meant to be?! — RB., Apr 14 '16 at 12:16
I recommend you model your `Card` as an immutable struct that has a suite (which can be an enum) and a rank (which would be another enum). Right now you cannot tell the difference between a King of Hearts or a King of Diamonds because they have the same value. — juharr, Apr 14 '16 at 12:18
@RB Why it is fundamentally broken? It is perfectly legal and makes sense in some cases, however this is not one of them. — lukbl, Apr 14 '16 at 12:27
@lukbl I call it fundamentally broken because it breaks the commutative relationship that ordinarily exists between an enum member and it's value. You can use it without that relationship existing, but I consider it a fundamental relationship, hence fundamentally broken. Hope that explains my usage of the term - I can see that you could have a different view, but I suspect/hope any disagreement would come down to semantics! — RB., Apr 14 '16 at 12:31
Side note: you have an array/list, [shuffle it](http://stackoverflow.com/questions/273313/randomize-a-listt-in-c-sharp) and pop the top off. Much easier. Also, you aren't going to spin useless cycles trying to get an unused card. — Clockwork-Muse, Apr 14 '16 at 12:39
I think someone else has exactly the same problem!: http://stackoverflow.com/questions/36623501/get-values-of-an-enum-variable-having-duplicate-values-with-different-keys — Sean, Apr 14 '16 at 12:55
Sorry for not replying until now but I've fixed the problem. It was just impossible with my current knowledge of c# to make it work with only one enumeration. I fixed it by using two enums like @juharr said and then using another class for creating my cards and putting them into a card array. Thanks for the help guys and if anybody wants I can put my code online. — Robin Landerloos, Apr 15 '16 at 16:28

Wicher Visser · Answer 1 · 2016-04-14T12:37:00.233

0

The reason you are getting the 'same result' is because you assigned identical values to multiple items in your enumeration. Instead, give each item a unique value.

public enum Card
{
   AceClubs = 11,
   AceDiamonds = 12,
   AceHearts = 13,
   AceSpades = ...,
}

In addition, you could check uniqueness of your sample by comparing the name of your enum item, not the value assigned to it. Like so:

var propertyName = typeof(Card).GetEnumName(Card.AceClubs);

edited Apr 14 '16 at 12:37

answered Apr 14 '16 at 12:21

Wicher Visser

1,513
12
21

An `enum` doesn't have an inherent name that you can check. You can use `Enum.GetName`, but that's going to take the `int` value and look it up. So if two `enum`s have the same value they will map back to the same name. – juharr Apr 14 '16 at 12:29
Well the problem is that the decks are used for a small game of "higher, lower". So the aces need to have the same value so that I can compare them with for example an eight of hearts to see which one is higher and which one is lower. I forgot to note that I'm still a student at my first year of basic c# programming so my knowledge isn't that great. – Robin Landerloos Apr 14 '16 at 12:32
@WicherVisser That only works if all the enums have unique values. – juharr Apr 14 '16 at 12:34
1

@RobinLanderloos That's why you should split up the suite and rank and make it a class or struct instead of an enum. – juharr Apr 14 '16 at 12:35

score 0 · Answer 2 · answered Apr 14 '16 at 12:34

The variables usedPlayerNrs and usedComputerNrs will always have unique values in it but playerDeck and computerDeck have duplicate values and you're fetching the card based on unique index as:

 Card randomCard = playerDeck[index]

since, playerDeck has duplicate values you're getting same card multiple times.

Carra · Answer 3 · 2016-04-14T12:43:06.823

You've got an error in here:

public Card getPlayerCard(List<int> usedPlayerNrs, List<Card> playerDeck)
{
    int index = random.Next(playerDeck.Count());
    while (usedPlayerNrs.Contains(index))
    {
        index = random.Next(playerDeck.Count());
    }
    usedPlayerNrs.Add(index);//Only add your index to the userplayednumbers here!
    Card randomCard = playerDeck[index];
    return randomCard;
}

Also, look up a shuffling algorithm, it will help with getting a random collection of cards in a more efficient way.

And as has been said, having multiple enums with the same value is a code smell. Are you sure you don't want to number them 1-52? Or go for an object oriented design and add a number + Spades/Diamonds/... enum to that class.

C# index number randomly generated and put into array to not occur again, still get the same result out of list with the number

3 Answers3

Linked