Basic playing card generation and shuffle

This is my first attempt at Java so I appreciate any criticism or pointers. The program should generate a full set of given decks and then shuffle them.

```
import java.util.Scanner;
import java.util.Arrays;
import java.util.Random;

public class shuffle {
//Make it easier to find array items
static int value = 0;
static int suit = 1;
static int remaining = 2;

public static void main(String Args[]) {
int[][] deck;

String clabel;
//number of decks to generate
int deckcount = 1;
int decksize = deckcount * 52;

deck = new int[decksize][2];
//loop through deck building until deck reaches card count
for (int c = 0; c 10 || card[value] == 1) {
switch (card[value]) {
case 1:
v_name = "ACE";
break;
case 11:
v_name = "Jack";
break;
case 12:
v_name = "Queen";
break;
case 13:
v_name = "King";
break;
}
} else {
v_name = "" + card[value];
}
c_label = v_name + " of " + s_name;

return c_label;

}
//shuffle deck
public static int[] shuffle(int size) {
Random rgen = new Random(); // Random number generator
int[] order;
order = new int[size];

for(int i = 0; i < order.length; i++) {
order[i] = i;
}

for (int i = 0; i < order.length; i++) {
int randomPosition = rgen.nextInt(order.length);
int temp = order[i];
order[i] = order[randomPosition];
order[randomPosition] = temp;
}

return order;
}
private static void pressAnyKeyToContinue() {
System.o

First off I want to say that your decision to make an array that represents the ordering permutation is a really good idea. Many people would have passed in the array to be shuffled and had it mutated. Returning a permutation and applying that to an ordered array is a conceptually much nicer way to go.

Your shuffle algorithm has an interesting and common bug. Your shuffle algorithm is "generate the vector {0, 1, 2, 3, 4, ..., 51}. For each position 0 through 51, swap the current contents with a randomly-generated position".

Let's do a little math. Suppose we have only three cards, to make the math easier. How many decks are there? Six:

• 0, 1, 2

• 0, 2, 1

• 1, 0, 2

• 1, 2, 0

• 2, 0, 1

• 2, 1, 0

Each deck should be as likely as any other.

How many paths through the algorithm are there? Twenty-seven, each equally likely:

• Swap 0 with 0, swap 1 with 0, swap 2 with 0

• Swap 0 with 1, swap 1 with 0, swap 2 with 0

• Swap 0 with 2, swap 1 with 0, swap 2 with 0

• Swap 0 with 0, swap 1 with 1, swap 2 with 0

• Swap 0 with 1, swap 1 with 1, swap 2 with 0

• Swap 0 with 2, swap 1 with 1, swap 2 with 0

• Swap 0 with 0, swap 1 with 2, swap 2 with 0

• Swap 0 with 1, swap 1 with 2, swap 2 with 0

• Swap 0 with 2, swap 1 with 2, swap 2 with 0

... I won't list all of them.

Now we have 6 possible outcomes and 27 paths through the algorithm, so plainly some of the outcomes must be produced more often than the others. There is no way to divide 27 paths into 6 sets such that each set has the same number of elements.

The same is true with your shuffle; there are \$52!\$ possible decks and \$52^{52}\$ paths through the code, and those don't divide evenly into each other.

What you've done is the most common incorrect implementation of the Knuth Shuffle. Now go do some research and figure out how to fix your bug; it's an easy fix.

Once you've done that, you've still got a problem. There are 52! possible decks but only \$2^{32}\$ possible seeds to Random, so you are still not sampling the space of all possible decks fairly. If you care about this, you should probably use a crypto-strength randomness generator.

You might be interested in my articles on these subjects.

• http://ericlippert.com/2013/04/15/producing-permutations-part-one/

• http://ericlippert.com/2014/10/13/producing-combinations-part-one/

Other problems with your code:

int[][] deck;

What the heck? A deck of cards is an array of arrays of integers? That's craziness. I want to see

Deck deck = new Deck();

Programming computers is the art of creating meaningful abstractions. A deck is not an array of arrays of integers. A deck is a deck. Make a class that represents the concept of deck that abstracts away this mechanism.

String clabel;

We pronounce that "klabel" of course.

Seriously, typing out the remaining letters of a word will not cause you to die younger.

Later...

String c_label, s_name = "", v_name = "";

Oh, it's c_label now when it was clabel before. Now would be a good time to get into good habits. I don't care what your naming strategy is, so long as it is (1) comprehensible, and (2) consistent. Your strategy is neither. My preference as a C# programmer is

cardLabel

but

card_label

is idiomatic in Java. Better entirely though would be to make a Card type that has a label getter.

order = new int[decksize];
        order = shuffle(decksize);

If you saw

x = 1;
x = 2;

you would immediately note that the first line is pointless. But your code is equally pointless for exactly the same reason.

Something made you think that you needed to initialize order to a new array immediately before throwing it away, just like the contents of x are thrown away. What incorrect process of thought led you to this conclusion? Can you examine that process?

These are not rhetorical questions. I analyze these sorts of programming errors for a living and I am very interested in the beginner-mind psychology that leads to them. Any insights would be appreciated.

WHY IS "ACE" IN ALL CAPS? It seems weird.

StackExchange Code Review Q#79759, answer score: 11