HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Creating Sudoku generator and validator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
creatingvalidatorgeneratorandsudoku

Problem

I've created two classes that contain only sudoku logic. One class Sudoku generates sudoku grid and returns SudokuGrid object with has specific number of values in it(removing values from filled grid). Second class SudokuGrid contains all logic that player needs, checking whether any rules are broken. In my opinion Sudoku class turned out great, but SudokuGrid class not so much, I tried to clean it up as much as I could and this is best I could do:

SudokuGrid class:

```
package application;

import java.awt.*;
import java.util.*;
import java.util.List;
import java.util.stream.Collectors;

public class SudokuGrid {

/**
* Contains all values that grid can contain
*/
public enum SudokuValue {
ONE("1"),
TWO("2"),
THREE("3"),
FOUR("4"),
FIVE("5"),
SIX("6"),
SEVEN("7"),
EIGHT("8"),
NINE("9"),
EMPTY(" ");

public final String TEXT;

private SudokuValue(String text) {
this.TEXT = text;
}
}

/**
* Helps this class with point creating, validating
*/
private static class SudokuPoint {

/**
* @param point - arbitrary point to check
* @return true if point is ing grid else return false
*/
private static boolean isValidPoint(Point point) {
return point.x getInvalidFields() {
Set fields = new HashSet<>();
fields.addAll(_getInvalidLineFields());
fields.addAll(_getInvalidRowFields());
fields.addAll(_getInvalidBlockFields());
return fields;
}

/**
* @return set of points with make contradiction in sudoku grid
* if there are two same values in LINE returns these values coordinates
* otherwise returns empty set
*/
private Set _getInvalidLineFields() {
Set fields = new HashSet<>();

for(int line = 0; line lineValues = Arrays.asList(grid[line]);

fields.addAll(_get

Solution

Parameterized Sudoku

In a standard Sudoku grid,
there should be \$n^2\$ blocks of \$n^2\$ numbers,
typically \$3^2 = 9\$ blocks of \$3^2 = 9\$ numbers,
arranged in a square.
As such, the LINES, ROWS, BLOCK_SIZE variables are a bit confusing,
as they could all be derived from \$n = 3\$.

It would be nice if \$n\$ was a parameter of the Sudoku generator / validator, to work with standard sudoku grids of arbitrary sizes,
instead of using those hardcoded static constants.

Class design

The class design and organization have many strange, unnatural elements.
To name just the few most prominent ones:

-
SudokuGrid hardcodes the game parameters in static constants and an enum. This effectively prevents extending the game to other setups with \$n <> 3\$.

-
java.awt.Point shouldn't be used in this Sudoku model. java.awt.Point is a graphical element. To follow proper separation of MVC principles, objects from graphical libraries don't belong here. Just because you want to put x and y somewhere and there is an existing class, that doesn't make it suitable.

Coding style

There are many coding style issues.
To highlight some of the most prominent:

  • Creating multiple instances of Random is both inefficient and unnecessary. You could create just one and reuse it



  • throw new AssertionError without a message parameter to explain itself



  • It's not a common convention to prefix method names with _. Stick to camelCase



  • Some for loops can be replaced with for-each



  • Some variables are poorly named. For example in printGrid, the variable names index and index2 are not good.



  • On a class named SudokuGrid, the term "grid" in printGrid is redundant

Context

StackExchange Code Review Q#92157, answer score: 5

Revisions (0)

No revisions yet.