Wieso stockt mein Sudoku-Spiel ab einem bestimmten Punkt?

Ich möchte im Java Editor ein 4x4 Sudoku-Spiel programmieren. Es wird ausgegeben, aber stockt irgendwann ab einem bestimmten Punkt.

Das Sudoku an sich läuft, aber bin noch nicht ganz fertig. Ich bin gerade dabei, einen Checkbutton zu machen, welcher prüfen soll, ob gültige Werte eingegeben wurden (1-4) oder andere zeichen wie "-". Dann soll das NumberFieldsarray mit einer Musterlösung abgeglichen werden.

In dem Checkbutton läuft dann irgendetwas schief, denn da kommt es zum Stocken ohne Fehlermeldung.

https://pastebin.com/iCLAZPps

Die zwei Methoden "nullen_rein /raus" wollte ich als Umweg nutzen, da ich nie das Numberfield auf eine Leere testen konnte. Also wollte ich das ursprüngliche Arrray einsetzen, denn da hab ich freie Stellen eines Sudokus mit 0 ersetzt, sonst hätte ich kein Array machen können.

Ab Zeile 488 wird nichts mehr ausgegeben, wenn ich z.B. ein

JOptionPane.showMessageDialog(null, "test");

mache.

Ich bin am Verzweifeln.

Answer 1

[regex9]

Dein Programmcode ist nicht sonderlich gut. Es gibt etliche Wiederholungen, irrelevante Elemente (damit meine ich vor allem die Kommentare), falsche Einrückungen und obendrein wird Swing falsch eingesetzt. Es ist nicht verwunderlich, wenn dabei die Übersicht verlorengeht.

Auf ein paar Punkte gehe ich einmal genauer ein.

1) Den expliziten Import für das JOptionPane brauchst du nicht. Die Klasse wird doch bereits importiert.

2) Bei deinen Bezeichnern gibt es ein ziemliches Wirrwarr. Das Muster ändert sich immer wieder (Bsp. bMittel, musterL3, leicht1, Felder), die gewählte Sprache hältst du nicht konsistent ein.

Nach gängiger Java-Konvention starten Klassen-/Typnamen übrigens immer mit einem Großbuchstaben, sodass sie leicht von Variablen unterschieden werden können. Von dieser Regel abzuweichen ist nicht sinnvoll, sondern begünstigt eher Fehler. Tutorials, die so etwas womöglich vormachen, würde ich raten, zu meiden, da die Wahrscheinlichkeit nicht gering ist, dass die dann auch an anderen Stellen schlechte Praktiken lehren.

3) Es gibt keinen Grund, die Variablen Felder, schwierigkeitsgrad und zwischenspeicher statisch zu deklarieren. Sie sind doch an ein Spielfeld gebunden. Ein weiteres Spielfeld sollte seinen eigenen Schwierigkeitsgrad sowie seine eigenen Spielfelder mit ihren eigenen Zuständen haben.

Des Weiteren gilt initial für jedes Feld erst einmal das Prinzip der Kapselung. Der private (oder protected) modifier wäre demzufolge für alle Felder zu bevorzugen.

4) Um Berechnungsfehler zu vermeiden, ist es ratsam, wenig hartcodiert Werte zu verwenden, sondern mehr mit Variablen zu arbeiten. Es vereinfacht auch die Wartung bzw. macht dein Programmcode flexibler. Stell dir nur einmal vor, du möchtest das Sudokufeld doch auf 25 Felder ändern: Dann musst du jede Codestelle finden, die irgendwie mit der Anzahl an Spalten/Reihen in Verbindung steht.

Statt also in Zeilen wie diesen:

/* ... */ = new JNumberField[16];
/* ... */ = new int[16];
/* ... */ = (int) (Math.random() * 4) + 1;

feste Werte hineinzuschreiben, wäre es besser, eine Konstante für die Berechnung einzusetzen. Ich werde in meinen Beispiel-Snippets übrigens englische Wörter für die eingesetzten Elemente verwenden.

private final int SIZE = 4;
/* ... */ = new JNumberField[SIZE * SIZE];
/* ... */ = new int[fields.length];
/* ... */ = (int) (Math.random() * SIZE) + 1;

5) Da der Schwierigkeitslevel nur durch bestimmte konkrete Werte beschrieben wird, wäre eine Enumeration zur Beschreibung dieses Typs besser geeignet.

