Warum werden immer noch doppelte Zahlen ausgegeben ?

[Käsekuchen]

    public static void main(String[]args) {
        
        
        int [] zahlen = new int [6];
        for(int i = 0;i<6;i++) {
            int x =(int)( Math.random()*6)+1;
            zahlen[i]=x;
            
            
            for(int a = 0; a<i;a++) {
                
                if(zahlen[a]==zahlen[i]) {
                    x =(int)( Math.random()*6)+1;
                    zahlen[i]=x;
                    a=0;
                    
                }
            }
        }
        
        for(int i = 0;i<6;i++) {
            System.out.println(zahlen[i]);
        }
        
    }

Ich möchte, dass jede Zahl genau 1 mal ausgegeben wird, deshalb habe ich die for-Schleife (Zeile-10) eingebaut, wenn eine Zahl doppelt vor kommt, dann wird eine neue "erstellt", da diese neue Zahl ja auch wieder doppelt sein könnte habe ich dann a=0 gemacht, um diese neue Zahl wieder zu testen usw. Trotzdem bekomm ich immer noch ab und zu doppelte Zahlen, woran liegt das? Was ist hier bei mir der Fehler?

 

[DrPils]

Ich würde die Prüfung ob eine Zahl bereits in einem array ist in eine separate Methode auslagern. Und diese dann bei der Befüllung entsprechend aufrufen. So fällt es dir sicherlich leichter den Fehler zu erkennen (und mir auch )

 

[watten]

woran liegt das? Was ist hier bei mir der Fehler?

An der Zufallskomponente deines Programms.

 

[Robert Zenz]

Also ich muss ehrlich sagen, ich habe einen Moment gebraucht um den subtilen Fehler zu finden, der ist ganz nett. Wenn du die Loesung selbst finden willst, dann lass' dir mal alle Zustaende ausgeben, also welcher Index gerade gesetzt wird, welcher Index gerade geprueft wird und so weiter.

Die Loesung selbst ist eine falsche Annahme. Wenn wir jetzt diese Schleife nehmen:

for (int counter = 0; counter < 5; counter++) {
    // Koerper
}

Dann kann man diese in etwa so aufloesen (etwas artistische Freiheit, das ist nicht wie die JVM funktioniert...glaube ich):

int counter = 0;

goto ERSTER_START; // Ueberspringen der Inkrementierung beim ersten Lauf.

:SCHLEIFEN_START
counter = counter + 1;
:ERSTER_START
if (counter < 5) {
    // Koerper
    
    goto SCHLEIFEN_START; // Zum naechsten Durchlauf.
}

Wenn wir das jetzt mit deinem Koerper befuellen ergibt sich folgendes:

int a = 0;

goto ERSTER_START; // Ueberspringen der Inkrementierung beim ersten Lauf.

:SCHLEIFEN_START
a = a + 1;
:ERSTER_START
if (a < i) {
    // Deine Logik
    if(zahlen[a] == zahlen[i]) {
        x =(int)(Math.random() * 6) + 1;
        zahlen[i] = x;
        a = 0;
    }        
    // Ende deiner Logik
    
    goto SCHLEIFEN_START; // Zum naechsten Durchlauf.
}

Erkennst du das Problem in dem Code jetzt?

 

[Jw456]

Du sollst die random zahl erst prüen bevor du sie in dem Array speicherst.
Du speicherst schon in Zeile 7.
Aber geprüft hast du da noch nicht

 

[Käsekuchen]

Also ich muss ehrlich sagen, ich habe einen Moment gebraucht um den subtilen Fehler zu finden, der ist ganz nett. Wenn du die Loesung selbst finden willst, dann lass' dir mal alle Zustaende ausgeben, also welcher Index gerade gesetzt wird, welcher Index gerade geprueft wird und so weiter.

Die Loesung selbst ist eine falsche Annahme. Wenn wir jetzt diese Schleife nehmen:

for (int counter = 0; counter < 5; counter++) {
    // Koerper
}

Dann kann man diese in etwa so aufloesen (etwas artistische Freiheit, das ist nicht wie die JVM funktioniert...glaube ich):

int counter = 0;

goto ERSTER_START; // Ueberspringen der Inkrementierung beim ersten Lauf.

:SCHLEIFEN_START
counter = counter + 1;
:ERSTER_START
if (counter < 5) {
    // Koerper
   
    goto SCHLEIFEN_START; // Zum naechsten Durchlauf.
}

Wenn wir das jetzt mit deinem Koerper befuellen ergibt sich folgendes:

int a = 0;

goto ERSTER_START; // Ueberspringen der Inkrementierung beim ersten Lauf.

:SCHLEIFEN_START
a = a + 1;
:ERSTER_START
if (a < i) {
    // Deine Logik
    if(zahlen[a] == zahlen[i]) {
        x =(int)(Math.random() * 6) + 1;
        zahlen[i] = x;
        a = 0;
    }       
    // Ende deiner Logik
   
    goto SCHLEIFEN_START; // Zum naechsten Durchlauf.
}

Erkennst du das Problem in dem Code jetzt?

Nein leider nicht, steh irgendwie aufn Schlauch

 

[KonradN]

Also da einfach einmal paar deutliche Worte zu Dingen, die man machen oder eben nicht machen sollte:

1. Der Hinweis von @DrPils ist sehr wichtig. Immer versuchen, den Code übersichtlich zu halten. Ein wichtiger Punkt dabei ist, dass man Code in Methoden unterteilt. Das macht dann einiges einfacher.

2. Es gibt diverse Schleifen. Dabei ist wichtig, das man diese so nutzt, wie sie gedacht sind. Es ist en Unding, in einer Zählschleife innerhalb der Schleife die Zählvariable zu verändern. Der Code ist ein super Beispiel:

Nein leider nicht, steh irgendwie aufn Schlauch

Nach jedem Durchlauf wird die Zählvariable erhöht. Du setzt die auf 0, dann wird sie um 1 erhöht. Bei jeder Wiederholung startet die Prüfung also mit Index 1 und nicht Index 0! Innerhalb der Schleife müsste also der Index auf -1 gesetzt werden ... aber das ist ja total unleserlich! Statt einer for Schleife könnte da eine while Schleife verwendet werden.

 

[Robert Zenz]

Um jetzt mal die Neugierde aller zu befriedigen, wie die JVM der Eclipse Compiler Schleifen handhabt:

for (int counter = 0; counter < 5; counter++) {
    // Koerper
    System.out.println("");
    // Koerper Ende
}

   0: iconst_0      
   1: istore_1        /* counter */
   2: goto            16
      // Koerper
   5: getstatic       java/lang/System.out:Ljava/io/PrintStream;
   8: ldc             ""
  10: invokevirtual   java/io/PrintStream.println:(Ljava/lang/String;)V
      // Koerper Ende
  13: iinc            counter, 1
  16: iload_1         /* counter */
  17: iconst_5      
  18: if_icmplt       5
      linenumber      12

Also in Schoenschrift:

int counter = 0;

goto SCHLEIFEN_PRUEFUNG;

:SCHLEIFEN_KOERPER
// Koerper
System.out.println("");
// Koerper Ende

counter = counter + 1;

:SCHLEIFEN_PRUEFUNG
if (counter < 5) {
    goto SCHLEIFEN_KOERPER;
}

Nein leider nicht, steh irgendwie aufn Schlauch

Du setzt a = 0, anschlieszend wird a fuer den naechsten Schleifendurchlauf inkrementiert. Index "0" wird also nur beim ersten Durchlauf geprueft, danach nie wieder. Lass' dir a bei jedem Schleifendurchlauf mal ausgeben:

            for(int a = 0; a<i;a++) {
                System.out.println("Index a: " + Integer.toString(a));

                if(zahlen[a]==zahlen[i]) {
                    x =(int)( Math.random()*6)+1;
                    zahlen[i]=x;
                    a=0;
                   
                }
            }

