Esben86
Esben86

Reputation: 493

Manual list shuffling in Java produces same result each time

I am trying to create a method for shuffling a list manually. The list is shuffled, but the same result is produced every time. Code example below:

package ch11;
import java.util.ArrayList;
import java.util.Arrays;
public class Chapter_11_E07_ShuffleArrayList {

    public static void main(String[] args) {

        Integer[] array = {1, 2, 3, 4, 5, 6, 7, 8};

        ArrayList<Integer> intList = new ArrayList<>(Arrays.asList(array));

        System.out.println("Before shuffle: ");
        for (Integer x: intList)
            System.out.print(x + " ");

        shuffle(intList);

        System.out.println("\nAfter shuffle: ");
        for (Integer x: intList)
            System.out.print(x + " ");

    }

    public static void shuffle(ArrayList<Integer> intList) {
        // Simple solution
        // java.util.Collections.shuffle(intList);

        // Manual shuffle
        for (Integer x: intList) {

            int newIndex = (int) Math.random() * intList.size();

            Integer temp = intList.get(x);
            intList.set(x, intList.get(newIndex));
            intList.set(newIndex, temp);    
        }   
    }

}

It seems to work to some extent, but is Math.random * intList.size() producing the same random index every time? Inputs are highly appreciated!

Upvotes: 2

Views: 950

Answers (2)

Martin
Martin

Reputation: 239

To show what I've meant in the comments above, here the code:

for (Integer x: intList) {
    int newIndex = (int) (Math.random() * intList.size());
    int oldIndex = intList.indexOf(x);
    Integer temp = intList.get(newIndex);
    intList.set(newIndex, x);
    intList.set(oldIndex, temp);    
}

Upvotes: 1

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726489

This is because

int newIndex = (int) Math.random() * intList.size();

is not parenthesized properly. It should be

int newIndex = (int)(Math.random() * intList.size());

To avoid simple errors like this, make new Random object, and call nextInt(intList.size()).

Upvotes: 12

Related Questions