Reading text file into a matrix in C

Question

I have a text file that has 200 x 150 data in it (every number is separated with a space). I am trying to store these numbers into a matrix. My code is this:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int i,j, matrix[200][150];

void getMatrix(){
    FILE *file;
    file = fopen("PICTURE.txt", "r");
    char *line, *number;
    j = 0;
    while(!feof(file)){
        i = 0;
        fscanf(file, "%s", line);
        number = strtok(NULL, " ");
        while(number!= NULL){
            matrix[i][j] = atoi(number);
        }
        printf("\n");
        j++;
    }

}

void printMatrix(){
    int a, b;
    for(a=0; a<i; a++){
        for(b=0; b<j; b++){
            printf("%d", matrix[a][b]);
        }
        printf("\n");
    }
}

int main(){
    getMatrix();
    printMatrix();
}

It prints out literally nothing. I don't understand why. How can I fix my code or do you have any other suggestions for rewriting it?

Answer 1

if you have a file, with 200x150 integer values (either on 200 lines, or all values on one line), then simply looping though the file with fscanf() reading and storing an integer at a time is all that is needed. You must validate each read and provide a way to communicate any failure within a function through the function return (or though a pointer updated within the function)

To begin, do not use MagicNumbers in your code and do not hardcode filenames. Instead, #define the constants you need (or use a global enum) and pass the open FILE* pointer to your read-function as a parameter after having opened (and validated that the file is open for reading) in the calling function. (If the file open fails -- there is no reason to make the read-function call to begin with)

In your case, you only have two needed constants:

#define ROWS 200    /* if you need a constant, #define one (or more) */
#define COLS 150

Do not use global variables unless absolutely necessary (there will be virtually no case where globals are needed when you begin learning C). Passing the matrix (2D array) as a parameter to your read-function, it can be written as:

/* fill m[ROWS][COLS] with values read from fp.
 * returns 1 on success, 0 otherwise.
 */
int getmatrix (int (*m)[COLS], FILE *fp)
{
  for (int row = 0; row < ROWS; row++) {
    for (int col = 0; col < COLS; col++) {
      if (fscanf (fp, "%d", &m[row][col]) != 1) {
        fprintf (stderr, "error reading m[%d][%d]\n", row, col);
        return 0;
      }
    }
  }
  
  return 1;
}

(note: how the return type for the function is int where 1 (true) is returned on success and 0 (false) is returned on failure. Also note that if 200x150 values are unable to be read, failure is returned)

