HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

RandomCount class: Randomising numbers in an array and generating them in a random order

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomorderrandomcountarraynumbersgeneratingthemandclassrandomising

Problem

public class SimpleRandomCount extends RandomCount
{
 /**
 * Generate an array containing elements in a random order
 * 
 * @param size the size of the array to be generated
 */
public SimpleRandomCount(int size) {
super(size);
}

/**
* Randomise the array
*/
protected void randomise() {
int[] copy = new int[array().length];
int randomIndex;

// used to indicate if elements have been used
boolean[] used = new boolean[array().length];
Arrays.fill(used,false);
for (int index = 0; index < array().length; index++) {
    do {
        randomIndex = randomIndex();
    } while (used[randomIndex]);

    copy[index] = array()[randomIndex];
    used[randomIndex] = true;
}

for (int index = 0; index < array().length; index++) {
    array()[index] = copy[index];
   }
 }
      public static void main(String [] args){
      RandomCount count = new SimpleRandomCount(5);
      System.out.println("Array is " + Arrays.toString(count.array()));
     }
}


The SimpleRandomCount class is part of another class called RandomCount. I was wondering if there was a cleverer, shorter way of randomising the numbers without using a boolean value to help do it?

Thanks for any help it is very much appreciated.

Solution

Here's a simple algorithm lifted directly from the Collections source code. It is modified to shuffle an int[] in-place. That is, the array passed in will be modified. You probably want to give clients the opportunity to specify their own Random instance in a constructor. If you keep concrete inheritance, it might also make sense for the Random to be stored in RandomCount.

public final class SimpleRandomCount extends RandomCount {
    private final Random random = new Random();

    public SimpleRandomCount(int size) {
        super(size);
    }

    protected void randomise() {
        this.shuffleInPlace(array(), this.random);
    }

    private void shuffleInPlace(final int[] values, final Random random) {
        for (int i = values.length; i > 1; i--) {
            final int swapIndex = random.nextInt(i);
            final int temp = values[i - 1];
            values[i - 1] = values[swapIndex];
            values[swapIndex] = temp;
        }
    }

    public static void main(String [] args){
        RandomCount count = new SimpleRandomCount(5);
        System.out.println("Array is " + Arrays.toString(count.array()));
    }
}


Unrelatedly, it's probably a Bad Idea to expose your raw array in a public method like array(). If you must use extension instead of a RandomCount interface, either make the array member variable protected or provide a protected direct accessor. Clients using the public accessor should only have access to a copy of the array. Otherwise they're going to do Bad Things to it, and that will affect the internals of your class.

Code Snippets

public final class SimpleRandomCount extends RandomCount {
    private final Random random = new Random();

    public SimpleRandomCount(int size) {
        super(size);
    }

    protected void randomise() {
        this.shuffleInPlace(array(), this.random);
    }

    private void shuffleInPlace(final int[] values, final Random random) {
        for (int i = values.length; i > 1; i--) {
            final int swapIndex = random.nextInt(i);
            final int temp = values[i - 1];
            values[i - 1] = values[swapIndex];
            values[swapIndex] = temp;
        }
    }

    public static void main(String [] args){
        RandomCount count = new SimpleRandomCount(5);
        System.out.println("Array is " + Arrays.toString(count.array()));
    }
}

Context

StackExchange Code Review Q#107275, answer score: 2

Revisions (0)

No revisions yet.