0

Ok, I know the code might be long but I minimized it as much as I could. I wanted the code to work so you could recreate my problem. My problem is that when I try to read a text file, it doesn't read all of the items. It seems to read only the last few. I might've accidentally changed something without noticing because it worked just fine before. When you're in the program and registering items, it counts the items just fine. But when you open the file you just saved your items in. The number of items it counts is less than what's in the actual file and the arrays get all wrong. If anyone can see the problem in my code, that would be very appreciated.

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

#define MAX 20

struct items
{
    int itemnumber;
    char name[30];
    int balance;
};

void open_file(FILE *enter_filename, char filename[], struct items aItems[], int *num_items)
{
    int i=0;

        printf("Choose filename (.txt).\n");
        scanf("%19s", filename);
        enter_filename=fopen(filename, "r+");
        if(enter_filename)
        {
            while(!feof(enter_filename))
            {
                 for(i = 0; i < *num_items; i++) 
                    {
                    fscanf(enter_filename, "Itemnumber: %d\n", &aItems[i].itemnumber);
                    fscanf(enter_filename, "Name: %s\n", aItems[i].name);
                    fscanf(enter_filename, "Balance: %d\n", &aItems[i].balance);
                    }
                if(!feof(enter_filename))
                {
                    *num_items=*num_items + 1;     
                }
            }
            printf("\nNumber of items: %d \n",*num_items);
            fclose(enter_filename);
        }
        else
        {
            printf("That file doesn't exist! Create a new one.\n");
            printf("What name do you want for your new file?\n");
            scanf("%19s", filename);
            enter_filename=fopen(filename, "w+");
            printf("File is created!\n");
            *num_items = 0;                 
            fclose(enter_filename);
        }
}
void register_item(struct items *aItems, int *num_items)
{
    int success=1;
    if(*num_items < MAX)
        {   
        while(1)
        {   
            printf("Item number:\n");                             
            scanf("%d", &aItems[*num_items].itemnumber);
            for(int i=0; i < *num_items; i++)
            {
            if(aItems[*num_items].itemnumber == aItems[i].itemnumber)
                {
                printf("Item number already exists, choose a unique item number.\n");
                success=0;
                break;
                }
            else
                {
                success=1;
                }
            }
        if(success)break;
        }
        printf("Name:\n");
        scanf("%29s", aItems[*num_items].name);
        strlwr(aItems[*num_items].name);
        printf("Balance:\n");
        scanf("%d", &aItems[*num_items].balance);   
        *num_items+=1;
        }
}
void print_item(struct items aItems[], int num_items)
{
    int i;
    for (i=0; i < num_items; i++)
    {
    printf("%d. Item number: %d Name: %s Balance: %d\n", i+1, aItems[i].itemnumber, aItems[i].name, aItems[i].balance);         
    }
}
void quit_program(char filename[], struct items aItems[], int *num_items)
{
    FILE *fil;
    fil=fopen(filename, "w+");                                             
    int i;
    for(i = 0; i < *num_items; i++) 
        {
        fprintf(fil, "Itemnumber: %d\n", aItems[i].itemnumber);
        fprintf(fil, "Name: %s\n", aItems[i].name);
        fprintf(fil, "Balance: %d\n\n", aItems[i].balance);
        }
    fclose(fil);
}   
int main(void)
{
    FILE *enter_filename;
    struct items aItems[MAX];

    int menu, num_items=0;
    char filename[20]; 

    open_file(enter_filename,filename, aItems, &num_items);

    while(menu!=3)
    {
        printf("\n");
        printf("1. Register new items to inventory.\n");
        printf("2. Print all items from inventory.\n");
        printf("3. Quit\n");
        scanf("%d", &menu);

        if(menu==1)
        {
           register_item(aItems, &num_items);
        }

        if(menu==2)
        {
          print_item(aItems, num_items); 
        }

        if(menu==3)
        {
         quit_program(filename, aItems, &num_items);  
        }
    }
return 0;
}
Camel
  • 37
  • 7
  • 2
    I don't see how this could be a [mcve] for problems with reading a file. Maybe create one from scratch? In general, I recommend reading [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). –  Oct 10 '17 at 12:44
  • 2
    See [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong), too. That's a really bad pattern. Also, please consider showing part of the file it's supposed to be reading. – unwind Oct 10 '17 at 12:52
  • If I remove more code from this or try to make it from scratch, it's going to be a different program. I only added the stuff necessary to run the program. – Camel Oct 10 '17 at 12:52
  • 2
    Yes, but most of the code is about interactively prompting the user for stuff, which doesn't seem pertimnent to the problem. Plus it's annoying for anyone who has to test it to figure out what kind of input is wanted. Hard-code some items, write them out, read them in again and then print them. – M Oehm Oct 10 '17 at 12:55
  • `FILE *enter_filename; ... open_file(enter_filename, ...);`You pass an unitialized `FILE*` to your function. Lateron you pass it again to `quit_program(filename...`. This will not work. The naming `filename` for a `FILE*` is highly suspicious, – Gerhardh Oct 10 '17 at 12:56
  • @Gerhardh I'm not sure what you mean. Can you try to explain it again ? – Camel Oct 10 '17 at 13:06
  • Sorry, I misread the parameter name. The part about `quit_program` was nonsense. Nevertheless the `FILE*`parameter for `open_file` is useless as you cannot return anything with it and don't pass anything into the function. And calling it `enter_filename` while it is not related to any filename is also a bit weird. – Gerhardh Oct 10 '17 at 13:10

