0
public static void selectionShuffle(int[] values) {
    Random rand = new Random();
    for(int i = values.length; i > 0; i--) {
         int r = rand.nextInt(i); 
         swap(values, r, i);
    }
//updated answer to include below method
public static void swap(int[]a, int i, int j){
    int temp = a[i];
    a[i] = a[j];
    a[j] = temp;
}

I am writing a card game and need to randomly sort the cards in place, I got an ArrayOutOfBoundsException when I ran the above code, with the compiler complaining particularly about the reassignment from values[i] = values[r] . To be clear, this is , not intended to be a selection sort but instead a selection shuffle. Any help is appreciated!

tobydank
  • 13
  • 4
  • 1
    `values[i]` should be `values[i-1]` – Iłya Bursov Feb 22 '19 at 01:26
  • Are you sure that the `selectionShuffle` function can really shuffle the cards? And maybe you can find some solutions in https://stackoverflow.com/questions/1519736/random-shuffling-of-an-array – gaoxinge Feb 22 '19 at 01:31

2 Answers2

0

Your start condition needs to be values.length-1, as Java arrays are 0 to length-1.

Also the test should be >=0, since as you wrote it'll only go down to index 1.

for(int i = values.length - 1; i >= 0; i--) {

Or as pointed out by Iłya Bursov, you could compare at index i-1 and leave your for loop conditions as you have them. Not sure what's the best form I always do it as in my example above.

Also, you aren't shuffling. You set the value at index i to the one at r, but you lost the value that was previously at i. You need a temp variable to swap the values.

tmp = values[i];
values[i] = values[r];
values[r] = tmp;
Jeffrey Blattman
  • 21,054
  • 8
  • 74
  • 127
0

Alternatively, you may try Collections.shuffle() but it would be more handy if you have to use the wrapper class Integer instead of the primitive type intat first.

Integer[] version:

public static void selectionShuffle(Integer[] values) {
    Collections.shuffle(Arrays.asList(values));
}

int[] version:

public static void selectionShuffle(int[] values) {
     Integer[] boxedArr = Arrays.stream(values).boxed().toArray( Integer[]::new );
     Collections.shuffle(Arrays.asList(boxedArr));
     for(int i=0 ; i<values.length ; i++){
         values[i] = boxedArr[i];
     }
 }
jackycflau
  • 994
  • 1
  • 10
  • 30