fgetc() doesn't work properly

Question

I have file that goes like 7 4 5 1 etc. I want to put these number in a multi-dimensional array.

for(x=0;x<9;x++)
       for(y=0;y<9;y++)
       {
         current=fgetc(fp);
         if(current!=EOF&current!=' '&current!='\n')
              sudokuArray[x][y] = current-'0';
       }

This code doesn't work properly. sudokuArray[0][0] gives the result true(first number in the file) but [0][1] gives some random number like 131231304. [0][2] gives what [0][1] supposed to be. Why is that?

I think you should swap the "for x" and "for y" loops. BTW I am missing the definition of current. Also: you want &&, not &. And you should add an else clause, setting the array to some special value, eg zero or -1. — wildplasser, Jun 07 '12 at 12:09
How is `sudokuArray` defined ? And how is `current` defined ? — Paul R, Jun 07 '12 at 12:10

score 1 · Answer 1 · answered Jun 07 '12 at 12:11

I am not sure why you use nested loops to read from file. I see one problem in your code:

Change & to &&:

     if(current!=EOF&current!=' '&current!='\n')

to:

 if(current!=EOF && current!=' ' && current!='\n')

&& is the logical AND and & is bitwise AND.

Since, you have only integers and you seem to know the exact no. of integers. You can simply use fscanf:

fscanf(fp, "%d", &arr[x][y]);

score 1 · Accepted Answer · edited Jun 07 '12 at 12:55

It's because if your inner loop:

for(y=0;y<9;y++)
{
    current=fgetc(fp);
    if((current!=EOF) && (current!=' ') && (current!='\n'))
        sudokuArray[x][y] = current-'0';
}

If your input is 7 4 5 1, then when y == 0, you read '7', which gets put in sudokuArray[0][0].

Now on your next loop, y == 1, and you read ' '. Because of your if statement, you don't put that in sudokuArray but y still gets incremented.

Next time you do the loop, y == 2, you read '4' and it gets put in sudokuArray[0][2].

So fgetc() does work properly and your code is doing exactly what you told it to do. Code is very obedient that way.

Edit: Also note your if statement should contain && instead of &. They are different operators. A little whitespace and some parenthesis make the code easier to read and maintain as well.

Bob Jarvis - Слава Україні · Answer 3 · 2012-06-07T12:23:53.663

Inside your loop you're ignoring characters - but then you're incrementing the indexes even though you haven't filled the related array element. Try re-implementing as:

#include <stdlib.h>

for(x=0;x<9;x++)
    for(y=0;y<9;y++)
      {
      /* Get next character */

      current=fgetc(fp);

      /* Loop, retrieving additional chars, until either EOF is hit or 
         you find a non-whitespace character */

      while(current != EOF && iswhite(current))
        current = fgetc(fp);

      if(current != EOF)
        sudokuArray[x][y] = current - '0';
      }

Also - do you want to check current != EOF, or should this be !feof(fp)?

Share and enjoy.

fgetc() doesn't work properly

3 Answers3