1

I'm working on a program to generate random grids, latin squares, and sudoku. I am working on the latin squares and pretty much have it all working except I am in a continuous loop. If I break them up they work fine. There is probably something small I am doing wrong and I can't find it. Can you spot what's wrong?

EDIT: For those that don't know what Latin Square is (if someone doesn't know) its usually a 9x9 grid that doesn't have repeats in either rows nor columns.

UPDATE: I found a problem with the notSame equaling true right before if(notSame) statement. It was always equaling true so wouldn't finish checking the row. Now when I run it is no longer in continuous loop but instead the rows have no repeats but columns still do.

UPDATE #2: I now redid a lot of the coding for the columns. My professor asked me to change a few things but it still puts me in to a continuous loop.

int row = 0, col = 0, count = 0;
bool notSame = true;
// setting up rows and columns
for (row = 0; row < grid.GetLength(0); row++)
{
   for (col = 0; col < grid.GetLength(1); col++)
   {

       grid[row, col] = rnd.Next(1, 10);

       //for loop to check rows for repeats
       for (int c = 0; c < col; c++)
       {
           // if there is repeat go back a column and set bool = false
           if (grid[row, col] == grid[row, c])
           {
               col--;
               count++;
               notSame = false;
               break;
            }

            //notSame = true;
        }

     // if bool = true loop to check columns for repeats
     if (notSame)
     {

         for (int r = 0; r < row; r++)
         {
         // if repeat then go back row
            if (grid[row, col] == grid[r, col])
            {
                notSame = false;
                count++;
                break;
             }

          }
          if (notSame == false && count <= 50)
          {
               row--;
               //break;
          }
          else if (notSame == false && count > 50)
          {
               count = 0;
               col = 0;
               row = 0;
               break;
          }
      }
   }
}

I am using a 2D array called grid.

finnw
  • 47,861
  • 24
  • 143
  • 221
  • Have you stepped through it? You are probably getting duplicates to the point the it keeps reducing the row and col values(row-- and col--) and the for loop is never ending. Step though this with a debugger and you should be able to see this. – jzworkman Mar 28 '12 at 18:02
  • Am I right in thinking that you are randomly generating numbers, then checking if they fit, and if not randomly picking another number? If so, that's a pretty poor algorithm in the first place since it is not guaranteed to ever quit! I wrote some code to generate latin squares before. I'll see if I can find it. – Matt Burland Mar 28 '12 at 18:16
  • This is the algorithm I was told to use, whether good or bad, by my professor, whom is strict with grading. So I am using the algorithm he asked us to use. I know it's poor, but I still need to find what's wrong with it so I can move on to Sudoku part of the project. I found one mistake and it fixed it in a way but it's failing at checking the columns now. – Robert Joel Goitia Aten Mar 28 '12 at 18:21
  • Ok, you are stuck with this.. can you show the declaration of everything? once you say notSame is false, you never say it true again in any place... and that is your problem.. also you are confusing row with cols... – gbianchi Mar 28 '12 at 18:39
  • alsoooooooo: // if repeat then genereate new random and go back row if (grid[row, col] == grid[r , col]) { grid[row, col] = rnd.Next(1, 10); and you lost the check in columns??? ;) – gbianchi Mar 28 '12 at 18:45
  • Sorry about that, I will edit my original post to show the declarations. I'm assuming you tried to hint something with that post? But I didn't catch on. – Robert Joel Goitia Aten Mar 28 '12 at 18:45
  • @RobertJoelGoitiaAten: So did he give you this non-working code with the objective to fix it? Or did he give you a description on an algorithm in words to implement? – Matt Burland Mar 28 '12 at 19:05
  • Also, guessing from you indentation, are you missing a closing brace } from your code to close the "for (int c = 0; c < col; c++)" loop? – Matt Burland Mar 28 '12 at 19:08
  • @MattBurland Bad indentation on my part. Apologies. He gave us the algorithm and told us to use it. – Robert Joel Goitia Aten Mar 28 '12 at 19:12
  • @gbianchi I am not confusing rows with columns. To check a row for repeat you must go back a column and to check a column for repeats you have to go back a row. Look at the answer below with the code snippets and you will understand what I mean. – Robert Joel Goitia Aten Mar 28 '12 at 19:15

