2

What Im trying to do is take my array coins[]. And basically rearrange each coin to a different position. This is what i have so far. When I do this though, nothing happens. Meaning all the values stay the same. Except for the last one. That one changes. 

public void shake() 
{
    for (int i = 0; i < coins.length; i++)
    {
        int index = Coin.RANDOM.nextInt(coins.length);
        Coin temp = coins[index];
        coins[index] = coins[i];
        coins[i] = temp;

        System.out.print(coins[i] + ", ");
    }
}

I instantiate random like this:

public static long SEED = System.currentTimeMillis();
public static Random RANDOM = new Random(SEED);
Marcus Scaffidi
  • 77
  • 2
  • 9
  • Can you post the input example, and the output that it provided. Preferably several tries – Cruncher Sep 24 '13 at 20:38
  • How do you instantiate `Coin.RANDOM`? – AxiomaticNexus Sep 24 '13 at 20:40
  • Even if this worked the way you wanted, wouldn't it just shuffle with the first 11 items? Should it be .nextInt(coins.length)? – jwl Sep 24 '13 at 20:41
  • @Marcus edit your post. Don't post in comments. – Prateek Sep 24 '13 at 20:43
  • Thanks joe i fixed that, and Prateek I didnt think it mattered. @YasmaniLlanes Added the coin.random instantiation. – Marcus Scaffidi Sep 24 '13 at 20:45
  • why are you printing value at `swap` instead of `i`? Any specific reason? – Prateek Sep 24 '13 at 20:48
  • No actually.. lol. thanks. that gave me a very different output actually to. But what its doing now. is copying coins it moves and duplicates them. – Marcus Scaffidi Sep 24 '13 at 20:52
  • Note that `java.util.Collections` has a `shuffle()` method which does what you want your `shake()` method to do, but it works on lists. You can use it on an array like this: `Collections.shuffle(Arrays.asList(coins));` – Jesper Sep 24 '13 at 20:55

5 Answers5

2

Please notice that this line

System.out.print(coins[swap] + ", ");

displays the already moved (swapped) coin. Maybe you were thinking about displaying the new coin at i index: coins[i] (which wouldn't be correct anyway, as the already displayed coin still can be swapped in the next iterations). Probably it's better to create a second for loop to display final coin values.

But this isn't only problem here. To randomly shuffle an array you should use Fisher-Yates algorithm which is slightly different than your method. You can find Java implementation of this algorithm on SO.

If you had a List<Coin> instead of Coin[] (list instead of array) you could use the Collections.shuffle method and be sure that the algorithm is correct and you'll always get random result.

Community
  • 1
  • 1
Tomek Rękawek
  • 9,204
  • 2
  • 27
  • 43
  • Okay I changed mine to represent that, now the only problem, is I have coins duplicating themselves. – Marcus Scaffidi Sep 24 '13 at 21:10
  • If you have an array, you only have to type a few more characters: Collections.shuffle(Arrays.asList(coins)) – lbalazscs Sep 24 '13 at 21:16
  • Well I still cannot use that I dont believe. Im pretty sure I have to create my own shake method. – Marcus Scaffidi Sep 24 '13 at 21:23
  • Even if you implement your own shake method I would strongly suggest to try out `Collection.shuffle` and learn how you can incorporate it for your usage. – Prateek Sep 24 '13 at 21:32
1

As you are using swap as index with which you will be swapping the current value you can edit your Random number generator to generate random numbers between certain range (say 0 - coins.length) and then you can change your implementation to something like this

public void shake() 
{

    Coin temp;

    for (int i = 0; i < coins.length; i++)
    {
        //int swap = Coin.RANDOM.nextInt(coins.length);
        temp = coins[swap];
        coins[swap] = coins[i];
        coins[i] = temp;

        System.out.print(coins[i] + ", ");
    }
}

For the commented line in your code check THIS to update your random number generator to generate numbers between two values. Then each time you generate swap(index) between i+1 - coins.length and continue this till you fully exhaust the array. This ensures that you don't make a swap at the index the value for which you have already displayed. But I am not completely confident that this would indeed be a random shuffle as in the beginning of the loop you have more choices for the swap index then you would have sometime later in the loop and the shake is not completely random. This solution is only in case you want to strictly implement your own shake method without using the Collections.shuffle as @Tomek mentioned.

Community
  • 1
  • 1
Prateek
  • 1,916
  • 1
  • 12
  • 22
1

why don't you using Collections? its so simple to assign random indexes to each value in array or ArrayList.

Collections.shuffle(coins);//if coins is array
Collections.shuffle(Arrays.asList(coins));//if coins is an ArrayList
Ehsan Rafique
  • 107
  • 1
  • 7
0

You might use Knuth's shuffling algorithm which rearranges the array so that a result is a uniformly random permutation. Algorithm is simple but works like a charm:

  1. Iterate over array and in iteration i pick random integer swap between 0 and i
  2. Swap array[i] and array[swap]

Note that in your implementation random is generated between 0 and 11, which doesn't seem to produce good shuffling.

Here is a code example with shuffling for array of integers:

import java.util.Random;

public class Test {

public static long SEED = System.currentTimeMillis();
public static Random RANDOM = new Random(SEED);

public static void shuffle(int[] numbers)
{
    for (int i = 0; i < numbers.length; i++)
    {
        int swap = RANDOM.nextInt(i + 1);
        int temp = numbers[swap];
        numbers[swap] = numbers[i];
        numbers[i] = temp;
    }

    for (int i = 0; i < numbers.length; i++) {
        System.out.print(numbers[i] + ", ");
    }
}

public static void main(String[] args) {
    shuffle(new int[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11});
}

}

Output for test run is:

5, 11, 6, 1, 3, 10, 9, 2, 4, 7, 8,
abarinoff
  • 1
  • 1
  • I dont know why this made a difference. But I just moved my print statement to a seperate loop. and it works like it should now... Also I put the i+1 there. – Marcus Scaffidi Sep 24 '13 at 21:21
0

Use this method and pass your array in parameter

Collections.shuffle(arrayList);

This method return void so it will not give you a new list but as we know that array is passed as a reference type in Java so it will shuffle your array and save shuffled values in it. That's why you don't need any return type.

You can now use arraylist which is shuffled.

Sourse: https://stackoverflow.com/a/16112539/4291272

Faisal Shaikh
  • 3,900
  • 5
  • 40
  • 77