0

Im trying to implement a function that searches a multidimensional array finds out if a value is in it, then moving that function. My search function

 bool search(int value, int values [][d], int n)
 {   
     bool In = false;
 //d is an it that is the maximum length and height 
 //e.g 3x3 or 4x4 as in the image below

 for(int row=0; row<d; row++)
 {  
    for(int col=0; col<d; col++)
    {   
        if(values[row][col] == value)
        {   
            //checking if this loop is executed
            printf("EXECUTED!! :) \n");
            In=true;
        }
         printf("Row:%i & Col%i: %i \n",row,col,values[row][col]);

    }
 }

//Usleep for debugging purpouses 
// as another function clears the screen
 usleep(50000000);

 if(In==true){return true;}
 if(In==false){return false;}


}

This is what is printed which is weird as to print the 4x4 box above i used the same array and the search function does not alter the array in anyway. This is my "move" function

bool move(int tile)
{   

if(search(tile,board,d))
{
    printf("please execute this code pretty please clang\n");
    return true;
}
else
{   
    printf("NOO\n");
    return false;
}

}

And here is the function that initilazes the variable in the first place

void init(void)
{
    bool even;

if((d & 1) == 0)
{
    even = true;
}
else
{
    even = false;
}

int value = d*d - 1;

for(int row =0; row<d; row++)
{
    for(int col=0; col<d; col++)
    {
        board[row][col]=value;
        value--;
    }
}

  //for this game to work if d is even the values of the third
  // and second last arrays must be switched
  if(even==true)
    {  
        int temp = board[d-1][d-2];
        board [d-1][d-2] = board[d-1][d-3];
        board [d-1][d-3] = temp;

    }
}

EDIT: Here is the pastebin for the full code http://pastebin.com/yS8DDEqZ Note Cs50 is a custom libary that was implemented by the class im taking, it defines a string a helper functions which get user input GetInt() etc.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Intent Filters
  • 189
  • 1
  • 15
  • let's see how you initialize `board` – Ed S. Jan 28 '14 at 21:16
  • 5
    note: `return In;`. don't waste 8 lines on such a simple thing. – Karoly Horvath Jan 28 '14 at 21:16
  • You still didn't show the declaration of `board`. – Ed S. Jan 28 '14 at 21:18
  • @EdS. Done, :Karoly Hovarth sorry i dotn understand but what do you mean? – Intent Filters Jan 28 '14 at 21:20
  • 1
    `if(In==true) { return true; } ....` – Karoly Horvath Jan 28 '14 at 21:21
  • In the same vein, below, just `bool even = d % 2;` would do. – Jens Gustedt Jan 28 '14 at 21:22
  • `int values [][d]` in `bool search(...` seems very wrong. Unless you use C99, this should not even compile. – kuroi neko Jan 28 '14 at 21:25
  • @KarolyHorvath if i do that how can i write an else statement without making it too messy? other than just writing if(In==false) { return false; } – Intent Filters Jan 28 '14 at 21:25
  • @JensGustedt `(d & 1) == 0` will be much faster since it's just a bitwise operator, while the other performs an arithmetic operation, anyway, compilers may optimize an replace one for the other. – ichramm Jan 28 '14 at 21:25
  • @kuroineko i dont understand that part, i found a solution online that said to put the [d] there to let it compile. Can you tell me a bettet way to do it? – Intent Filters Jan 28 '14 at 21:27
  • Not done. Where is the *declaration*, i.e., `int board[x][y]`? – Ed S. Jan 28 '14 at 21:27
  • @IntentFilters: you just `return In`. It's already a boolean. – Ed S. Jan 28 '14 at 21:27
  • here is the pastebin for the full code http://pastebin.com/yS8DDEqZ – Intent Filters Jan 28 '14 at 21:29
  • 1) could you tell us if you are compiling plain old C or C99? 2) Could you point me to this online solution saying "you should put d there"? 3) could you post the code where you actually declare your array (the one that is passed to your search function)? – kuroi neko Jan 28 '14 at 21:31
  • @ichramm, much faster? Any modern compiler will compile this into exactly the same code, `%2` is one of the simplest optimizations, ever. Only that `%` shows exactly what the code seems to intend. A typical case of premature optimization. – Jens Gustedt Jan 28 '14 at 21:34
  • 1) Im using C99, 2) Here is [the site](http://stackoverflow.com/questions/10003270/gcc-array-type-has-incomplete-element-type) i found it by just typing in the error code array has incomplete element type 'int []' into google 3) Done – Intent Filters Jan 28 '14 at 21:34
  • is `d` a `#define` or a variable ? – woolstar Jan 28 '14 at 21:37
  • d is a variable it's value is decided by the user at run time – Intent Filters Jan 28 '14 at 21:45
  • You're treating a 9x9 array as one with smaller dimensions... you might get away with that for a single dimension, but with two your rows don't match up in memory. – Dmitri Jan 28 '14 at 21:46
  • Ok i think i understand what you mean. But why dont i get the same bug when printing to the screen? – Intent Filters Jan 28 '14 at 21:47
  • As a global, you're treating a 9x9 as a 9x9... in your other function, you treat it as 4x4 (as in the function header). So when you access the global, your rows are 9 `int`s apart, and in your search function they're 4 `int`s apart (with d=4). – Dmitri Jan 28 '14 at 22:08
  • Ahh i get it now. Any ideas on how i can search without getting an incomplete element type error? – Intent Filters Jan 28 '14 at 22:11

1 Answers1

1

Okay, I got it.

You're defintely using a C99 compliant compiler, that allows variable length arrays.

Relevant extracts from your code:

#define MAX 9
int board[MAX][MAX]; // <- board is an int[9][9]

int d; // <- d is a global variable

bool search(
    int value,
    int values[MAX][d], // <- tells compiler to handle values as int[9][d]
    int n); 

// from within init()
for(int row =0; row<d; row++)
for(int col=0; col<d; col++)
    board[row][col]=value--; // <- board inited as an int[9][9]

A fixed size array is a big lump of contiguous memory, with rows stored one after the other.

Example:

int a[2][3] is stored as 6 ints corresponding to:

a[0][0]
a[0][1]
a[0][2]
a[1][0]
a[1][1]
a[1][2]

here board memory layout is 9x9 :

000000000
111111111
222222222
333333333
444444444
555555555
666666666
777777777
888888888

or in linear memory:

000000000111111111222222222333333333444444444555555555666666666777777777888888888

Now if d is 4 as your screenshot shows, the search function will think its layout is :

0000
1111
2222
3333
4444
5555
6666
7777
8888

or in linear memory:

000011112222333344445555666677778888

Your init function uses the 9x9 layout, so it puts the values like this:

15 14 13 12 x x x x x
11 10  9  8 x x x x x etc.

but your search function reads them like :

15 14 13 12
 x  x  x  x 
 x 11 10  9
 8  x  x  x 
etc.

Basically, you declared a structure for your array in the prototype of search() that is inconsistent with its declaration.

You recklessly violated one of the C little known golden rules :
always keep function parameters and array declarations consistent.
and C punished you with a cryptic bug.

Read this little essay of mine for more details.

Community
  • 1
  • 1
kuroi neko
  • 8,479
  • 1
  • 19
  • 43
  • Thanks a lot that makes sense. Sorry about this but I've got two questions firstly how did you get from a[0][0] 000000000 and etc. And 2) how can i fix the bug? – Intent Filters Jan 28 '14 at 22:21
  • I don't get your first question. As for the second, well, declare your parameter consistently, i.e. `int search (... int values[MAX][MAX], int d)` or use the global `board` array in your function. – kuroi neko Jan 28 '14 at 22:25