Pass the filename to read from as the 1st argument to your program (that's what int argc, char **argv parameters to main() are for). Alternatively, you can prompt the user and read the filename as input. You can provide a fixed filename as a default value if no filename is entered (stdin is used as the default below), but otherwise do not hardcode filenames. You should not have to recompile your program simply to read from a different filename.

With those improvements made, your main() could be:

int main (int argc, char **argv) {
  
  int matrix[ROWS][COLS] = {{0}};   /* do not use global variables */
  /* use filename provided as 1st argument (stdin by default) */
  FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

  if (!fp) {  /* validate file open for reading in caller */
    perror ("file open failed");
    return 1;
  }
  
  if (!getmatrix (matrix, fp)) {    /* pass array and open FILE*, validate */
    return 1;
  }
  fclose (fp);
  
  prnmatrix (matrix);       /* output results */
}

The complete program is:

#include <stdio.h>

#define ROWS 200    /* if you need a constant, #define one (or more) */
#define COLS 150

/* fill m[ROWS][COLS] with values read from fp.
 * returns 1 on success, 0 otherwise.
 */
int getmatrix (int (*m)[COLS], FILE *fp)
{
  for (int row = 0; row < ROWS; row++) {
    for (int col = 0; col < COLS; col++) {
      if (fscanf (fp, "%d", &m[row][col]) != 1) {
        fprintf (stderr, "error reading m[%d][%d]\n", row, col);
        return 0;
      }
    }
  }
  
  return 1;
}

void prnmatrix (int (*m)[COLS])
{
  for (int row = 0; row < ROWS; row++) {
    for (int col = 0; col < COLS; col++) {
      printf (col ? " %4d" : "%4d", m[row][col]);
    }
    putchar ('\n');
  }
}

int main (int argc, char **argv) {
  
  int matrix[ROWS][COLS] = {{0}};   /* do not use global variables */
  /* use filename provided as 1st argument (stdin by default) */
  FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

  if (!fp) {  /* validate file open for reading in caller */
    perror ("file open failed");
    return 1;
  }
  
  if (!getmatrix (matrix, fp)) {    /* pass array and open FILE*, validate */
    return 1;
  }
  fclose (fp);
  
  prnmatrix (matrix);       /* output results */
}

Example Use/Output

With 200 x 150 random values less than 10000 generated and written to dat/int-200x150.txt, you would run the program as:

$ ./bin/read2darrint dat/int-200x150.txt
9240 5939 3063 5789 7401 7869 4363 3321 7788 1008 7850 ...
2760 5263 5338 6390 8146  155  324  916 4489 3718 1616 ...
...

Where the output is 200 lines of 150 numbers each.

There are many ways to do this. One (better) way would be to loop reading each line of integer values as a string with fgets() and converting each set of ASCII digits into an integer value with strtol(). I say "better" in a sense that strtol() provides far better error reporting in the case of a failed conversion that the simple pass/fail that can be obtained from fscanf().

The read before looping with strtol() would be with fgets() into a sufficiently sized buffer. That way if a matching-failure occurs, the error only impacts that line of data and no offending data is left in your input stream unread (as will occur with a matching-failure and fscanf().

That said, with a properly formatted space-separated file full of data, there is nothing wrong with using fscanf() directly, just be aware there are more robust ways to do the read and conversion.

To use strtol(), to work through each line of values, you simply use a pointer to the beginning of each set of digits calling strtol() which then updates the endptr to one past the last digit converted allowing you to work your way though each number by updating the pointer (nptr) to endptr after each validated conversion. See man 3 strtol. You could updated the code above with:

...
#include <stdlib.h>
#include <limits.h>
#include <errno.h>
...
#define MAXC 2048   /* (150 int + space) max of 1800 chars per-line */

and finally change your function to use strtol(), e.g.

/* fill m[ROWS][COLS] with values read from fp.
 * returns 1 on success, 0 otherwise.
 */
int getmatrix (int (*m)[COLS], FILE *fp)
{
  char buf[MAXC];   /* buffer to hold each line */
  
  for (int row = 0; row < ROWS; row++) {
    char *nptr = buf, *endptr = nptr;   /* nptr and endptr for strtol */
    
    if (!fgets (buf, MAXC, fp)) {   /* read line of input */
      fprintf (stderr, "error: failed to read line %d\n", row);
      return 0;
    }
    
    for (int col = 0; col < COLS; col++) {
      errno = 0;                                  /* reset errno */
      long tmp = strtol (nptr, &endptr, 10);      /* convert w/strtol */
      
      if (nptr == endptr) { /* no ASCII digits coverted to a number */
        fputs ("error: no digits converted.\n", stderr);
        return 0;
      }
      else if (errno) { /* error in converstion, under/overflow */
        fputs ("error: under/overflow occurred in conversion.\n", stderr);
        return 0;
      }
      /* conversion outside range of int */
      else if (tmp < INT_MIN || INT_MAX < tmp) {
        fputs ("error: conversion out of range of integer.\n", stderr);
        return 0;
      }
      
      m[row][col] = tmp;   /* assign tmp to m[row][col] */
      nptr = endptr;       /* update nptr to endptr */
    }
  }
  
  return 1;
}

If an error occurs, you will know exactly what caused the failure. Look things over and let me know if you have further questions.

Answer 2

Many problems:

• Never use while(!feof(file)){. Instead check the return value of fscanf(file, ...)

• Do not use "%s" with out a width limit.

• Do not use pass line to a function without initializing/assigning it first.

• i never incremented.

• fclose(file) when done.

• Check success of fopen(...);

---

GTG, maybe more later.