# Sudoku Solver - Sauber programmiert?



## LeopoldStotch (17. Mrz 2016)

An meiner FH läuft gerade ein Blockkurs zur vorbereitung auf den Programmierkurs2 im 2. Semester. Eine Aufgabe war es, ein Programm zu schreiben, dass Sudokus löst. Ich bin soweit zufrieden mit meinem Programm, mich würde aber interessieren, ob meine Programmierung soweit sauber ist. Ich kann dort leider nicht fragen, weil ich Krank bin und die Aufgaben zuhause gemacht habe.


```
public class Spielfeld {
    /*************Objektvariablen****************/
    private final int[][] gitterVorlage = new int[][] {  
        {2, 0, 0, 1, 0, 0, 0, 8, 0},
        {5, 0, 0, 0, 0, 8, 0, 0, 9},
        {0, 3, 0, 0, 0, 0, 7, 0, 0},
        {0, 0, 0, 0, 0, 0, 0, 0, 5},
        {0, 0, 0, 4, 0, 0, 0, 0, 3},
        {0, 6, 0, 0, 0, 2, 0, 4, 0},
        {7, 0, 1, 0, 2, 0, 0, 3, 0},
        {0, 0, 0, 7, 0, 5, 0, 0, 1},
        {0, 9, 0, 0, 1, 0, 6, 0, 0} };
      
    private int[][] gitter = new int [9][9];
    private long versuche = 0;
      
    /**************Getter&Setter****************/
    public long getVersuche(){
        return this.versuche;
    }
  
    /**************Konstruktor***************/
    public Spielfeld(){
        for(int i = 0; i < 9; i++)
            for(int j = 0; j < 9; j++)
                this.gitter[i][j] = gitterVorlage[i][j];
    }
  
    /*****************Methoden*******************/
    //Aktuelle Reihe auf das Vorkommen einer bestimmten Zahl prüfen
    private boolean checkReihe(int feld, int zahl){
        int reihe = feld/9;
      
        for(int i = 0; i < 9; i++)
            if(this.gitter[reihe][i] == zahl)
                return false;
        return true;
    }
  
    //Aktuelle Spalte auf das Vorkommen einer bestimmten Zahl prüfen
    private boolean checkSpalte(int feld, int zahl){
        int spalte = feld%9;
      
        for(int i = 0; i < 9; i++)
            if(this.gitter[i][spalte] == zahl)
                return false;
        return true;
    }
  
    //Aktuelle Box auf das Vorkommen einer bestimmten Zahl prüfen
    private boolean checkBox(int feld, int zahl){
        int reihe = feld/9;
        int spalte = feld%9;
      
        //Feststellen in welcher Box sich das Feld befindet
        int boxReihe = reihe/3;
        int boxSpalte = spalte/3;
      
        //Box prüfen
        for(int i = boxReihe*3; i < boxReihe*3 + 3; i++)
            for(int j = boxSpalte*3; j < boxSpalte*3 + 3; j++)
                if(this.gitter[i][j] == zahl)
                    return false;
      
        return true;
    }
  
    //Anzahl der Zahlen in einer Reihe zurückgeben
    private int zahlenInReihe(int feld){
        int reihe = feld/9;
        int anzahlZahlen = 0;
      
        for(int i = 0; i < 9; i++)
            if(this.gitter[reihe][i] != 0)
                anzahlZahlen++;
        return anzahlZahlen;
    }
  
    //Anzahl der Zahlen in einer Spalte zurückgeben
    private int zahlenInSpalte(int feld){
        int spalte = feld%9;
        int anzahlZahlen = 0;
      
        for(int i = 0; i < 9; i++)
            if(this.gitter[i][spalte] != 0)
                anzahlZahlen++;
        return anzahlZahlen;
    }
  
    //Anzahl der Zahlen innerhalb eines 3x3 Feldes zurückgeben
    private int zahlenInBox(int feld){
        int reihe = feld/9;
        int spalte = feld%9;
      
        //Feststellen in welcher Box sich das Feld befindet
        int boxReihe = reihe/3;
        int boxSpalte = spalte/3;
        int anzahlZahlen = 0;
      
        //Anzahl der Zahlen merken
        for(int i = boxReihe*3; i < boxReihe*3 + 3; i++)
            for(int j = boxSpalte*3; j < boxSpalte*3 + 3; j++)
                if(this.gitter[i][j] != 0)
                    anzahlZahlen++;
        return anzahlZahlen;
    }
  
    //Index des Feldes mit der größten Zahl zurückgeben
    private int bestesFeld(int[] feld) {
        int index = 0;
      
        for(int i = 0; i < feld.length; i++)
            if(feld[i] > feld[index])
                index = i;
        return index;
    }
  
    //Das Beste Feld (mit der geringsten Anzahl an Möglichkeiten) bestimmen
    public void feldFinden(){              
        //Freie Felder bewerten
        int[] feldWertung = new int[81];
        for(int i = 0; i < 81; i++)
            if(this.gitter[i/9][i%9] == 0)
                feldWertung[i] = zahlenInReihe(i) + zahlenInSpalte(i) + zahlenInBox(i);
      
        //Feldnummern (Indizes) der besten Felder, absteigend sortieren
        int bestesFeld = bestesFeld(feldWertung);
      
        //Lösung für das Feld suchen
        //feldAusgeben();
        try{
            //Thread.sleep(1000);
        }catch(Exception e){};
        System.out.println("");
        loesen(bestesFeld);
    }
  
    //Erstmögliche passende Zahl in ein bestimmtes Feld eintragen (Rekursiv)
    private void loesen(int feld){
        int reihe = feld / 9;
        int spalte = feld % 9;
      
        //Erstbeste Zahl eintragen
        for(int i = 1; i < 10; i++){
            if(checkReihe(feld, i) && checkSpalte(feld, i) && checkBox(feld, i)){
                this.gitter[reihe][spalte] = i;
                this.versuche++;
                feldFinden();
            }
        }
        if(!istSudokuGeloest())
            this.gitter[reihe][spalte] = gitterVorlage[reihe][spalte];
    }
  
    //Prüft, ob alle Felder eine Zahl enthalten
    private boolean istSudokuGeloest(){
        for(int i = 0; i < 81; i++)
            if(gitter[i/9][i%9] == 0)
                return false;
        return true;
    }
      
    public void feldAusgeben(){
        for(int i= 0; i < 9; i++){
            for(int j = 0; j < 9; j++)
                System.out.print(" " + this.gitter[i][j] + " ");
          
            System.out.println("");
        }
    }
}
```