Nach jedem Durchlauf wird die Zählvariable erhöht. Du setzt die auf 0, dann wird sie um 1 erhöht. Bei jeder Wiederholung startet die Prüfung also mit Index 1 und nicht Index 0! Innerhalb der Schleife müsste also der Index auf -1 gesetzt werden ... aber das ist ja total unleserlich! Statt einer for Schleife könnte da eine while Schleife verwendet werden.

Du bist sogar dann noch schneller wenn ich als erster antworte...es gibt kein gewinnen gegen dich, oder?

 

[Käsekuchen]

Also da einfach einmal paar deutliche Worte zu Dingen, die man machen oder eben nicht machen sollte:

1. Der Hinweis von @DrPils ist sehr wichtig. Immer versuchen, den Code übersichtlich zu halten. Ein wichtiger Punkt dabei ist, dass man Code in Methoden unterteilt. Das macht dann einiges einfacher.

2. Es gibt diverse Schleifen. Dabei ist wichtig, das man diese so nutzt, wie sie gedacht sind. Es ist en Unding, in einer Zählschleife innerhalb der Schleife die Zählvariable zu verändern. Der Code ist ein super Beispiel:

Nach jedem Durchlauf wird die Zählvariable erhöht. Du setzt die auf 0, dann wird sie um 1 erhöht. Bei jeder Wiederholung startet die Prüfung also mit Index 1 und nicht Index 0! Innerhalb der Schleife müsste also der Index auf -1 gesetzt werden ... aber das ist ja total unleserlich! Statt einer for Schleife könnte da eine while Schleife verwendet werden.

danke für die Antwort

 

[Käsekuchen]

Um jetzt mal die Neugierde aller zu befriedigen, wie die JVM der Eclipse Compiler Schleifen handhabt:

for (int counter = 0; counter < 5; counter++) {
    // Koerper
    System.out.println("");
    // Koerper Ende
}

   0: iconst_0     
   1: istore_1        /* counter */
   2: goto            16
      // Koerper
   5: getstatic       java/lang/System.out:Ljava/io/PrintStream;
   8: ldc             ""
  10: invokevirtual   java/io/PrintStream.println:(Ljava/lang/String;)V
      // Koerper Ende
  13: iinc            counter, 1
  16: iload_1         /* counter */
  17: iconst_5     
  18: if_icmplt       5
      linenumber      12

Also in Schoenschrift:

int counter = 0;

goto SCHLEIFEN_PRUEFUNG;

:SCHLEIFEN_KOERPER
// Koerper
System.out.println("");
// Koerper Ende

counter = counter + 1;

:SCHLEIFEN_PRUEFUNG
if (counter < 5) {
    goto SCHLEIFEN_KOERPER;
}

Du setzt a = 0, anschlieszend wird a fuer den naechsten Schleifendurchlauf inkrementiert. Index "0" wird also nur beim ersten Durchlauf geprueft, danach nie wieder. Lass' dir a bei jedem Schleifendurchlauf mal ausgeben:

            for(int a = 0; a<i;a++) {
                System.out.println("Index a: " + Integer.toString(a));

                if(zahlen[a]==zahlen[i]) {
                    x =(int)( Math.random()*6)+1;
                    zahlen[i]=x;
                    a=0;
                  
                }
            }

Du bist sogar dann noch schneller wenn ich als erster antworte...es gibt kein gewinnen gegen dich, oder?

dank dir

 

[Robert Zenz]

Die einfachere Variante von dem was du versuchst zu machen ist im uebrigen das Array zu befuellen und dann zufaellig zu verwuerfeln:

int zahlen = new int[6];

// Befuellen des Arrays.
for (int index = 0; index < zahlen.length; index++) {
    zahlen[index] = index + 1;
}

Random random = new Random();

