Swapping integer array values in a for loop - Is it possible?

Question

So I decided to create a simple card deck shuffling program, nothing fancy just a single array with 52 "cards" numbered from 0 to 52. I'm not necessarily sure if it is the best method.

I created a random integer method (randInt) to get random numbers and a shuffling method (shuffle) which gets two random cards and swaps their values.

My method is simple enough but when I run the code (and some debug code I added) I seem to get each value becoming the exact same value (d[2] = d[1] = randCard1) which results in multiples of the same card and obviously other values completely disappearing. Given enough shuffles the whole deck would likely become 52 cards all consisting of the same value.

I have read some questions about swapping integer arrays but my question is different as it is about the issue of using a for loop, (as my method of swapping array values is similar to those questions already, but final/static keywords aren't helpful when I am modifying a few times I have concluded that for loops are to blame) which apparently prevents the arrays from updating properly.

Is this the case if so how do I over come this and if not what am I missing? Is there another problem with my way of swapping integer array values?

Here is my code:

import java.util.Random;

public class Cards {

    public static void main(String[] args) {

        int[] deck = new int[] {
            0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
            10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
            20, 21, 22, 23, 24, 25, 26, 27, 28, 29,
            30, 31, 32, 33, 34, 35, 36, 37, 38, 39,
            40, 41, 42, 43, 44, 45, 46, 47, 48, 49,
            50, 51
        };

        printArray(deck, "Original Deck:");
        deck = shuffle(deck);
        printArray(deck, "Shuffled Deck:");
        }

        public static int[] shuffle(int[] d) {
            //shuffle between 100 to 200 to ensure a thorough shuffle
            int randLoops = randInt(100, 200);
            int randCard1;
            int randCard2;

        for (int i = 0; i <= randLoops; i++) {
            int temp;

            randCard1 = randInt(0, 51);

            do {randCard2 = randInt(0, 51);}
            while (randCard2 == randCard1);

            System.out.print("*" + randCard1 + "|" + randCard2 + "/");
            temp = randCard1;
            d[randCard1] = d[randCard2];
            d[randCard2] = d[temp];
            System.out.println(d[randCard1] + "|" + d[randCard2] + "*");
        }
        return d;
    }

    public static int randInt(int min, int max) {
        Random rand = new Random();    
        int randomNum = rand.nextInt((max - min) + 1) + min;

        return randomNum;
    }

    public static void printArray(int[] d, String t) {
        System.out.println(t);
        for (int i = 0; i < d.length; i++) {
                System.out.print(d[i] + ", ");
        }

        System.out.println();
    }
}

Here is an example of my output with debugging code (hopefully it helps and doesn't hinder):

Original Deck:
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 
*40|41/41|41*
*29|40/41|41*
*19|26/26|26*
*2|27/27|27*
*4|30/30|30*
*24|25/25|25*
*7|1/1|1*
*17|47/47|47*
*14|5/5|5*
*42|11/11|11*
*23|42/11|11*
*33|30/30|30*
*19|42/9|9*
*11|26/9|9*
*40|21/9|9*
*36|11/9|9*     <--------- at the end I don't know why the values = 9?
Shuffled Deck:
9, 11, 30, 11, 30, 30, 31, 45, 11, 11, 51, 9, 11, 45, 45, 45, 51, 11, 9, 9, 44, 9, 30, 44, 27, 11, 9, 27, 31, 11, 11, 21, 11, 11, 45, 35, 9, 11, 11, 30, 9, 11, 9, 11, 21, 31, 9, 30, 45, 30, 11, 45, 
BUILD SUCCESSFUL (total time: 2 seconds)

You will get a better, more uniform, shuffle if you use Fisher-Yates shuffle — Patricia Shanahan
– Patricia Shanahan, Commented Apr 28, 2015 at 20:18
I would put it in a list and sort it with a random comparator: Collections.sort(deck, new RandomComparator()) where the comparators compare function is just return rand.nextInt(2) - 1; or something like that. — justin.m.chase
– justin.m.chase, Commented Apr 28, 2015 at 20:31
@justin.m.chase Collections.sort is dependent on getting consistent results from the Comparator. It assumes transitivity: A belongs before B and B belongs before C implies A belongs before C. Can you prove it will result in equal probability of all shuffles? — Patricia Shanahan
– Patricia Shanahan, Commented Apr 28, 2015 at 22:55
@justin.m.chase If the objective is a 1-line shuffle, I would use Collections.shuffle. If the objective is to roll-your-own, the OP is almost there - the code could be modified very slightly to get a nice, provably correct Fisher-Yates shuffle. — Patricia Shanahan
– Patricia Shanahan, Commented Apr 28, 2015 at 23:50

JB Nizet · Accepted Answer · 2015-04-28 20:15:46Z

6

This is where the problem lies:

temp = randCard1;
d[randCard1] = d[randCard2];
d[randCard2] = d[temp];

Instead of saving the card value in temp, you're saving its index. You need instead

temp = d[randCard1];
d[randCard1] = d[randCard2];
d[randCard2] = temp;

Note that the JDK has a shuffle method:

// create the deck
Integer[] deck = new Integer[52];
for (int i = 0; i < deck.length; i++) {
    deck[i] = i;
}

// shuffle it
Collections.shuffle(Arrays.asList(deck));

answered Apr 28, 2015 at 20:15

JB Nizet

694k94 gold badges1.3k silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Daedric Over a year ago

OMG you are the best thank you. I feel like an idiot now lol.

Patricia Shanahan · Accepted Answer · 2015-04-29 04:34:35Z

This is really an extended comment on the pitfalls of rolling one's own shuffle algorithm and the need for testing. I applied Collections.shuffle, which uses a Fisher-Yates shuffle, to a four element List. With only 24 possible outcomes, I could count the number of instances of each during a test:

[1, 2, 0, 3] 4169320
[0, 3, 1, 2] 4169046
[0, 2, 1, 3] 4166184
[1, 3, 0, 2] 4166060
[2, 1, 0, 3] 4166150
[2, 3, 0, 1] 4169336
[0, 3, 2, 1] 4169641
[0, 1, 2, 3] 4165100
[2, 0, 1, 3] 4168724
[3, 2, 0, 1] 4169374
[2, 3, 1, 0] 4165930
[3, 1, 0, 2] 4167949
[1, 3, 2, 0] 4165973
[0, 2, 3, 1] 4165752
[1, 0, 2, 3] 4167152
[0, 1, 3, 2] 4164295
[3, 2, 1, 0] 4163534
[3, 0, 1, 2] 4165647
[1, 0, 3, 2] 4168772
[1, 2, 3, 0] 4163711
[3, 1, 2, 0] 4163498
[2, 0, 3, 1] 4166876
[2, 1, 3, 0] 4162543
[3, 0, 2, 1] 4169433

I did the same thing, using the sort with random comparator idea suggested in comments on the question. The original order, and its reverse, both appeared far more often than they should. Other results appeared much less frequently. The important point is not the particular algorithm, but that any shuffle algorithm, even if it sounds plausible, needs to be analyzed and tested.

[1, 3, 0, 2] 3123183
[0, 3, 1, 2] 6248696
[0, 2, 1, 3] 1561055
[1, 2, 0, 3] 1564132
[2, 1, 0, 3] 4686711
[2, 3, 0, 1] 1562854
[0, 1, 2, 3] 18750411
[0, 3, 2, 1] 1562861
[2, 3, 1, 0] 4687112
[2, 0, 1, 3] 1562640
[3, 2, 0, 1] 1562653
[3, 1, 0, 2] 3123363
[0, 1, 3, 2] 6251613
[1, 0, 2, 3] 3125297
[1, 3, 2, 0] 1561043
[0, 2, 3, 1] 1564091
[3, 2, 1, 0] 17183252
[3, 0, 1, 2] 6251310
[1, 2, 3, 0] 1563693
[1, 0, 3, 2] 3126874
[2, 1, 3, 0] 4685909
[3, 1, 2, 0] 1565535
[2, 0, 3, 1] 1562618
[3, 0, 2, 1] 1563094

Here is my test program:

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Random;

public class Test {
  public static void main(String args[]) {
    Map<List<Integer>, Integer> resultMap = new HashMap<List<Integer>, Integer>();
    List<Integer> initial = new ArrayList<Integer>();
    initial.add(0);
    initial.add(1);
    initial.add(2);
    initial.add(3);
    for(int i=0; i<100000000; i++){
      List<Integer> working = new ArrayList<Integer>(initial);
      shuffle(working);
      //Collections.shuffle(working, rand);
      Integer oldCount = resultMap.get(working);
      if(oldCount == null){
        resultMap.put(working, 1);
      } else {
        resultMap.put(working, oldCount+1);
      }
    }
    for(Map.Entry<List<Integer>, Integer> entry : resultMap.entrySet()){
      System.out.println(entry.getKey().toString()+" "+entry.getValue());
    }
  }

  static Random rand = new Random(3);
  static Comparator<Integer> comp = new Comparator<Integer>() {
    @Override
    public int compare(Integer arg0, Integer arg1) {
      return rand.nextInt(2) - 1;
    }
  };

  static void shuffle(List<Integer> in) {
    Collections.sort(in, comp);
  }
}

Collectives™ on Stack Overflow

Swapping integer array values in a for loop - Is it possible?

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related