0

I'm quite new to C++ and I wanted to start out with a program generating 4x4 sudoku grids. I'm just getting started, but for some reason my program prints out 4 numbers in a line (different ones, so that's nice) but then loads endlessly. I know it's probably a stupid qestion, but here's my code if anyone might care to have a look (don't worry about all the #include, I tried different things out) thanks a lot!

EDIT : I forgot to tell, the program started doing that once I implemented the srand(time(NULL)) in it

EDIT2 : This isn't because I'm calling srand several times, I tried a different version with srand before both for and it's still the same

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <stdio.h>
#include <stdlib.h>

using namespace std;

int main() {
  int nbs[4][4];
  int l = 0;
  int c = 0;
  int val;
  int check;
  for (l = 0; l < 4; l++) {
    srand(time(NULL));
    for (c = 0; c < 4; c++) {
      check = 0;
      while (check == 0) {

        nbs[l][c] = rand() % 4 + 1;
        val = nbs[l][c];
        if (val == nbs[l][c - 1]) {
        }

        else if (val == nbs[l][c - 2]) {
        } else if (val == nbs[l][c - 3]) {
        } else if (val == nbs[l - 1][c]) {
        } else if (val == nbs[l - 2][c]) {
        } else if (val == nbs[l - 3][c]) {
        } else {
          check = 1;
          cout << nbs[l][c];
        }
      }
    }
    cout << "\n";
  }
  cout << "hello world!";
  return 0;
}
Biffen
  • 6,249
  • 6
  • 28
  • 36
  • 4
    All the `if (val==nbs[l][c-1]){}` access the array out of bounds. That causes undefined behavior. You start with `c=0`, `l=0` and access `nbs[l][c-1]`. What do you expect from `nbs[0][-1]`? –  Apr 09 '21 at 09:28
  • 3
    Does this answer your question? [srand() — why call it only once?](https://stackoverflow.com/questions/7343833/srand-why-call-it-only-once) – Zoe Apr 09 '21 at 09:29
  • 1
    Usage of `srand` is wrong, but not the problem, so I don't see why this would be a duplicate. – Devolus Apr 09 '21 at 09:34
  • @jabaa I thought this would contain 0, do you have an idea for another way to do it? – thibault Seidel Apr 09 '21 at 09:34
  • How could you leave the while loop if `nbs[0][0] == 0`, `nbs[0][1] == 1`, `nbs[0][2] == 2`, `nbs[1][0] == 3`, `nbs[2][0] == 4`. There is no way out. All the `if`/`else if` blocks are empty and you can't enter the `else` block. You shouldn't try to fix your code. You should go a step back and try to fix your algorithm. That's the actual problem. Your algorithm contains cases with infinite loops. –  Apr 09 '21 at 09:35
  • @jabaa that's done it, I put for each if/else if bounds checks like you said, thanks a lot! – thibault Seidel Apr 09 '21 at 09:43
  • To solve your out of bounds problem you could add bounds checks like: `if (c >= 1 && val==nbs[l][c-1]){}` –  Apr 09 '21 at 10:04

1 Answers1

0

Your code tries to generate random 4x4 grids with unique values in each row and column. But in your algorithm you're randomly filling cells and go on to the next cell without a check if this grid can even be completely filled, e.g.

1 3 2 4
4 2 1 3
3 4 X

will block your code in an infinite loop. There is no valid value for X.

You have to improve your algorithm, e.g. by using a backtracking approach.

Here is a simple fixed version of your code. It doesn't use backtracking but stops 20 attempts to find a value:

#include <iostream>
#include <cstdlib>
#include <ctime>

int main() {
    srand(time(NULL));
    int nbs[4][4];
    for (std::size_t l = 0; l < 4; l++) {
        for (std::size_t c = 0; c < 4; c++) {
            int check = 1;
            while (check>0) {
                nbs[l][c] = rand() % 4 + 1;
                auto val = nbs[l][c];
                if (c >= 1 && val == nbs[l][c-1]){++check;}
                else if (c >= 2 && val == nbs[l][c-2]){++check;}
                else if (c >= 3 && val == nbs[l][c-3]){++check;}
                else if (l >= 1 && val == nbs[l-1][c]){++check;}
                else if (l >= 2 && val == nbs[l-2][c]){++check;}
                else if (l >= 3 && val == nbs[l-3][c]){++check;}
                else {
                    check=0;
                    std::cout << nbs[l][c];
                }
                if (check > 20) {
                    std::cerr << "No solution found\n";
                    return EXIT_FAILURE;
                }
            }       
        }
        std::cout << '\n';
    }
    return EXIT_SUCCESS;
}