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

Checking validity of NxN Sudoku

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

Problem

Given a multi-dimensional array representing a board in Sudoku, the function should be able to return whether the sudoku is valid or not.

Examples:

var goodSudoku = [
  [7,8,4, 1,5,9, 3,2,6],
  [5,3,9, 6,7,2, 8,4,1],
  [6,1,2, 4,3,8, 7,5,9],

  [9,2,8, 7,1,5, 4,6,3],
  [3,5,7, 8,4,6, 1,9,2],
  [4,6,1, 9,2,3, 5,8,7],

  [8,7,6, 3,9,4, 2,1,5],
  [2,4,3, 5,6,1, 9,7,8],
  [1,9,5, 2,8,7, 6,3,4]
];

var badSudoku = [
  [1,1,1, 1,1,1,  2,2,2],
  [5,3,9, 6,7,2, 8,4,1],
  [6,1,2, 4,3,8, 7,5,9],

  [9,2,8, 7,1,5, 4,6,3],
  [3,5,7, 8,4,6, 1,9,2],
  [4,6,1, 9,2,3, 5,8,7],

  [8,7,6, 3,9,4, 2,1,5],
  [2,4,3, 5,6,1, 9,7,8],
  [1,9,5, 2,8,7, 6,3,4]
];

validSudoku(badSudoku); // false;
validSudoku(goodSudoku); // true;


Specific questions

  • How can I improve my function? What am I doing bad/good?



  • Is my algorithm efficient? Advice on it?



Here is my code

function validSudoku(data) {
    var valid = true, 
        temp = [], 
        data,
        side,
        slot;

    // Check wrong size
    if (data[0].length !== data.length) valid = false;

    // slot*slot
    slot = Math.sqrt(data.length);

    // Verifiy horizontal
    data.forEach(function(arr) {
        valid = valid && arr.every(function(val, i) { return arr.indexOf(i + 1) > -1; });
    });

    // Verifiy vertical lines
    data.forEach(function(arr, i) {
        temp  = data.map(function(val) { return val[i]; });
        valid = valid && arr.every(function(val, i) { return temp.indexOf(i + 1) > -1; });
    });

    // Verifiy boxes
    for (var i = 0; i  0) {
                for (var j = 1; j <= data.length; j++)
                    if (temp.indexOf(j) < 0) valid = false;                 
                temp = [];
            }

        });

    }
    return valid;
}

Solution

There are a few problems with your validation, and the code itself could be a bit neater.

First up, your code will try to validate impossible puzzles. Sudoku puzzles need to be square-sided You check that the puzzle is square, but your slot-size is not valid. It assumes a side that is a perfect square (4, 9, 16, etc.).

Additionally, you only check the first row is the right length, I suspect your code will validate as true if one of the rows has an extra element (with a carefully-chosen value).

In multiple places you set valid = false, and in those places you should just return false. You know the puzzle is not valid, so why continue checking?

Finally, it would be nice if you could make the checking generic. There are 3 things to check:

  • all expected elements in each row are present



  • all expected elements in each column are present



  • all expected elements in each mini-square are present.



I would try to put that together as a collection of functions for each check:

var extractions = [
    {   name: "rows",
        get:  function(data,span,block,member){
                  return data[block][member];
              }
    },
    {   name: "cols",
        get:  function(data,span,block,member) {
                  return data[member][block];
              }
    },
    {   name: "minis",
        get:  function(data,span,block,member) {
                  var coloff = (block % span) * span;
                  var rowoff = (block / span) * span;
                  return data[rowoff + (member / span)][coloff + (member % span)];
              }
    },
]

function isvalid(data) {
    var size = data.length;
    var slot = Math.sqrt(size);
    for (var block = 0; block < size; block++) {
        if (data[block].length != size) {
            return false;
        }
        for (var ext = 0; ext < extractions.length; ext++) {
            var tmp = [];
            for (var member = 0; member < size; member++) {
                tmp.push(extractions[ext].get(data, slot, block, member));
            }
            for (var i = 1; i <= size; i++) {
                if (tmp.indexOf(i) < 0) {
                    return false;
                }
            }
        }
    }
    return true;
}

Code Snippets

var extractions = [
    {   name: "rows",
        get:  function(data,span,block,member){
                  return data[block][member];
              }
    },
    {   name: "cols",
        get:  function(data,span,block,member) {
                  return data[member][block];
              }
    },
    {   name: "minis",
        get:  function(data,span,block,member) {
                  var coloff = (block % span) * span;
                  var rowoff = (block / span) * span;
                  return data[rowoff + (member / span)][coloff + (member % span)];
              }
    },
]

function isvalid(data) {
    var size = data.length;
    var slot = Math.sqrt(size);
    for (var block = 0; block < size; block++) {
        if (data[block].length != size) {
            return false;
        }
        for (var ext = 0; ext < extractions.length; ext++) {
            var tmp = [];
            for (var member = 0; member < size; member++) {
                tmp.push(extractions[ext].get(data, slot, block, member));
            }
            for (var i = 1; i <= size; i++) {
                if (tmp.indexOf(i) < 0) {
                    return false;
                }
            }
        }
    }
    return true;
}

Context

StackExchange Code Review Q#86895, answer score: 9

Revisions (0)

No revisions yet.