Sign in to follow this  

Card Shuffle - Opinions

This topic is 1955 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I would like some general opinions on this shuffling technique I've implemented on my Deck class. Here's what happens:[list]
[*]A base deck is created with an array.
[*]The base deck is copied to a new list.
[*]A secondary list is created.
[*]One card at a time is swapped from the copied list to the new list via random number.
[*]The card is returned to a new slot via random number.
[/list]
I was going to better define a random number, but I would rather hear some thoughts before I proceed.

[code]
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace Cards
{
class Deck
{
Random rand;
Card[,] baseDeck;
string[,] str;
int suit;
int value;
public enum Suit
{
Spades, Diamonds, Clubs, Hearts
}
public enum Value
{
Ace, Two, Three, Four, Five, Six,
Seven, Eight, Nine, Ten, Jack,
Queen, King
}
public void CreateBaseDeck()
{
str = new string[4, 13];
baseDeck = new Card[4, 13];
suit = 0;
value = 0;
foreach (string s in Enum.GetNames(typeof(Suit)))
{
foreach (string v in Enum.GetNames(typeof(Value)))
{
baseDeck[suit, value] = new Card(s, v);
value++;
}
value = 0;
suit++;
}
suit = 0;
}
public void ShuffleDeck()
{
#region CopyBaseDeck
List<Card> newDeck = new List<Card>(52);
for (int i = 0; i < newDeck.Capacity; i++)
{
newDeck.Add(baseDeck[suit, value]);
value++;
if (value == 13)
{
value = 0;
suit++;
}
}
suit = 0;
#endregion
// List<Card> newDeck;
List<Card> shuffle = new List<Card>(1);
int seed = 25565;
for (int i = 0; i < 2500; i++)
{
rand = new Random(seed);
int randomIndex1 = rand.Next(0, 52);
int randomIndex2 = rand.Next(0, 52);
shuffle.Add(newDeck[randomIndex1]);
newDeck.Remove(newDeck[randomIndex1]);
newDeck.Insert(randomIndex2, shuffle[0]);
shuffle.Clear();
seed++;
}
Console.Clear();
int index = 0;
foreach(Card s in newDeck)
{
Console.WriteLine("{0} \tof {1}.", newDeck[index].value, newDeck[index].suit);
index++;
}
}
}
}
[/code]

[code]
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace Cards
{
class Card
{
public string suit { get; set; }
public string value { get; set; }
public Card(string suit, string value)
{
this.suit = suit;
this.value = value;
}
}
}
[/code]

Share this post


Link to post
Share on other sites
Picking random numbers to move an element from the original array into a shuffled array seems to be a pretty standard approach.

You'll find background info under "random permutation algorithm", such as http://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle

Share this post


Link to post
Share on other sites
There are a few problems with your shuffle:
#1, Use a unique value for the seed. Typically, the current time in seconds is used as the seed value, since it should be different every time you run the program.
#2, Only seed and create your rand variable once. What you are doing, you'll get the same random values every time since you're using the same seed every loop.
#3, you don't need newDeck nor Shuffle list. Something like this should suffice:
[code]
randIndex = rand.Next(0, 52);
Card tempCard = baseDeck[randIndex];
baseDeck.Remove(tempCard);
baseDeck.Insert(rand.Next(0, 51), tempCard);
[/code]
No need for copying lists, or any of that.

Good Luck!

