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

Recursive backtracking Sudoku solver

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

Problem

I'm trying to learn more algorithmic techniques and I came across an interesting application of recursive backtracking: solving a Sudoku puzzle. I'm looking for a review concerning code style, quality, algorithmic correctness, and maybe tips on how to extend the algorithm/program.

#include 
#include 
#include 
#include 
#include 

template 
class SudokuBoard {
private:
    std::array arr_;
    static const T UNINITIALIZED = 0;

    const T& at(size_t r, size_t c) const { return arr_[(r * N) + c]; }
    T& at(size_t r, size_t c) { return arr_[(r * N) + c]; }

    bool find_uninitialized_location(size_t &r, size_t &c) const
    {
        for (r = 0; r 
std::ostream& operator& board)
{
    for (size_t i = 0; i  board(values);
        std::cout << "The board: \n" << board;

        if (board.solve()) {
            std::cout << "The solved board: \n" << board;        
        } else {
            std::cout << "The board has no solution!\n";
        }

    } catch (const std::invalid_argument &e) {
        std::cerr << "Caught an invalid argument exception: " << e.what() << "\n";
    } catch (...) {
        std::cerr << "Unknown error caught.\n";
    }
}

Solution

-
Template arguments

-
Since the row, column and a block must accommodate the same set of numbers, N is necessarily a square number. I recommend to template the board on the modulus M, deduce N as M^2, and use M everywhere you use 3.

-
Templating on T looks a bit strange. At least I don't see what advantage it may serve.

-
Unnecessary computations

Instead of recomputing uninitialized locations at each call to solve I recommend to compute the vector of such locations once, and pass it around by reference, removing and restoring locations, along the lines of

bool solve(std::vector>& uninitialized)
    {
        if (uninitialized.size() == 0)
            return true;

        auto row = uninitialized.back.first;
        auto col = uninitialized.back.second;
        uninitialized.pop_back();

        for (size_t num = 1; num <= N; ++num) {
            if (is_safe(row, col, num)) {
                at(row, col) = num;

                if (solve(uninitialized)) {
                    return true;
                }
            }
        }

        at(row, col) = UNINITIALIZED;
        uninitialized.push_back(std::make_pair(row, col));
        return false;
    }


Please notice that you shall iterate until num

-
Magic numbers
3 and 9`

addressed above.

Code Snippets

bool solve(std::vector<std::pair<int, int>>& uninitialized)
    {
        if (uninitialized.size() == 0)
            return true;

        auto row = uninitialized.back.first;
        auto col = uninitialized.back.second;
        uninitialized.pop_back();

        for (size_t num = 1; num <= N; ++num) {
            if (is_safe(row, col, num)) {
                at(row, col) = num;

                if (solve(uninitialized)) {
                    return true;
                }
            }
        }

        at(row, col) = UNINITIALIZED;
        uninitialized.push_back(std::make_pair(row, col));
        return false;
    }

Context

StackExchange Code Review Q#117878, answer score: 3

Revisions (0)

No revisions yet.