Yay! I spent all day yesterday and much of today rewriting my matrix class

changing to a () indexer instead of a [][] indexer only to find that the following code will no longer compile: work.

1
2
3
4
5
6
7
8
9
void sudoku::cleargrid()
{
    for (int i = 0; i < SIZE; ++i)
        {
            for (int j = 0; j < SIZE; ++j)
                //SU[i][j] = 0;
                SU(i, j) = 0;
        }        
}


where SU is a matrix. Previously with SU[i][j] = 0; it had worked fine, but now somehow it is resetting the size of possibilities (another matrix member of my sudoku class).

Why is this happening to me? How can I fix this without adding code to my cleargrid function such as:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
void sudoku::cleargrid()
{
    int tr = possibilities.myRows, tc = possibilities.myCols;
    for (int i = 0; i < SIZE; ++i)
        {
            for (int j = 0; j < SIZE; ++j)
                //SU[i][j] = 0;
                SU(i, j) = 0;
        }        
    if (possibilities.myRows != tr || possibilities.myCols != tc)
        {
             possibilities.myRows = tr;
             possibilities.myCols = tc;
        }
}


Which provides a fix but is sure mindboggling to me how the error is happening in the first place.

Plus i have other errors I need to concentrate on since rewriting my matrix class.

Thanks in advance for your help!!!


edit: it seems like it is resetting the size of possibilities at each of the following iterations of the for loop:

i, j = 2 8
i, j = 3 4
i, j = 3 5
i, j = 4 0
i, j = 4 1
Last edited on
Sounds like buffer overflow / memory corruption.

Please post your () operator and your matrix class.
my matrix class is over 250 lines long but here is my () operator:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
template <class itemType>
itemType &
matrix<itemType>::operator ( ) ( int k, int y)
{
    if (k < 0 || myRows <= k)
    {
        cerr << "Illegal row index: " << k << " max index = ";
        cerr << myRows-1 << endl;        
        exit(y);
    }
    if (y < 0 || myCols <= y)
        {
            cerr << "Illegal column index: " << y << " max index = " ;
            cerr << myCols-1 << endl;
            exit(k);
        }
    return myMatrix[k][y];

}


template <class itemType>
const itemType &
matrix<itemType>::operator ( ) (int k, int y) const
{
    if (k < 0 || myRows <= k)
    {
        cerr << "Illegal row index: " << k << " max index = ";
        cerr << myRows-1 << endl;
        exit(y);
    }
    if (y < 0 || myCols <= y)
        {
            cerr << "Illegal column index: " << y << " max index = ";
            cerr << myCols-1 << endl;
            exit(k);
        }
    return myMatrix[k][y];
}


edit: I forgot to mention that possibilities is declared as matrix<dlist> possibilities;
but I don't know if this should make any difference or not.
Last edited on
also problem with the following line of code in my dlist class:

1
2
3
4
5
6
7
8
9
10
11
12
13
bool dlist::addN(const int & x, string s)
{    
    static int counter;
    if (x <= 0) {cout << "out of bounds error from dlist::addN(const int & x, string s). " << x << endl;  cout << "s = " << s << endl; exit(1); }
    if (!digits[x-1])
        {
              ++counter; cout << "counter = " << counter << "\n";
              digits[x-1] = true;      //doesn't make it here but it used to before I switched to using ()                                                 //instead of [][]
         }

    //return new bool;
    if (digits[x-1]) return true;
}


technically it makes it to that line of code some 171 times before crashing.
Last edited on
WTF!!!
I find this to be so bewildering. I am using the exact same computer as the computer I was working with yesterday. All I did was change a couple hundred of lines from [][] to () to try to speed up my program.

i find that having done that the following line of code:
 
    SIZE = (size * (size = rows));

with input rows = 3
was setting size equal to 3 and SIZE equal to 9

but after testing my program with the latest changes it was now assigning
size equal to 3, but SIZE was assigned to 6875532.

This doesn't make any sense at all.

I'm going to take the rest of the day off I think, maybe in the morning I will have thought of a solution, or at least that would have given you guys more time to respond...
Last edited on
SIZE = (size * (size = rows));

This is undefined behavior.

You must never use a variable in the same statement that you assign it.

Change it to this:

1
2
size = rows;
SIZE = size * size;


Cramming everything on the same line is generally to be avoided. There's nothing wrong with spreading things out. It increases clarity and avoids nasty "gotchas" like what you just experienced.
Last edited on
well does that explain why it would work as expected before I had changed the implementation method of my matrix class and not afterwards? All I did mind you was change the indexing operator from [][] to (), granted I rewrote tons of code, I didn't touch "SIZE = (size * (size = rows));".

Maybe I will go back to using [][]. The only reason why I switched in the first place was upon running a test as follows:
1
2
3
4
5
6
7
8
    typedef int matrix[NINE][NINE];
    int counter = 0;
    int begin = clock();
    while (clock() - begin < 10000 && ++counter)
        //apmatrix<int> x;  //uncomment this line to test speed of apmatrix
        //matrix x;              //uncomment this line to test speed of matrix
        ;
    cout << "You declared " << counter << " matrices.\n";


I get on average 7 times as more matrix declarations as I do apmatrix (which uses the [][] operator).

I was able to similarily compile the same test with my new matrix.h class and it gives similar results, with about 387,179,376 iterations in 10 seconds for matrix<int> x; and only 54,226,347 in 10 seconds for apmatrix<int> x;.

Is worth it? I won't be able to see the exact results until i get my sudoku class with my matrix.h implementation working correctly, but it may be I may be able to achieve more optimum results by focussing my energies on reworking the recursive algorithm that generates random puzzles instead.



or at least that would have given you guys more time to respond...


Sorry that sounded bad. I didn't mean to suggest that I am demanding help. All the help that I have received in the past and may continue to receive I am very greatful for. I don't know what I would do without this forum.
Topic archived. No new replies allowed.