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

Boggle solver in Java

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

Problem

This is a boggle solver in Java. I would like code reviewers to optimize the code, suggest better coding practices, and help make it cleaner.

I am also unsure of the complexity and would appreciate an explanation of it.

public final class Boggle {

    private static final NavigableSet dictionary;
    private final Map> graph = new HashMap>();

    static {
        dictionary = new TreeSet();
        try {
            FileReader fr = new FileReader("/Users/ameya.patil/Desktop/text.txt");
            BufferedReader br = new BufferedReader(fr);
            String line;
            while ((line = br.readLine()) != null) {
                dictionary.add(line.split(":")[0]);
            }
        } catch (Exception e) {
            throw new RuntimeException("Error while reading dictionary");
        }
    }

    private Boggle() {}

    public static List boggleSolver(char[][] m) {
        if (m == null) {
            throw new NullPointerException("The matrix cannot be null");
        }
        final List validWords = new ArrayList();
        for (int i = 0; i  validWords) {
        assert m != null;
        assert validWords != null;

        for (int i1 = Math.max(0, i - 1); i1  0) {
                        solve(m, i1, j1, word, validWords);
                    }
                }
            }
        } 
    }

    public static void main (String[] args) {
          char[][] board = { {'a', 'b', 'c', 'd' },
                             {'n', 'x', 'p', 'q' },
                             {'k', 't', 'i', 'w' },
                             {'e', 'f', 'g', 's' },
                          };

          List list = Boggle.boggleSolver(board);
          for (String str :  list) {
              System.out.println(str);
          }
    }
}

Solution

According to the rules, you can only use each tile once per word. You haven't attempted to keep track of tile usage.

Consider storing your dictionary using a trie, such as the implementation in Apache Commons. It should provide an efficient .prefixMap(), which is like your dictionary.subSet().

You defined graph, but you never use it.

I think that an object-oriented interface would feel natural. Create a new instance of Boggle by passing the char[][] matrix to the constructor.

Method names should be verbs, and shouldn't be redundant with the class name. Therefore, .boggleSolver() should just be .solve().

The purpose of the loop…

for (int i1 = Math.max(0, i - 1); i1 < Math.min(m.length, i + 2); i1++) {
    for (int j1 = Math.max(0, j - 1); j1 < Math.min(m[0].length, j + 2); j1++) {
        if (i1 != i || j1 != j) {
            // loop body
        }
    }
}


… is to iterate through all the neighbours of tile (i, j). A comment to that effect would be appreciated. Also, I would invert the if-condition to avoid a level of nesting:

// Iterate through the neighbours of tile (i, j)
for (int i1 = Math.max(0, i - 1); i1 < Math.min(m.length, i + 2); i1++) {
    for (int j1 = Math.max(0, j - 1); j1 < Math.min(m[0].length, j + 2); j1++) {
        // Skip the tile (i, j) itself
        if (i1 == i && j1 == j) continue;

        String word = prefix+ m[i1][j1];
        if (!dictionary.subSet(word, word + Character.MAX_VALUE).isEmpty()) {
            if (dictionary.contains(word)) {
                validWords.add(word);
            }
            solve(m, i1, j1, word, validWords);
        }
}


Note that the dictionary.contains(word) check can be moved inside the !dictionary.subSet(...).isEmpty() check.

Code Snippets

for (int i1 = Math.max(0, i - 1); i1 < Math.min(m.length, i + 2); i1++) {
    for (int j1 = Math.max(0, j - 1); j1 < Math.min(m[0].length, j + 2); j1++) {
        if (i1 != i || j1 != j) {
            // loop body
        }
    }
}
// Iterate through the neighbours of tile (i, j)
for (int i1 = Math.max(0, i - 1); i1 < Math.min(m.length, i + 2); i1++) {
    for (int j1 = Math.max(0, j - 1); j1 < Math.min(m[0].length, j + 2); j1++) {
        // Skip the tile (i, j) itself
        if (i1 == i && j1 == j) continue;

        String word = prefix+ m[i1][j1];
        if (!dictionary.subSet(word, word + Character.MAX_VALUE).isEmpty()) {
            if (dictionary.contains(word)) {
                validWords.add(word);
            }
            solve(m, i1, j1, word, validWords);
        }
}

Context

StackExchange Code Review Q#33045, answer score: 5

Revisions (0)

No revisions yet.