enum DifficultyLevel { EASY, MEDIUM, HARD }

6) Um ein Projekt übersichtlich zu halten, sollte es zerlegt werden (Stichwort: modularer Aufbau). Auf oberer Ebene ist es beispielsweise sinnvoll, die reine Anwendungslogik und Datenschicht von der Logik für die grafische Oberfläche zu trennen. In der konkreteren Code-Schicht sind es vor allem Methoden, die Programmteile strukturieren.

Diese einzelnen Teile werden dann auch besser testbar und bergen weniger Fehlerrisiken, da sie stärker von anderen Programmteilen entkoppelt werden. Im gleichen Zug wirst du evt. dazu gezwungen, abstraktere Formulierungen vorzunehmen, was diese Teile auch besser wiederverwendbar macht.

Konkret bei deinem Projekt würde ich empfehlen:

a) Die Feldzustände auszulagern. Dabei könnte man auch gleich eine Neustrukturierung vornehmen, mittels einer extra Klasse, die sich sowohl den Initial- als auch den Lösungszustand merkt.

Beispiel:

public class Board {
  private final int[] initialState;
  private final int[] solution;

  public Board(final int[] initialState, final int[] solution) {
    this.initialState = initialState;
    this.solution = solution;
  }

  public int[] getInitialState() {
    return initialState;
  }

  public int[] getSolution() {
    return solution;
  }
}

Eine Überlegung wäre es wert, im Konstruktor noch eine Validation hinsichtlich der Arraygrößen einzubauen.

Die konkreten Werte wiederum könnten in einer GameFactory definiert werden:

public class GameFactory {
  private final Map<DifficultyLevel, List<Board>> boards;

  public GameFactory() {
    boards = new HashMap<>();
    boards.put(DifficultyLevel.EASY, Arrays.asList(
      new Board(
        new int[] { 0, 0, 0, 0, 3, 0, 1, 2, 0, 3, 0, 0, 4, 0, 3, 1 },
        new int[] { 2, 1, 4, 3, 3, 4, 1, 2, 1, 3, 2, 4, 4, 2, 3, 1 }
      ),
      /* other variants ... */
    ));
    /* put other difficult level boards ... */
  }

  public Board getRandomBoard(final DifficultyLevel level) {
    var levelBoards = boards.containsKey(level)
      ? boards.get(level)
      : boards.get(DifficultyLevel.EASY);
    var random = (int) (Math.random() * levelBoards.size());
    return levelBoards.get(random);
  }
}

Man könnte sie aber auch in eine / mehrere Dateien auslagern und über eine GameFactory-Klasse einlesen lassen.

b) Den Code aus dem Konstruktor in verschiedene Helfermethoden aufzuteilen. Gerade der Umstand, das du öfter immer wieder dieselben Anweisungen schreibst, sollte dir doch auch längst aufgefallen sein.

Für das Erstellen eines Levelauswahl-Buttons könnte man bspw. eine abstrakte Factorymethode bereitstellen:

private JButton getLevelButton(String title, Color backgroundColor) {
  var button = new JButton(title);
  button.setMargin(new Insets(2, 2, 2, 2));
  button.setFont(new Font("Arial", Font.BOLD, 16));
  button.setBorder(BorderFactory.createLineBorder(Color.BLACK));
  button.setBackground(backgroundColor);
  return button;
}

Die setFont-Methode brauchst du im Übrigen nur einmal aufrufen, wenn du den Zustand des Font-Objekts gleich von Anfang an vollständig richtig setzt.

Zum Erstellen eines Rahmens solltest du des Weiteren immer die BorderFactory benutzen, da die im Hintergrund dafür sorgt, dass nicht immer wieder neue Border-Objekte angelegt werden müssen.

c) Für die Initialisierung des Spielfeldes brauchst du nur eine einzige (Action Handler)-Methode.

private void setupBoard(DifficultyLevel level) {
  currentBoard = gameFactory.getRandomBoard(level);
  var initialFieldValues = currentBoard.getInitialState();

  for (int i = 0; i < fields.length; ++i) {
    var value = initialFieldValues[i];

    if (value == 0) {
      fields[i].setEditable(true);
      fields[i].clear();
      continue;
    }

    fields[i].setInt(value);
  }

  layout.last(getContentPane());
}