2 Answers2

1

Your error is here:

        while(!feof(enter_filename))
        {
             for(i = 0; i < *num_items; i++) 
                {
                fscanf(enter_filename, "Itemnumber: %d\n", &aItems[i].itemnumber);
                fscanf(enter_filename, "Name: %s\n", aItems[i].name);
                fscanf(enter_filename, "Balance: %d\n", &aItems[i].balance);
                }
            if(!feof(enter_filename))
            {
                *num_items=*num_items + 1;     
            }
        }

in the first pass through your loop, *num_items is zero, so the iner loop that does the actual reading will not be entered and aItems[i] will be uninitialised. You then increase the item counter.

In the next pass, exactly one item, namely aItems[1] will be read, but it will receive the data of the first item.You increase the counter. In the third pass, you read aItems[2], but immediately overwrite it, because your inner loop makes as many passes as there are currently elements in the array. Clearly, that's wrong.

When you read the file, you don't know how many items there are. Therefore, you must read an item, test whether it could be read and then increase the counter accordingly. Testing whether the file has ended or whether the input was correct is done via the return value of fscanf,which returns the number ofitems successfully converted.

Your loop could work like this:

        while (*num_items < MAX) {
            struct items *p = &aItems[*num_items];

            if (fscanf(f, "Itemnumber: %d\n", &p->itemnumber) < 1
             || fscanf(f, "Name: %s\n", p->name) < 1
             || fscanf(f, "Balance: %d\n", &p->balance) < 1) {
                break;
            }

            (*num_items)++;
        }

(I've called the file handle f: enter_filename is both too long and too misleading.)

Other things to note:

  • The file handle and filenames are local to your functions; they are not used outside and the handle is opened and closed properly. Therefore,you should not pass them as arguments, but make them local variables.
  • The variable menu is uninitialised and may well be 3 when you start the program.
M Oehm
  • 28,726
  • 3
  • 31
  • 42
  • You explained my problem perfectly. Do you mind explaining that code as well ? I don't really understand the conditions in the if statement. <1 – Camel Oct 10 '17 at 13:43
  • `fscanf` returns the number of values that were converted, more or less how many of the `%` sequences in the format string were understood. The special value `EOF` indicates that the end of the file was encountered. `EOF` is negative,usually −1. If any of the data lines can't be read, you break out of the loop. – M Oehm Oct 10 '17 at 13:53
  • (Controlling file input via the return values of the reading functions is the preferred way to read files; see the link in Serkan's answer. The functions `feof()` and `ferror()` can be used after ´EOF` was found whether the cause for that was a reading error or the actual end of the file.) – M Oehm Oct 10 '17 at 13:55
  • Interesting. So that while loop solved both my problem and my bad usage of the while loop. Thanks a lot for the help. I learned a lot. – Camel Oct 10 '17 at 14:02
0

In your open_file function you use while(!feof(enter_filename)).

This is not a reliable way of reading a file, as it is stated in this question. For your case, what you end up is that since you set num_items in a faulty while(!feof(enter_filename)) loop, your num_items holds a wrong value, and it propagates through your program logic because you are using it almost everywhere, especially when writing back to the file again, which explains the missing lines. Once you implement one of the methods mentioned in here or here, use your debugger to ensure that num_items are consistent with your input file.

Serkan Pekçetin
  • 658
  • 7
  • 14