// Zufaelliges verwuerfeln
for (int index = 0; index < zahlen.length; index++) {
    int tauschIndex = random.nextInt(zahlen.length);
    
    int temp = zahlen[index];
    zahlen[index] = zahlen[tauschIndex];
    zahlen[tauschIndex] = temp;
}

Aber je nach Aufgabenstellung war das vielleicht nicht gefragt. Das sollte auch ein Fisher-Yates Shuffle sein, oder so aehnlich.

 

[Blender3D]

Das sollte auch ein Fisher-Yates Shuffle sein, oder so aehnlich.

Nicht ganz also so ähnlich.
https://en.wikipedia.org/wiki/Fisher–Yates_shuffle
Fisher Yates tauscht einen bereits getauschten Index nicht mehr da er den Index sukzessiv einschränkt.

public class Shuffle {
    private static Random rnd = new Random();

    public static void main(String[] args) {
        int[] array = { 1, 2, 3, 4, 5, 6 };
        shuffle(array);
        System.out.println(Arrays.toString(array));
    }

    public static void shuffle(int[] data) {
        for (int i = data.length - 1; i > 0; i--) {
            int swapId = rnd.nextInt(i);
            int tmp = data[swapId];
            data[swapId] = data[i];
            data[i] = tmp;
        }
    }

}

 

[Marsupilami]

https://www.java-forum.org/thema/wie-kann-man-in-java-nach-arrays-auf-duplikate-pruefen.201596/#post-1346392

Beschreibt das Problem eigentlich auch ganz gut und zeigt mehrere Lösungswege auf!

 

[KonradN]

Wobei er ja einen Algorithmus hat. Nur eben hat er einen Fehler in seinem Algorithmus weil er halt eine Zählschleife missbraucht hat und so leider einen relativ schwer zu findenden Fehler eingebaut hat.

Ein Grund, wieso ich sowas nicht durchgehen lassen würde in einem Codereview. Man muss nur daran denken, dass dies auch ein schwer zu testender Fehler ist! So Dinge können durchaus mehrfach erfolgreich durchlaufen und dennoch ist es falsch. Mach da mal die Range größer - also Zahlen von 1-100 werden gezogen. Dann kann man da einen Test schreiben, der sich 6 so Zahlen geben lässt und prüft da dann auf Eindeutigkeit der Zahlen. Der Test wird mehrfach gut durchlaufen und hin und wieder auf einen Fehler stoßen. Aber das wird dann schon irgend ein Fehler des Build Systems gewesen sein ... ein neues mvn clean package lief ja erfolgreich (incl. Unit Tests) durch.

Daher: So Dinge immer gut unterteilen. Alleine schon, um wirklich ein guten Test für alle Teile haben zu können. Mit ist bewusst, dass ich hier jetzt über das Ziel hinaus schieße - es geht um eine Anfänger Aufgabe und da sind noch keine Unit Tests und co vorhanden. Aber dennoch macht es Sinn, hier einen Ausblick zu geben. Und es mag durchaus Andere geben, die über den Thread kommen, für die das Interessant ist.

Also betrachten wir die Aufgabe einfach einmal mit einem "tieferen Wissenstand".

Es sollen n unterschiedliche Zahlen erwürfelt werden mit einem Würfel mit n Seiten. (Ich habe mal statt 6 direkt ein n gesetzt) Und wir haben uns überlegt, dass wir das 1:1 umsetzen: KISS: Keep it simple, stupid. Also an der Stelle keine Optimierungen!

Wie gehen wir da vor? Wenn wir da keine Ahnung haben, dann probieren wir es aus. Egal wie: n = 2: Wir werfen eine Münze. n=6: Wir nehmen einfach einen Würfel. Wir nehmen einen Würfel und spielen das etwas durch und beobachten, was wir machen:

1. Wir würfeln
2. Wir schauen, ob die gewürfelte Zahl bereits auf dem Zettel steht. Ist das der Fall, dann starten wir beim ersten Schritt
3. Wir schreiben die gewürfelte Zahl auf den Zettel.
4. Wir prüfen, ob wir die gewünschte Anzahl an Zahlen haben. Wenn nicht, dann starten wir wieder beim ersten Schritt.

Wir verifizieren, ob das Funktioniert. Wir spielen es etwas durch und merken uns, was wir so brauchen. Alles, was wir brauchen, muss auf dem Zettel von uns stehen. Also haben wir auf dem Zettel: gewürfelte Zahl und die Liste an gewürfelten Zahlen. Evtl. auch schon die Anzahl der bereits notierten Zahlen (wobei wir die auch zählen könnten).
Wir merken aber beim durchspielen auch direkt: Das ist problematisch: Bei der 5ten und 6ten Zahl würfeln wir teilweise ganz schön lange. Evtl. überlegen wir uns also doch eine andere Lösung. Eine Idee könnte dann das "Kartenspiel" sein. Statt zu würfeln haben wir ein Kartenspiel mit n Karten, wir mischen es und haben dann die entsprechende Lösung. Sowas in der Art. Siehe Link aus #13 für diese Ideen. Darum geht es aber nicht. Wir sind bei der Umsetzung unserer Idee.

Und dann kommen wir jetzt dazu, den Algorithmus etwas aufzuschreiben.

Und das wird dann etwas wie:

Erwürfel die Zahlenreihe:
1. So lange noch nicht genug Zahlen gewürfelt wurden: Würfel die nächste eindeutige Zahl.

Würfel die nächste eindeutige Zahl
1. Würfel so lange eine neue Zahl, bis die Zahl eindeutig ist
2. gib die eindeutige Zahl zurück

Ist Zahl eindeutig?
1. Gehe alle bisher gewürfelten Zahlen durch:
1.1 Ist die aktuell gewürfelte Zahl == atuelle Zahl dann gib false zurück
2. gib true zurück

Würfel Zahl
1. Ermittel Zufallszahl von [1..n] und gib diese zurück

Somit haben wir schon 4 Algorithmen / Methoden. Das beschreibt das reine Vorgehen. Wir müssen jetzt nur überlegen, wie wir das mit den Daten modellieren. Da kann man dann schauen, was es so gibt. Wenn man nicht objektorientiert vorgehen will, dann hat man da z.B. einfach das Array mit int Werten, ein int wert, das angibt, bis wohin es gefüllt wurde und ein int wert mit der aktuell gewürfelten Zahl. Und man schaut dann, wo was benötigt wird und übergibt die Dinge einfach. Oder man macht es objektorientierter. Dann hat man z.B. eine Klasse Würfel. Man hat eine Klasse mit gewürfelten Zahlen. Man hat eine Klasse, die die Logik enthält. Wenn man sowas schaut, dann kann man überlegen, was es da jeweils an Daten und an Funktionalität gibt. Man kann überlegen, was es bereits gibt. Und dann stellt man fest, dass man ggf. manche Klasse gar nicht zwingend braucht:
a. Würfel ist evtl. einfach die Klasse Random. Wobei das schon lesbarer sein kann, da eine eigene Klasse zu haben.
b. Die Liste gewürfelter Zahlen kann evtl. einfach eine List oder hier speziell Set sein. Wenn es ein Set ist, dann wird plötzlich vieles einfacher. Da ein Set keine doppelten Zahlen haben kann, wird aus dem Algorithmus einfach ein
"Füge dem Set gewürfelter Zahlen so lange die nächste gewürfelte Zahl hinzu, bis das Set n Elemente hat."

Auf jeden Fall kann man dann sowas unterteilen. Man erhält dann mehrere keine Methoden. Jede Methode für sich ist gut überschaubar. Und dann kann man jede Methode auch einzeln testen.

Das nur einmal als sehr ausführliche Beschreibung zu dem, was bereits geschrieben wurde. Vielleicht hilft das ja dem Einen oder Anderen.