Dabei brauchst du auch die vielen Ein-/Ausblendelogiken überhaupt nicht, wenn du stattdessen mit einem CardLayout arbeitest. Darauf gehe ich später noch ein.

Die ActionListener-Definition kannst du mittels Lambda-Ausdruck abkürzen.

Beispiel:

var easyLevelButton = getLevelButton("Einfach", Color.GREEN);
easyLevelButton.addActionListener(event -> setupBoard(DifficultyLevel.EASY));

7) Bezogen auf das Frame-Setup tümmelt sich innerhalb des Konstruktors gelinde gesagt noch einiger Unsinn.

a) Zum einen kann Fenstertitel direkt an den Superkonstruktor überreicht werden. So sparst du eine Codezeile ein und gibst dem expliziten Aufruf von super auch einen Sinn (so lange es einen parameterlosen Basiskonstruktor gibt, wird der implizit schon immer aufgerufen).

super("sudoku3");

b) Die extra Berechnung für die Fensterposition ist unnötig. Mit:

setLocationRelativeTo(null);

klappt das in einer Zeile und ist zudem auch noch zuverlässiger.

c) Indem du das Layout-Modell von Swing aushebelst, eine Fensterskalierung durch den Nutzer deaktivierst und alle Komponenten absolut positionierst, machst du deine Anwendung kurz gesagt für viele Nutzer unbedienbar/unbrauchbar.

Ich habe es gerade einmal auf unterschiedlich großen Bildschirmen mit verschiedenen Auflösungen geprüft: Es ist nicht möglich, auf alle Felder zuzugreifen oder Elemente werden deplatziert und die falsche Berechnung der Fensterpositionierung (s.o.) verschiebt das Fenster zusätzlich zu weit nach oben, sodass ein Nutzer nicht einmal die Titelleiste bedienen kann.

Verwende daher immer Layout Manager zur Anordnung deiner Komponenten. Die setBounds-Methode wird somit obsolet. Nutzer bei der Fensterskalierung einzuschränken, solltest du außerdem möglichst vermeiden. Tutorials, die dir deinen bisherigen Weg anweisen, sollten definitiv gemieden werden.

Sollte dir das alles zu kompliziert sein, dann setze den GUI-Designer ein, der im Java Editor integriert ist. In anderen IDEs gibt es ebenfalls solche Tools, meist sogar noch deutlich weiterentwickelter (Bsp.: WindowBuilder in Eclipse, GUI Designer in IntelliJ IDEA, Matisse in NetBeans).

8) Für die Darstellung fixer Texte sind JLabel-Objekte gedacht. Die JTextField-Komponente hingegen ist für Nutzereingaben konzipiert.

Beispiel:

var welcomeLabel = new JLabel("Willkommen bei Sudoku!", SwingConstants.CENTER);
welcomeLabel.setFont(new Font("Arial", Font.PLAIN, 37));
add(welcomeLabel);

9) Die einzelnen Schleifen zum Setup der Felder (Rahmen, Sichtbarkeit, Editierbarkeit, usw.) machen keinen Sinn. Dein Programmcode wird lediglich länger und die Programmausführung langsamer.

Eine Schleife, die alles in einem Rutsch macht, genügt. Die kann sogar direkt mit der Anordnung der Felder (in einem GridLayout) kombiniert werden.

var fieldBoard = new JPanel(new GridLayout(Board.SIZE, Board.SIZE, 2, 2));

for (int i = 0; i < fields.length; ++i) {
  fields[i] = new JNumberField();
  fields[i].setPreferredSize(new Dimension(152, 152));
  fields[i].setBorder(BorderFactory.createLineBorder(Color.BLACK));
  fields[i].setEditable(false);
  fields[i].setHorizontalAlignment(JFormattedTextField.CENTER);
  fields[i].setFont(new Font("Arial", Font.PLAIN, 100));
  fields[i].setBackground(Color.WHITE);
  fields[i].clear();
  fieldBoard.add(fields[i]);
}

10) Wie schon oben angedeutet, könntest du dein Layoutwechsel viel einfacher mit einem CardLayout lösen.

private final CardLayout layout;

// in constructor:
layout = new CardLayout();
setLayout(layout);

setupMenu();
setupBoard();

In den beiden setup-Methoden baust du dir jeweils ein Panel zusammen, welches den Startbildschirm / das Spielfeld repräsentiert und fügst es dem Frame als Kindknoten hinzu.

Beispiel für den Startbildschirm (setupMenu):

