Sudoku Solver in Java

This is an assignment posted here.

This is my first foray into OO for me. Is this design OK? Or is there something very wrong that I'm not seeing at all. Any suggestions are most welcome and needed.

```
import java.util.*;

/*
* Encapsulates a Sudoku grid to be solved.
* This project is based on the assignment given in CS108 Stanford.
*/
public class Sudoku {

/* "Spot" inner class that represents a single spot
* on the grid of the Sudoku game.
* Each Spot object knows its place on the Sudoku grid as
* it stores the row and column number as fields.
*/
private class Spot implements Comparable {

/ Properties/fields of each individual Spot /
private int row, col;
private int value;
private int part;

/* Stores all possible values for a Spot that is empty
according to the rules of the game /
private HashSet possibleValues;

Spot(int x, int y, int val) {
row = x;
col = y;
value = val;
part = getPart(x, y);

possibleValues = new HashSet<>();
}

Spot(Spot s) {
this(s.row, s.col, s.value);
part = s.part;
possibleValues = new HashSet<>(s.possibleValues);
}

/ Sets the value for this Spot on the Solution Grid (solutionGrid) /
void setValue(int val) {
value = val;
}

/ Returns the value of this Spot /
int getValue() {
return value;
}

/ Returns the part of the grid where this Spot belongs /
int getPartForSpot() {
return part;
}

/ Returns true iff this Spot is not filled /
boolean isEmpty() {
return value == 0;
}

/* Returns a HashSet of all legal values that can be
* filled in this Spot.
*/
HashSet getPossibleValues() {
if (value != 0) return null;

the code is good in general, but there are some comments:

• Always try to expose abstract type rather than concrete implementations, so use List instead ArrayList, and Set instead of HashSet for fields types and return types, and this gives you more flexibility if you want to migrate to a different concrete type, OOP is about hiding implementation more than hiding data.

-
Use the this so you don't use horrible names for parameters, for example

Spot(int row, int col, int value) {
    this.row = row;
    this.col = col;
    this.value = value;    
}

• getPart is private, which is good, so it can't be overridden by subclasses because its being called in the constructor, but I would say declare it as final, so it stays safe even if you change its accessibility later

-
You can use String.valueOf instead of

@Override
public String toString() {
    return value + "";
}

StackExchange Code Review Q#66580, answer score: 11