1

My code so far is working almost to how it should be working. The instructions are above the method. The only problem that I'm encountering is that Math.random(); repeats itself when called multiple times. I wanted to know if there was a solution to prevent the Math.random(); from repeating itself;

/**
 * Apply an "efficient selection shuffle" to the argument.
 * The selection shuffle algorithm conceptually maintains two sequences
 * of cards: the selected cards (initially empty) and the not-yet-selected
 * cards (initially the entire deck). It repeatedly does the following until
 * all cards have been selected: randomly remove a card from those not yet
 * selected and add it to the selected cards.
 * An efficient version of this algorithm makes use of arrays to avoid
 * searching for an as-yet-unselected card.
 * @param values is an array of integers simulating cards to be shuffled.   
 */
public static void selectionShuffle(int[] values) {
    ArrayList<Integer> temp=new ArrayList<Integer>();
    int size=52;
    for(int j=0;j<size;j++){
        /*int random=(int)(Math.random()*51);
        temp.add(random);
        values[j]=temp.get(j);*/
        int random=(int)(Math.random()*51);
        temp.add(values[random]);
        values[j]=temp.get(j);
    }
}
Tariq Al-Attrash
  • 99
  • 1
  • 1
  • 7
  • `Math.random()` will run 52 times since you have placed it in the loop. It will run everytime you call the method. Is that your issue? – Mathews Mathai Feb 19 '16 at 06:43
  • Or do you mean the value returned by `Math.random()` ? – Mathews Mathai Feb 19 '16 at 06:45
  • I suggest you try to implement the comment above your method. Your current code doesn't do that. (I'd also suggest using the `Random` class instead of `Math.random`.) – Jon Skeet Feb 19 '16 at 06:45
  • You could use create `Random random = new Random();` outside of the loop, and then call `random.nextDouble()` inside. – Enigo Feb 19 '16 at 06:45
  • Tbh you should rather use the `Math.random` to **swap** random elements. Not to pick a random one and put it in the temp array, because then you'll surely have some duplicates... – radoh Feb 19 '16 at 06:45
  • Given a completely random distribution in the range [0..50] it is quite likely that you'll get duplicates. – Jim Garrison Feb 19 '16 at 06:46
  • http://stackoverflow.com/questions/8115722/generating-unique-random-numbers-in-java – user1933888 Feb 19 '16 at 06:47
  • @Enigo that is not a problem, `randomGenerator` is defined as static in `Math`. – radoh Feb 19 '16 at 06:47
  • See http://stackoverflow.com/questions/1519736/random-shuffling-of-an-array/1520212#1520212 for a *real* implementation of the algorithm described. – Jon Skeet Feb 19 '16 at 06:48
  • My code is supposed to run 52 times its supposed to shuffle a deck of cards. The code is supposed to take a random index out of values[] and put it in temp and then back into values[] shuffled. – Tariq Al-Attrash Feb 19 '16 at 06:48
  • @TariqAl-Attrash Try `Collections.shuffle(Arrays.asList(values));` – Pragnani Feb 19 '16 at 06:53
  • I guess what I'm asking is how can I store the number generated from the Math.random() temporarily, and then test it against the next number produced by the Math.random() – Tariq Al-Attrash Feb 19 '16 at 06:54
  • We're not allowed to use Collections.shuffle(); :D – Tariq Al-Attrash Feb 19 '16 at 06:55
  • 1
    @PragnaniKinnera, that won't work since `values` is a primitive array. If you have Guava, you could do `Collections.shuffle(Ints.asList(values))`. – shmosel Feb 19 '16 at 06:56
  • Please check my solution to ensure if that's what you want. – Mathews Mathai Feb 19 '16 at 07:27

6 Answers6

3

If you want the integers 0-51 in a random order without repeats:

  • Add those numbers to a list, in order
  • Call Collections.shuffle on that list

Simply using (int) (52 * Math.random()) 51 times will almost guarantee that some numbers will be repeated (and, of course, some will be missing), according to the birthday paradox.

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
0

From what I can tell, you believe that Math.random is "repeating" because you always choose a new number between 0 and 51 to add to your values array. And you are overwriting the early part of the values array before you might have the chance to select those numbers.

Think about the process as outlined in the instructions. If you're selecting from the initial array once per iteration, and moving those selections into the new temp array, then shouldn't values be getting smaller every time? Maybe you don't always want a random number from 0-51.

And ignore everyone who is telling you different ways to do the shuffle. Obviously you're trying to implement the algorithm as given for coding practice.

audiodude
  • 1,865
  • 16
  • 22
0

Not sure what your issue is but try to use java.util.Random instead.

public static void selectionShuffle(int[] values)
{
    ArrayList<Integer> temp=new ArrayList<Integer>();
    Random randomGenerator = new Random();

    int size=52;

    for(int j=0;j<size;j++){
        // If you wan to generate a random number up to a maximum value,
        // use randomGenerator.nextInt(maximum) instead
        int random = randomGenerator.nextInt() * 51;
        temp.add(values[random]);
        values[j]=temp.get(j);
    }
}
Falla Coulibaly
  • 759
  • 2
  • 11
  • 23
0

When numbers are random, every number has an equal chance of occurring, not matter what has occurred previously. This means random number must be able to repeat themselves or they no longer truly random.

If you want a set of numbers, cards, songs to appear in a random order without repeating, this is called a shuffle. Note: the last number, card, or song won't be random at all because it will be the only one left.

With a random sequence you can even get the same number appearing many times.

http://vanillajava.blogspot.co.uk/2011/10/randomly-no-so-random.html

Random random = new Random(441287210);
for (int i = 0; i < 10; i++)
    System.out.print(random.nextInt(10)+" ");

prints

1 1 1 1 1 1 1 1 1 1

and

Random random = new Random(-6732303926L);
for(int i = 0; i < 10; i++)
    System.out.print(random.nextInt(10)+" ");

prints

0 1 2 3 4 5 6 7 8 9
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
0

From what I understood, you don't want Math.random() to return a number more than once. If that's the issue, what we are going to do is allow Math.random() to return whatever it likes but we will use only the non-repeated numbers.

Try this:

public static void selectionShuffle(int[] values) {
    ArrayList<Integer> temp=new ArrayList<Integer>();
    int size=52;
    Integer a[]=new Integer[size];
    for(int j=0;j<size;j++){
        int random=(int)(Math.random()*51);
        if(a.[random]==null) //to ensure that only unique nos. are used.
        {
        temp.add(values[random]);
        values[j]=temp.get(j);
        a.[random]=1;
        // the above code will only be executed if `a[random]` is null
        //ensures that this executes only when unique number is generated
    }
    else
    {
      j--; 
    //so that 52 iterations occur successfully. We don't want any iteration to go waste.
    }
 }

I have used an Integer array to ensure that unique numbers are used. Each time unique number is generated, we are assigning 1 or some other integer to that particular index of the Integer array. Your dependent code will only be executed if the a[]'s value is not set at the index corresponding to the generated integer number that is random .

Mathews Mathai
  • 1,707
  • 13
  • 31
0

You want to do a random draw without replacement but you have implemented a random draw with replacement...

Put the cards in an array of size 52 then in your first draw do something like:

(int)(Math.random()*51 + 1);

Which gives you a random number between 1-52, say you get 4, then update the array by removing the 4th element leaving you with an array of size 51. Then make your second draw:

(int)(Math.random()*50 + 1);

And so on...

Jimmy
  • 187
  • 7