var levelButtonPanel = new JPanel(new FlowLayout(FlowLayout.LEFT, 50, 0));
levelButtonPanel.setBackground(Color.WHITE);
levelButtonPanel.setBorder(BorderFactory.createEmptyBorder(20, 0, 20, 0));

var label = new JLabel("Which difficulty level?", SwingConstants.CENTER);
label.setFont(new Font("Arial", Font.PLAIN, 25));
levelButtonPanel.add(label);
		
var easyLevelButton = getLevelButton("Easy", Color.GREEN);
easyLevelButton.addActionListener(event -> setupBoard(level));
levelButtonPanel.add(easyLevelButton);

/* add medium and hard level button, too ... */

var welcomePanel = new JPanel(new BorderLayout());
welcomePanel.setBackground(Color.WHITE);
var welcomeLabel = new JLabel("Welcome!", SwingConstants.CENTER);
welcomeLabel.setFont(new Font("Arial", Font.PLAIN, 37));
welcomePanel.add(welcomeLabel);

var menuPanel = new JPanel(new BorderLayout());
menuPanel.add(levelButtonPanel, BorderLayout.PAGE_START);
menuPanel.add(welcomePanel);
add(menuPanel);

In den Action Handlern kannst du anschließend zwischen den Panels hin- und herwechseln.

layout.first(getContentPane()); // show menu
layout.last(getContentPane()); // show board

11) Eine Validation ist nicht schwer, wenn du eine Struktur verwendest, wie ich sie oben vorgeschlagen habe. Du brauchst dann nämlich nur das von der GameFactory aktuell gewählte Board und kannst dann mit einer Schleife die Werte vergleichen.

private Board currentBoard;

// in validation action handler:
var correctValues = currentBoard.getSolution();

for (int i = 0; i < fields.length; ++i) {
  var field = fields[i];
  var fieldValue = field.getInt();

  if (fieldValue == correctValues[i]) {
    field.setBackground(Color.GREEN);
  }
  else {
    if (fieldValue > 4 || fieldValue < 0) {
      JOptionPane.showMessageDialog(this, fieldValue + " is an invalid number.");
    }

    field.setBackground(Color.RED);
  }
}

Zusätzliche Aktionen einzubauen (wie das Leeren der falschen Felder), sollte dann auch nicht so schwer sein.

Die zwei Methoden "nullen_rein /raus" wollte ich als Umweg nutzen, da ich nie das Numberfield auf eine Leere testen konnte.

Wird das denn überhaupt benötigt? Die getInt-Methode sollte bei einem leeren Feld oder invalidem Wert doch sicherlich eine Null zurückliefern. Somit stimmt der Wert mit der Lösung auch nicht mehr überein.

Wenn du explizit auf invalide Werte vorprüfen möchtest, wäre diese Klasse vermutlich nicht die beste Wahl. Es ist, das sollte dazugesagt werden, eh keine Swing-Standardkomponente, sondern eine Implementation, die nur der Java Editor mitliefert. Wie die konkret aussieht, kann ich nicht auf Anhieb sagen. Vielleicht handelt es sich lediglich um eine Subklasse von JTextField. Dann solltest du den Feldwert auch über getText erhalten.

Eine fixe Eigenimplementation wäre ansonsten nicht schwer.

import javax.swing.*;

public class JIntegerField extends JTextField {
  public void clear() {
    super.setText("");
  }

  public int getInt() {
    var text = getText();

    if (text.trim().length() == 0) {
      return 0;
    }

    return Integer.parseInt(getText());
  }

  public void setInt(int number) {
    super.setText(number + "");
  }

  @Override
  public void setText(String text) {
    if (isValid(text)) {
      super.setText(text);
    }
  }

  private boolean isValid(String text) {
    /* check something ... */
  }
}

Comments

[Lymea]

Das ist eine sehr gute Antwort! Vielen Dank :) Mittlerweile läuft es sogar :) Bin in der 13. Klasse, deswegen konnte ich nur mit dem arbeiten, was ich gelernt habe, aber mit der Kritik konnte ich sehr viel anfangen. Dankeschön :)

Answer 2

[FrageRunde59125]

Dein Code ist schwer lesbar. Also nicht nur optisch auch strukturell.

Comments

[Lymea]

Ist mir bewusst, hab nicht damit gerechnet hier zu fragen. Aber Danke