4 Answers4

2

My problem was an extra count in the check for rows. The count would always go over 50 and therefore would cause an infinite loop. To clarify:

grid[row, col] = rnd.Next(1, 10);

   //for loop to check rows for repeats
   for (int c = 0; c < col; c++)
   {
       // if there is repeat go back a column and set bool = false
       if (grid[row, col] == grid[row, c])
       {
           col--;
           count++;
           notSame = false;
           break;
        }

        //notSame = true;
    }

The count++ would increment and would at times end up > = 50 which would then kick in this code:

 else if (notSame == false && count > 50)
      {
           count = 0;
           col = 0;
           row = 0;
           break;
      }

Which then cause everything to be set back to 0 and would restart. Therefore, it caused an infinite loop. Thanks all for the help!

2

I do not know where your coding error is. But your algorithm is not very efficient.

Both Latin squares and sudokus are actually special cases of the "graph colouring" problem. That is, given a bunch of "nodes" that are arbitrarily "connected" together, find a way to colour each node so that no two nodes that are connected have the same colour.

This problem is in general quite difficult to solve quickly, but for the specific cases of sudokus and Latin squares it is pretty straightforward and can be done easily in C#. You create a "graph" that has 81 nodes, and each node is "connected" to the other nodes in its row and column. The "colours" are the numbers 1 through 9.

In my five-part series of articles I walk you through how to create an efficient graph colouring algorithm that can solve sudokus. It would not be difficult to adapt the algorithm to instead generate sudokus.

http://blogs.msdn.com/b/ericlippert/archive/tags/graph+colouring/

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • I like your blogs. I have to use this algorithm that i'm using now so I have no choice. I am definitely going to edit it to be using this algorithm after I hand it in. Very nice blogs you posted. – Robert Joel Goitia Aten Mar 28 '12 at 18:38
1

I think about the simplest way to generate a latin square is to start with a know latin square. Say:

1 2 3 4
2 3 4 1
3 4 1 2
4 1 2 3

Which is easy to generate for any size by cyclic permutations of rows, then simply assign each value in the grid (1,2,3,4) to another value, randomly generated. For example, you could simply shuffle a list of the numbers 1,2,3,4 and get, let's say 2,4,3,1. Now just replace the 1's in your square with the first entry in you shuffled list, the 2's with your second, and so on to give you:

2 4 3 1
4 3 1 2
3 1 2 4 
1 2 4 3

Now you can also shuffle the order of rows (and/or columns) as well if you wish and it should still be valid.

Edit: actually, thinking about this, it's probably easiest to just start with the first square and then shuffle the columns and rows. No need to do the substitution part.

Matt Burland
  • 44,552
  • 18
  • 99
  • 171
0

doing an explicit decrements of the variable that you are iterating with, is a no no. that's probably your problem. This sounds like a good place to use backtracking to avoid it :)

EDIT: I see saw many problems I don't know where to start. This algorithm will never give you what you need. First of all it can lead to a deadlock problem when you start filling it, and there is no way you can add a number to a particular row/column.. imagine that you have 12345 on row 5, and then you have on column 6 numbers 6 7 8 9.. well, you can't add a number to row 5, column 6 ;) see the problem there?? besides that, your code has several problems: getting the iteration variables change while iterating is a big problem and should be avoided.

once notSame = false; then it remain that way for the rest of your execution.

columns go vertical, and rows horizontal, so this (1,2) is row 1 column 2.. you are checking rows in the first phase.. and columns on the second..

// if bool = true loop to check columns for repeats
     if (notSame)
     {

         for (int r = 0; r < row; r++)
         {
             // if repeat then genereate new random and go back row
             if (grid[row, col] == grid[r , col])
             {
                 grid[row, col] = rnd.Next(1, 10);

that has a problem on itself.. if you change the number there, you should have been checked all the same that before!

tell you teacher to come here and read this ;).. I don't know how else to help you, this algorithm is totally wrong, and need a total refactor (and yes, you can do it using iteration, but not for, you need to use while and flags).

gbianchi
  • 2,129
  • 27
  • 35