0

I'm trying to display my output in a 4X4 format. The program receives name of a text file then get the values in the text file and store in the 2D array. Then later displays them like this

7 2 4 5 
5 2 6 4
2 2 5 5 
9 2 4 5

But the problem is, it doesn't display like that, I'm not sure if it's my loop or what. Any ideas. it runs fine with no errors but the numbers don't display right. Here's my code

int main () {

  int i = 0;
  int j = 0;

   int value = 0;
   int a[4][4];
   char ch, file_name[100];
   FILE *fp;

   printf("Enter file name?");
   gets(file_name);

   fp = fopen("file.txt", "r");    

   if (fp == NULL)
   {
      perror("Error \n");
   }
   else 
   {

       // printf("\n");
        while (!feof(fp) && fscanf (fp, "%d ", &value))
        {
            a[i][j] = value;

         for ( i = 0; i < 4; i++ ) 
                {
              for ( j = 0; j < 4; j++ ) 
              {
                 printf("%d ",a[i][j] );
              }   
             printf("\n");      

           }     
       }
   }

    fclose(fp);

   return 0;

}
11
  • 1
    Please see Why is while ( !feof (file) ) always wrong? You should control the loop with the return value from fscanf. Such as while (fscanf (fp, "%d ", &value) == 1) { } Commented Mar 16, 2018 at 22:17
  • 1
    Ack! gets is never to be used! Commented Mar 16, 2018 at 22:17
  • stackoverflow.com/questions/1694036/… Commented Mar 16, 2018 at 22:18
  • @WeatherVane I changed that it still didn't change anything. Commented Mar 16, 2018 at 22:23
  • 2
    "the numbers don't display right" is not an acceptable problem statement. Commented Mar 16, 2018 at 22:43

2 Answers 2

1

Your code suffers from various problems since you are mixing the code that reads the input and code that writes the output in one while loop.

Separate the code that reads the input and the code that creates the output. Ideally, put them into different functions.

Declare the functions as:

void readData(FILE* in, int a[4][4]);
void writeData(FILE* out, int a[4][4]);

Use them as:

int main()
{
   int a[4][4];

   // Your code to open the file.

   readData(in, a);

   writeData(stdout, a);
}

Implement them as:

void readData(FILE* in, int a[4][4])
{
   for ( i = 0; i < 4; i++ ) 
   {
      for ( j = 0; j < 4; j++ ) 
      {
         if ( fscanf(in, "%d", &a[i][j]) != 1 )
         {
            printf("Unable to read an array element.\n");
            exit(0);
         }   
      }   
   }
}

void writeData(FILE* out, int a[4][4])
{
   for ( i = 0; i < 4; i++ ) 
   {
      for ( j = 0; j < 4; j++ ) 
      {
         fprintf(out, "%d ", a[i][j]);
      }   
      fprintf(out, "\n");      
   }
}
Sign up to request clarification or add additional context in comments.

2 Comments

How is this an answer to the question?
@aschepler, i didn't want enumerate all the problems in the OP's code.
0

Well, let's take your problems apart a piece at a time. First, the salient part of your code regarding input and attempted output:

    int i = 0;
    int j = 0;
    int value = 0;
    int a[4][4];
    ...
    while (!feof(fp) && fscanf (fp, "%d ", &value))
    {
        a[i][j] = value;

     for ( i = 0; i < 4; i++ ) 
            {
          for ( j = 0; j < 4; j++ ) 
          {
             printf("%d ",a[i][j] );
          }   
         printf("\n");      

       }     
   }

Before we look at the !feof problem. let's look at the overall logic of your loop structure. When you enter your while loop, the values of i = j = 0;. Presuming your file is open and there is an integer to read, you will fill value with the first integer in the file and then assign that value to the first element of your array with:

a[i][j] = value;

Now, you have stored 1-integer at a[0][0]. (all other values in a are uninitialized and indeterminate) Inexplicably, you then attempt to output the entire array, ... uninitialized values and all.

     for ( i = 0; i < 4; i++ ) 
            {
          for ( j = 0; j < 4; j++ ) 
          {
             printf("%d ",a[i][j] );
          }   
         printf("\n");      

       }     

Attempting to access an uninitialized value invokes Undefined Behavior (your program is completely unreliable from the first attempt to read a[0][1] on). It could appear to run normally or it could SegFault or anything in between.

You need to complete the read of all values into your array before you begin iterating over the indexes in your array outputting the values.

But wait... there is more... If you did not SegFault, when you complete the for loops, what are the values of i & j now? Your loops don't complete until both i and j are 4, so those are the values they have at the end of the first iteration of your while loop.

