Jak poprawić generator losowych liczb JAVA

Question

for (int i = 0; i <= 19; i++) {
                rand[i] = (int) (Math.random() * 60 + 1);
            }
            for(int i=0;i<=19;i++)
            {
                rand_back[i]=rand[i];
            }

            for (int i = 0; i<=19;) {
                for(int j=0;j<=19;j++) {
                    //porównaj czy wsytapiła juz taka sama liczba
                    if((rand[i]==rand_back[j])&&(j!=i)) {
                        rand[j]=(int) (Math.random()*60+1);
                    }
                    if(j==19){
                        j=0;
                        i++;
                    }
                    if(i==19) {
                        break;
                    }
                }
                if(i==19) {
                    break;
                }
            }

Kod ten generuje 20 losowych liczb, błędem w nim jest to ze na powtórzenie liczby jest około 1/3 szansy. Czy wie ktoś jak to poprawić i czy istnieje prostszy i bardziej efektywny sposób ?

Answer 1

W twoim kodzie co chwile jest i==19 , j==19 itd. Lepiej jest to wrzucić do zmiennej.

int uniqeRoundNumbers = 20;
int randRange = 60;

Dzięki temu  łatwo zmienisz zakres losowania i ilość losowanych liczb.
Twój kod wyglądałby wtedy następująco.

int rand[] = new int[uniqeRoundNumbers];
        int rand_back[] = new int[uniqeRoundNumbers];


        for (int i = 0; i <= uniqeRoundNumbers-1; i++) {
            rand[i] = (int) (Math.random() * randRange + 1);
            loppcounter1++;
        }

        for(int i=0;i<=uniqeRoundNumbers-1;i++)
        {
            rand_back[i]=rand[i];
            loppcounter1++;
        }



        for (int i = 0; i<=uniqeRoundNumbers-1;) {
            for(int j=0;j<=uniqeRoundNumbers-1;j++) {
                loppcounter1++;
                //porównaj czy wsytapiła juz taka sama liczba
                if((rand[i]==rand_back[j])&&(j!=i)) {
                    rand[j]=(int) (Math.random()*randRange+1);
                }
                if(j==uniqeRoundNumbers-1){
                    j=0;
                    i++;
                }
                if(i==uniqeRoundNumbers-1) {
                    break;
                }
            }
            if(i==uniqeRoundNumbers-1) {
                break;
            }
        }

Zakładam że dążyłeś do czegoś co by wyglądało mniej więcej tak
 

int randTable[] = new int[uniqeRoundNumbers];
        int newrand;
        boolean valueExisatInTable;
        for (int i = 0; i <= uniqeRoundNumbers-1; i++) {
            newrand = (int) (Math.random() * randRange + 1);
            valueExisatInTable = checkCanValueExistInTable(randTable,newrand);

            if (valueExisatInTable == false){
                randTable[i] = newrand;
            }else {
                i--;
            }
            loppcounter2++;
        }

static private boolean checkCanValueExistInTable (int tab[],int newRand){
        for (int i:tab) {
            if(i == newRand){
                loppcounter2++;
                return true;
            }
        }
        return false;
    }

Ale można to zrobić krócej i lepiej

LinkedHashSet<Integer> randSet = new LinkedHashSet<>();
        do {
            randSet.add((int) (Math.random() * randRange + 1));
            loppcounter3++;
        }while(randSet.size()<=uniqeRoundNumbers-1);

Poniżej wklejam cały kod w którym dodałem z 3 metodami. Pierwsza metoda twoja (z powtórzeniami). Ponadto dodałem liczenie czasu wykonywania i ilość wykonanych pętli.
Przy wartościach uniqeRoundNumbers = 10000 i randRange = 100000 widać spore rozbieżności w czasie.

Twoja metoda
Loop counter = 100000002
Time = 263ms

Metoda na tablicy
Loop counter = 11094
Time = 58ms

Metoda na LinkedHashSet
Loop counter = 10535
Time = 10ms

import java.util.LinkedHashSet;
import java.util.Set;
import java.util.TreeSet;

public class Main {
    static int loppcounter1 = 0;
    static int loppcounter2 = 0;
    static int loppcounter3 = 0;
    public static void main(String[] args) {
        int uniqeRoundNumbers = 10000;
        int randRange = 100000;

        System.out.println("Losuj 1");
        losuj1(uniqeRoundNumbers,randRange);
        System.out.println();
        System.out.println("Losuj 2");
        losuj2(uniqeRoundNumbers,randRange);
        System.out.println();
        System.out.println("Losuj 3");
        losuj3(uniqeRoundNumbers,randRange);

    }