----------



## Jardcore (17. Mrz 2016)

Finde die Idee witzig einen Soduko Löser zu schreiben, ich habe ihn nicht getestet, aber zum Code kann ich folgendes sagen:

Also ich würde empfehlen for() und if() immer mit geschweiften Klammern zu versehen, egal ob nur eine Zeile. Das verringert Seiteneffekte und macht den Code leserlicher und konsistenter.

Dann solltest du versuchen deine Methoden aussagekräftig zu benennen. Dann sparst du dir die überflüssigen Kommentare.

```
//Das Beste Feld (mit der geringsten Anzahl an Möglichkeiten) bestimmen
public void feldFinden(){
```
zu:

```
public void bestimmeDasFeldMitDerGeringstenAnzahlAnMoeglichkeiten()
```
Ein Konstruktor braucht auch keinen Kommentar das er ein Konstruktor ist  Wer den Kommentar braucht um zu erkennen das es sich um einen Konstruktor handelt sollte lieber keinen Quellcode lesen^^

Weiterhin hast du viele Magic Numbers, das sind Zahlen die nicht sofort Aufschluss über ihren Nutzen geben. Das sind bei dir alle for() Beende Bedingungen oder die initiale Größe der Arrays.

Exceptions sind Ausnahme Behandlungen und sollte auch nur im Ausnahmefall benutzt werden. (Das nur Mal allgemein^^) Du hast einen leeren try{}catch{} Block 

Komplizierte ausdrücke wie "gitter[i/9][i%9]== 0" könntest du in extra Methoden auslagern. Bzw. die Methode drum her rum anders bezeichnen, sodass deine ganzen boolean Methoden nur einen Boolschen Ausdruck prüfen und keine Schleife mehr durchlaufen müssen.

Zur Ausgabe kannst du weiterhin einen StringBuilder verwenden oder auch String.format() benutzten, das verbessert wieder die Lesbarkeit. String.format(" %d ", this.gitter_[j]);
_
Die indices einer Schleife könnte man auch eindeutig benennen, mit zum Beispiel: "zeile" und "spalte".


----------