Now let's go to the next iteration of your while loop. Presuming there are two integer values in the file you are reading, you read the second integer into value. What happens next? You one-up yourself. After already invoking undefined behavior 15 times by attempting to read 15 uninitialized values a[0][1] -> a[3][3], you then attempt to write beyond the bounds of a with:

a[4][4] = value;         /* remember what i & j are now? */

You then hit your for loops..., again with a[0][1] holding the only validly initialized value and the cycle repeats.

But wait..., there is more... After having read the last integer in your file, you arrive at the beginning of your while loop one last time:

    while (!feof(fp) && fscanf (fp, "%d ", &value))

You test !feof(fp) and no EOF has been set because your last read was a valid read of the last integer which completed with the last digit and did not trigger a stream error, so you proceed to fscanf (fp, "%d ", &value) which now returns EOF (e.g. -1), but since you simply test whether fscanf(..), -1 tests TRUE so off you go again through the entire body of your loop invoking undefined behavior at least 16 more times.

Are you beginning to understand why the output may not have been what you wanted?

You have already had a good answer on how to go about the read and print. I'll offer just an alternative that does not use any functions so it may help you follow the logic a bit better. The following code just reads from the filename given as the first argument, or from stdin by default if no argument is given. The code validates that the file pointer points to a file that is open for reading and then enters a while loop reading an integer at a time into &array[rows][cols] with fscanf and validating the return of fscanf is 1. Anything else is considered failure.

The remainder of the read loop just increments cols until is reaches NCOLS (4 here - being the number of columns per-row), then it increments the row-count in rows and sets the column-index cols back to zero.

A final validation is performed before output to insure NROWS of integers were read and that cols was set to zero during the final iteration of the read-loop. The contents of array is then output. The code is fairly straight-forward:

#include <stdio.h>

#define NROWS 4
#define NCOLS NROWS

int main (int argc, char **argv) {

    int array[NROWS][NCOLS] = {{0}},    /* declare and intialize array */
        rows = 0, cols = 0;             /* rows / columns index */
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!fp) {  /* validate file open for reading */
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

    /* validate no more than NROWS of NCOLS integers are read */
    while (rows < NROWS && fscanf (fp, "%d", &array[rows][cols]) == 1) {
        cols++;                 /* increment column */
        if (cols == NCOLS) {    /* if row full */
            rows++;             /* increment rows */
            cols = 0;           /* zero columns */
        }
    }
    if (fp != stdin) fclose (fp);   /* close file if not stdin */

    if (rows != NROWS || cols) {    /* validate rows = NROWS & cols = 0 */
        fprintf (stderr, "error: read only %d rows and %d cols.\n", 
                 rows, cols);
        return 1;
    }

    for (int i = 0; i < NROWS; i++) {   /* output stored values */
        for (int j = 0; j < NCOLS; j++)
            printf (" %2d", array[i][j]);
        putchar ('\n');
    }

    return 0;
}

(you have already been advised not to use gets which is subject to easy buffer overflow and so insecure it has been completely removed from the C11 library. If your professor continues to suggest its use -- go find a new professor)

Example Input File

$ cat dat/arr2dfscanf.txt
7 2 4 5
5 2 6 4
2 2 5 5
9 2 4 5

Example Use/Output

$ ./bin/arr2dfscanf <dat/arr2dfscanf.txt
  7  2  4  5
  5  2  6  4
  2  2  5  5
  9  2  4  5

Also note, that by reading with fscanf in this manner, it doesn't matter what format your input is in -- as long as it is all integer values. This will produce the same output:

$ echo "7 2 4 5 5 2 6 4 2 2 5 5 9 2 4 5" | ./bin/arr2dfscanf

Look things over and let me know if you have further questions.

9 Comments

Thanks @David C Rankin. I understand the Logic and I like how you explained it. When I run the code, it just compiles but doesn't output anything.
Did you give it an input file? Or send it integers on stdin like it did above with the "echo" statement?
Yes. So something like this? FILE *fp = argc > 1 ? fopen (argv[('a.txt')], "r") : stdin;
NO. Just put a.txt on the command line as the first argument. e.g. ./program a.txt Or redirect a.txt to the program on stdin, e.g. ./program <a.txt, or cat a.txt | ./program, etc... (the last technically being a UUOc (Unnecessary Use Of cat), but for illustration purposes.
The line FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin; just says assign the file pointer to: (1) if argc > 1 (meaning there is an argument), then fopen (argv[1], "r") and read from the filename given as argv[1] -or- (2) if no argument is given, read from stdin. It is just the use of the ternary operator to read using argv[1] if given, else use stdin for input (it's a shortcut, sorry it was confusing) Let me know if you have any more problems. I'm happy to help until you get it sorted out in your mind.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.