Get random numbers without repetition in Java. The one I made is too slow

Question

import java.util.Random;
import java.util.ArrayList;

public class RandomExample {
    private ArrayList<Integer> occurredNum = new ArrayList<Integer>();
    private Random r = new Random();
    private int play = 5, repCounter = 0;
    private boolean stopCheck = false;

    public void Tem() {
        occurredNum.add(play);
    }

    public int getRandomWithoutRepetition() {
        while (!stopCheck) {
            play = r.nextInt(9) + 1;

            for (int e: occurredNum) {
                if (play == e) {
                    repCounter++;
                }
            }
            if (repCounter == 0) {
                stopCheck = true;
                occurredNum.add(play);
            }
        }

        repCounter = 0;
        stopCheck = false;

        return (play);
    }

    public static void main(String[] args) {
        RandomExample t = new RandomExample();
        t.Tem();
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
    }
}

It prints a random number of numbers(even though I've asked it to print 8 different numbers between 1-9 without repetition) in the output and then goes into a never ending loop while trying to find the next number to print. Never had all 8 numbers in the output. My method seems to be too slow. Is there a better way to get a random number without repeating the number 5(first value of the arrayList) and whichever comes after.

Have you tried to decrement repCounter ? This code only increase it so it never would be 0 — Exception_al, Jan 23 '17 at 17:58
It resets it's value to zero before returning play, so it shouldn't be a problem when I call the method again. — Skaper, Jan 23 '17 at 18:01
When you want to use a large proportion of your set, it's pretty hard to beat creating a list of values, applying [Collections#shuffle](http://docs.oracle.com/javase/8/docs/api/java/util/Collections.html#shuffle-java.util.List-), and iterating through the shuffled list. — pjs, Jan 23 '17 at 20:45

davidxxx · Answer 1 · 2017-01-23T18:25:02.157

it prints a random number of numbers(even though I've asked it to print 8 different numbers between 1-9 without repetition)

The argument used in nextInt() is a problem in terms of performance if you don't want to repeat a same number:

 while (!stopCheck) {
        play = r.nextInt(9) + 1;

You should not use a constant value as parameter of nextInt() otherwise you will loop a long time to find a not used yet number.
Here you have 10 numbers, imagine with 100 or 1000 numbers. The solution is not at all scalable in a some way.

The idea is :

using the size of the list in the nextInt() method to look for only a not retrieved yet value. nextInt() will return a index for your list.
getting the value from the list by using the index
removing the element from the list.

and so for...

Here is a simple code to allow you to get an idea of the solution :

 public class RandomExample {
   ...
    private List<Integer> valuesToGet = new ArrayList<>(Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9));


    public int getRandomWithoutRepetition() {
        int index = r.nextInt(valuesToGet.size());
        Integer value = valuesToGet.get(index);
        valuesToGet.remove(index);
        return value;
    }
    ...
  }

@Skaper keep attention to this answer, is the best one – firegloves Jan 23 '17 at 23:54 — firegloves, Jan 23 '17 at 23:54

score 0 · Answer 2 · answered Jan 23 '17 at 18:23

If you want to improve the performance a bit more, consider use a HashSet.

public class RandomExample {

    private HashSet<Integer> occurredNum = new HashSet<>();
    private Random r = new Random();
    private int play = 5;

    public void Tem() {
        occurredNum.add(play);
    }

    public int getRandomWithoutRepetition() {
        int play;
        while (true) {
            play = r.nextInt(9) + 1;

            // Check if the hash set contains the element
            if (!occurredNum.contains(play)) {
                occurredNum.add(play); // add the new value
                return play; // return it
            }
        }
    }

    public static void main(String[] args) {
        RandomExample t = new RandomExample();
        t.Tem();
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
    }

}

Git a look at http://www.java67.com/2012/07/difference-between-arraylist-hashset-in-java.html and then this comparison http://stackoverflow.com/a/19841066/5734097 — D.Kastier, Jan 23 '17 at 18:33

firegloves · Accepted Answer · 2017-01-23T18:26:14.203

public class RandomExample {
    private ArrayList<Integer> occurredNum = new ArrayList<Integer>();
    private Random r = new Random();
    private int play = 5;
    private boolean stopCheck;

    public void Tem() {
        occurredNum.add(play);
    }

    public int getRandomWithoutRepetition() {

        // reset boolean flag
        stopCheck = false;

        while (!stopCheck) {

            // get random num
            play = r.nextInt(9) + 1;

            // asking to the ArrayList if random num is an item already added => if ArrayList contains no primitive data type you should override data type's equals(Object obj) method
            if (! occurredNum.contains(play)) {

                // if num is not contained => add num to ArrayList
                occurredNum.add(play);

                // set stop checking flag
                stopCheck = true;
            }

        }

        // return new num
        return (play);
    }

    public static void main(String[] args) {
        RandomExample t = new RandomExample();
        t.Tem();
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
        System.out.println(t.getRandomWithoutRepetition());
    }
}

You don't need repCounter, your code is not much readable and you are not taking advantage of Java's Object Oriented Programming features.

Keep in mind that now you are comparing integers, so the code of a for iteration to check if an object is present into an ArrayList is not much long. But in the future you could have to compare complex objects, so in that case a better approach is to use ArrayList.contains() method and to override your object equals(Object obj) method, that is called during ArrayList.contains() comparison

Are you going to provide any explanation as to what the problem was or what is different in your code? — takendarkk, Jan 23 '17 at 18:11
It works. Thanks a lot. Could you tell me what caused my code to be so slow? — Skaper, Jan 23 '17 at 18:12
If he could find the errors himself he probably wouldn't post a question on this website asking us to find them for him. — takendarkk, Jan 23 '17 at 18:13
@takendarkk it wasn't an error. I proposed a different alghoritm, I can add some comments. I'm not working for others, I'm helping them. I've done my coding job for 8 hours, I'm helping others to solve their problem also in my free time, I think they can do their effort of thinking about solutions they get for free. Or at least to ask explanation, I'll be available to give them — firegloves, Jan 23 '17 at 18:17
@firegloves If the repcounter increases, the while loop goes on forever. Thats the problem. :D — Skaper, Jan 23 '17 at 18:19
@Skaper you are welcome. I have added some infos to my answer, read them if you like — firegloves, Jan 23 '17 at 18:26

Get random numbers without repetition in Java. The one I made is too slow

3 Answers3