EDIT: I realized, I need to insert via rand.Next(0, 51), not 52 (as, after the removal, there's only 51 cards) Edited by BeerNutts

Share this post


Link to post
Share on other sites
There is an excellent StackOverflow answer for shuffling an IList<T>.

[url="http://stackoverflow.com/questions/273313/randomize-a-listt-in-c-sharp"]Randomize a list in C#[/url]

Some important takeaways:[list]
[*]As already mentioned, you can do inline replacement. No need for a new list.
[*]Random may not be random enough. ([url="http://blog.thijssen.ch/2010/02/when-random-is-too-consistent.html"]When random is too consistent...[/url])
[*]Extension methods rock!
[/list]

Share this post


Link to post
Share on other sites
I was aware of the static seed number a few moments after I posted this, but here is the code for the new shuffling algorithm:

[code]
rand[0] = new Random();
List<Card> swap = new List<Card>(1);
int seed = rand[0].Next();
int[] randomIndex;
for (int i = 0; i < 2500; i++)
{
rand[1] = new Random(seed);
randomIndex = new int[2]{rand[1].Next(newDeck.Count), rand[1].Next(newDeck.Count - 1)};
swap.Add(newDeck[randomIndex[0]]);
newDeck.Remove(newDeck[randomIndex[0]]);
newDeck.Insert(randomIndex[1], swap[0]);
swap.Clear();
seed++;
}
[/code]

Notes:[list]
[*]I was later going to try out BeerNuts method so I wouldn't have to create a new list.
[*]This algorithm allows a new random number per loop...I think. :/ I tested it and it seemed correct.
[/list] Edited by Frogggg

Share this post


Link to post
Share on other sites
[quote name='Frogggg' timestamp='1344556177' post='4967942']
I was aware of the static seed number a few moments after I posted this, but here is the code for the new shuffling algorithm:

[code]
rand[0] = new Random();
List<Card> swap = new List<Card>(1);
int seed = rand[0].Next();
int[] randomIndex;
for (int i = 0; i < 2500; i++)
{
rand[1] = new Random(seed);
randomIndex = new int[2]{rand[1].Next(newDeck.Count), rand[1].Next(newDeck.Count - 1)};
swap.Add(newDeck[randomIndex[0]]);
newDeck.Remove(newDeck[randomIndex[0]]);
newDeck.Insert(randomIndex[1], swap[0]);
swap.Clear();
seed++;
}
[/code]

Notes:[list]
[*]I was later going to try out BeerNuts method so I wouldn't have to create a new list.
[*]This algorithm allows a new random number per loop...I think. :/ I tested it and it seemed correct.
[/list]
[/quote]

Frogggg, you misunderstand how Random works. You only need 1 rand class variable, [b]for your whole program.[/b] Everytime you call rand.Next(0, X), you'll get a random number from 0 to X-1. You don't need to call new Random every loop. Simple seed it once outside the loop with the current tick value, then just use that variable to get a new random number every time. It doesn't need to be an array, and you don't need to create an array to get the random values for removal and insertion.

Actually, in C#, calling rand = new Random(), it automatically creates the Random class with a time-dependent seed value, so you shouldn't be passing in a seed value UNLESS you specifically want to create a repeatable pseudo-random sequence (which, in this case, and most cases, you do not). Here's what MSDN says:
[b]Random() Initializes a new instance of the Random class, using a time-dependent default seed value.[/b]

[b]The distribution of the generated numbers is uniform; each number is equally likely to be returned.[/b]
[b]The default seed value is derived from the system clock and has finite resolution. As a result, different Random objects that are created in close succession by a call to the default constructor will have identical default seed values and, therefore, will produce identical sets of random numbers. This problem can be avoided by using a single Random object to generate all random numbers.[/b]

Like this (Note, I don't know C#, so I may make a syntactical error)
[code]
// create random class variable (will be seeded with time dependent value by default)
rand = new Random();

// I use 52, you can use more if you want, but it won't be much difference
for (int i = 0; i < 52; i++) {
// Select a card randomly from the full deck
Card tempCard = newDeck[rand.Next(0, 52)];
// Remove that Card
newDeck.Remove(tempCard);
// Re-Insert that card into a random location in the deck
newDeck.Insert(rand.Next(0, 51), tempCard);
}
[/code]

Everything else you are doing is just adding unnecessary complexity and inefficiency.

Good Luck! Edited by BeerNutts

Share this post


Link to post
Share on other sites

This topic is 1955 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this