Check if two numbers are the same in a box

Question

I am making a Sudoku game and I have managed to add random numbers, add numbers, and to checklines (if there is 2 numbers on the same line). I am trying to check the 3x3 boxes if the number is twice or more in the same box. I have managed to do it one way, but is this the easiest way? In that case I need to do this 81 more times, or is there a better way?

 bool checkBox(){
int box1[3][3];

for (int i = 0; i <= 2; i++){
    for (int j = 0; j <= 2; j++){
        box1[i][j] = board[i][j];
    }
}

if (box1[0][0] != 0){
    if (box1[0][0] == box1[0][1] || box1[0][0] == box1[0][2] || box1[0][0] == box1[1][0] || box1[0][0] == box1[1][1]
        || box1[0][0] == box1[1][2] || box1[0][0] == box1[2][0] || box1[0][0] == box1[2][1] || box1[0][0] == box1[2][2]){
        return true;
    }
}

Answer 1

Avoid magic numbers

int box1[3][3];

for (int i = 0; i <= 2; i++){
    for (int j = 0; j <= 2; j++){

Rather than manually strewing 3s and 2s throughout the code, just declare a constant:

const int SQUARE_SIZE = 3;

Then you can say things like

int box1[SQUARE_SIZE][SQUARE_SIZE];

for (int i = 0; i < SQUARE_SIZE; i++) {
    for (int j = 0; j < SQUARE_SIZE; j++) {

Changing the <= to < allows use of the same constant in both places.

Why copy?

if (box1[0][0] != 0){
    if (box1[0][0] == box1[0][1] || box1[0][0] == box1[0][2] || box1[0][0] == box1[1][0] || box1[0][0] == box1[1][1]
        || box1[0][0] == box1[1][2] || box1[0][0] == box1[2][0] || box1[0][0] == box1[2][1] || box1[0][0] == box1[2][2]){
        return true;
    }
}

You copy a bunch of values from board to box1. Why?

You could just replace all that with

if (board[0][0] != 0) {
    return false;
}

if (board[0][0] == board[0][1] || board[0][0] == board[0][2]
 || board[0][0] == board[1][0] || board[0][0] == board[1][1] || board[0][0] == board[1][2]
 || board[0][0] == board[2][0] || board[0][0] == board[2][1] || board[0][0] == board[2][2]) {
    return true;
}

Or the shorter

if (board[0][0] != 0) {
    return false;
}

for (int i = 0; i < SQUARE_SIZE; ++i) {
    for (int j = ((i > 0) ? 0 : 1); j < SQUARE_SIZE; ++j) {
        if (board[0][0] == board[i][j]) {
            return true;
        }
    }
}

return false;

Of course, that approach only really works on the corners.

Pick a different data structure or structures

You want to be able to answer the question: is there more than one box with the same number? So why not save the data in a structure than answers that? E.g.

const int BOARD_SIZE = 9;

bool available_rows[BOARD_SIZE][BOARD_SIZE + 1];

for (int i = 0; i < BOARD_SIZE; ++i) {
    available_rows[i][0] = false;
    for (int j = 1; j <= BOARD_SIZE; ++j) {
        available_rows[i][j] = true;
    }
}

Then whenever you go to use a number, you say something like

if (available_rows[row][number] && available_columns[column][number] && available_squares[row/SQUARE_SIZE][column/SQUARE_SIZE][number]) {
    available_rows[row][number] = false;
    available_columns[column][number] = false;
    available_squares[row/SQUARE_SIZE][column/SQUARE_SIZE][number] = false;

    board[row][column] = number;
} else {
    // throw exception?  Or otherwise handle an invalid selection.  
}

This does more work when selecting a number, but it self-maintains a valid board. This is a common result in computer science. It is often better to do work once and store it than to redo the same work repeatedly.

Note: but not always. :)

You can also build these structures inside an isValid function if you want.

bool isValid() {
    bool available_rows[BOARD_SIZE][BOARD_SIZE + 1];

    for (int i = 0; i < BOARD_SIZE; ++i) {
        available_rows[i][0] = false;
        for (int j = 1; j <= BOARD_SIZE; ++j) {
            available_rows[i][j] = true;
        }
    }

    bool available_columns[BOARD_SIZE][BOARD_SIZE + 1];

    for (int i = 0; i < BOARD_SIZE; ++i) {
        available_columns[i][0] = false;
        for (int j = 1; j <= BOARD_SIZE; ++j) {
            available_columns[i][j] = true;
        }
    }

    bool available_squares[SQUARES_PER_BOARD_SIDE][SQUARES_PER_BOARD_SIDE][BOARD_SIZE + 1];

    for (int i = 0; i < SQUARES_PER_BOARD_SIDE; ++i) {
        for (int j = 0; j < SQUARES_PER_BOARD_SIDE; ++j) {
            available_rows[i][j][0] = false;
            for (int k = 1; k <= BOARD_SIZE; ++k) {
                available_squares[i][j][k] = true;
            }
        }
    }

    for (int i = 0; i < BOARD_SIZE; ++i) {
        for (int j = 0; j < BOARD_SIZE; ++j) {
            if (board[i][j] == 0) {
                continue;
            }

            if (!available_rows[i][board[i][j]] || !available_columns[j][board[i][j]] || !available_squares[i/SQUARE_SIZE][j/SQUARE_SIZE][board[i][j]]) {
                return false;
            }

            available_rows[i][board[i][j]] = false;
            available_columns[j][board[i][j]] = false;
            available_squares[i][j][board[i][j]] = false;
        }
    }

    return true;
}

I'm assuming that this is inside a Board class, since you have direct access to the board variable in your original example.

This is off the top of my head. As others have noted, there are many more examples of Sudoku implementations that have thought through the ramifications of each choice at greater length. This is a common pattern that you should know, even if you choose not to use this particular solution here.