    private static void losuj3(int uniqeRoundNumbers, int randRange) {
        long millisActualTime = System.currentTimeMillis();

        LinkedHashSet<Integer> randSet = new LinkedHashSet<>();
        do {
            randSet.add((int) (Math.random() * randRange + 1));
            loppcounter3++;
        }while(randSet.size()<=uniqeRoundNumbers-1);


        long executionTime = System.currentTimeMillis() - millisActualTime;
        System.out.println("Loop counter = "+loppcounter3);
        System.out.println("Time = "+executionTime+"ms");
        soutTable(randSet);
        TreeSet<Integer> sortedSet = linkedHashSetToTreeSet(randSet);
        soutTable(sortedSet);
    }

    private static void losuj2(int uniqeRoundNumbers, int randRange ) {

        long millisActualTime = System.currentTimeMillis();

        int randTable[] = new int[uniqeRoundNumbers];
        int newrand;
        boolean valueExisatInTable;
        for (int i = 0; i <= uniqeRoundNumbers-1; i++) {
            newrand = (int) (Math.random() * randRange + 1);
            valueExisatInTable = checkCanValueExistInTable(randTable,newrand);

            if (valueExisatInTable == false){
                randTable[i] = newrand;
            }else {
                i--;
            }
            loppcounter2++;
        }

        long executionTime = System.currentTimeMillis() - millisActualTime;
        System.out.println("Loop counter = "+loppcounter2);
        System.out.println("Time = "+executionTime+"ms");
        soutTable(randTable);
        quicksort(randTable,0,randTable.length-1);
        soutTable(randTable);
    }

    static private boolean checkCanValueExistInTable (int tab[],int newRand){
        for (int i:tab) {
            if(i == newRand){
                loppcounter2++;
                return true;
            }
        }
        return false;
    }


    static private void losuj1(int uniqeRoundNumbers, int randRange ){
        loppcounter1 = 0;
        long millisActualTime = System.currentTimeMillis();
        int rand[] = new int[uniqeRoundNumbers];
        int rand_back[] = new int[uniqeRoundNumbers];


        for (int i = 0; i <= uniqeRoundNumbers-1; i++) {
            rand[i] = (int) (Math.random() * randRange + 1);
            loppcounter1++;
        }

        for(int i=0;i<=uniqeRoundNumbers-1;i++)
        {
            rand_back[i]=rand[i];
            loppcounter1++;
        }



        for (int i = 0; i<=uniqeRoundNumbers-1;) {
            for(int j=0;j<=uniqeRoundNumbers-1;j++) {
                loppcounter1++;
                //porównaj czy wsytapiła juz taka sama liczba
                if((rand[i]==rand_back[j])&&(j!=i)) {
                    rand[j]=(int) (Math.random()*randRange+1);
                }
                if(j==uniqeRoundNumbers-1){
                    j=0;
                    i++;
                }
                if(i==uniqeRoundNumbers-1) {
                    break;
                }
            }
            if(i==uniqeRoundNumbers-1) {
                break;
            }
        }

        long executionTime = System.currentTimeMillis() - millisActualTime;
        System.out.println("Loop counter = "+loppcounter1);
        System.out.println("Time = "+executionTime+"ms");
        soutTable(rand);
        quicksort(rand,0, rand.length-1);
        soutTable(rand);

    }

    static private TreeSet<Integer> linkedHashSetToTreeSet(LinkedHashSet<Integer> in){
        TreeSet<Integer> out = new TreeSet<>();
        for (Integer i : in){
            out.add(i);
        }
        return out;
    }

    static private void soutTable(int[] t ){
        for (int i : t) {
            System.out.print(i+" ");
        }
        System.out.println("");
    }
    static private void soutTable(Set<Integer> t ){
        for (int i : t) {
            System.out.print(i+" ");
        }
        System.out.println("");
    }

    private static void quicksort(int tablica[], int x, int y) {
        int i,j,v,temp;
        i=x;
        j=y;
        v=tablica[(x+y) / 2];
        do {
            while (tablica[i]<v)
                i++;
            while (v<tablica[j])
                j--;
            if (i<=j) {
                temp=tablica[i];
                tablica[i]=tablica[j];
                tablica[j]=temp;
                i++;
                j--;
            }
        }
        while (i<=j);
        if (x<j)
            quicksort(tablica,x,j);
        if (i<y)
            quicksort(tablica,i,y);
    }
}

 

Comments

Łał super kod dzieki wielkie za pomoc

Answer 2

Najprostszym wyjściem jest sprawdzenie już wylosowanych liczb i czy nowo wylosowana się tam znajduje. Jeśli tak powtórz losowanie. Jest to rozwiązanie pewne jednak w przypadku kiedy potrzeba generować dużą ilość może być czasochłonne (1000 czy nawet 10000 to nie jest dużo).