My delete function deletes the wrong record of struct

Question

I have this function which is supposed to delete the recipe requested by the user. I've always deleted records in this way, but I don't know why this time it doesn't work in the right way. Sometimes it states that there's no match but it deletes the recipe or it deletes all the recipes inside the file and I don't get it why since I basically created a temp file in which writing all the records except for the one the user wants to delete and I used a function that worked for another record so I really don't get it. Is that because the length of ingredients and procedure is not the same for all the recipes?

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

int stringCmpi (char *s1,char *s2);

struct _recipe
{
    char name[40];
    int count_i;
    char ingredients[20][40];
    char diff[12];
    int timr;
    int calories;
    int count_p;
    char procedure[30][500];
} recipe;

int main()
{
    FILE* fbr;
    FILE* temp;
    char ricetta_name[] = "ricette.bin";
    char temp_name[] = "temp.bin";
    fbr = fopen("ricette.bin", "rb");
    temp = fopen("temp.bin", "wb");
    int found = 0;
    char name_t[40];
    int i;
    char space = '\n';

    if(fbr == NULL)
    {
       printf("Couldn't open the file.\n");
       exit(1);
    }
    if(fbr == NULL)
    {
       printf("Couldn't open the file.\n");
       exit(1);
    }


    printf("Write the name of the recipe you want to delete:\n");
    fgets(name_t,sizeof(name_t),stdin);
    space = getchar();
    while(fread(&recipe,sizeof(recipe),1,fbr) && found == 0)
    {   
        if(stringCmpi(name_t,recipe.name) == 0)
        {
            found = 1;
            printf("Match found. Recipe deleted.\n");
        }

        else
        {

            fwrite(&recipe,sizeof(recipe),1,temp);

        }
            if(!found)
    {
        printf("No match.\n");
    }
    }



    fclose(fbr);
    fclose(temp);
    remove(ricetta_name);
    rename(temp_name,ricetta_name);
    system("PAUSE");
    return 0;
}

int stringCmpi (char *s1,char *s2)
{
    int i=0;
    for(i=0; s1[i]!='\0'; i++)
    {
        if( toupper(s1[i])!=toupper(s2[i]) )
            return 1;           
    }
    return 0;
}

Answer 1

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

struct _recipe
{
    char name[40];
    int count_i;
    char ingredients[20][40];
    char diff[12];
    int timr;
    int calories;
    int count_p;
    char procedure[30][500];
} recipe;

int main(int argc, char *argv[])
{
    FILE* fbr;
    FILE* temp;
    char *ricetta_name = "recipe.bin";
    char *temp_name = "temp.bin";
    int found = 0;
    char name_t[40];

    // Use command line arguments if possible
    if (argc == 2)
    {
        ricetta_name = argv[1];
    }

    if((fbr = fopen(ricetta_name, "rb")) == NULL)
    {
       printf("Couldn't open the file %s.\n", ricetta_name);
       exit(1);
    }
    if((temp = fopen(temp_name, "wb")) == NULL)
    {
       printf("Couldn't open the file %s.\n", temp_name);
       exit(1);
    }

    printf("Write the name of the recipe you want to delete:\n");
    fgets(name_t, sizeof(name_t), stdin);

    // Remove '\n'
    if (name_t[strlen(name_t) - 1] == '\n')
        name_t[strlen(name_t) - 1] = 0;

    while(fread(&recipe,sizeof(recipe), 1, fbr))
    {  
        // Use standard strcasecmp()
        if(strcasecmp(name_t, recipe.name) == 0)
        {
            found = 1;
            printf("Match found. Recipe deleted.\n");
        }
        else
        {
            fwrite(&recipe, sizeof(recipe), 1, temp);
        }
    }

    if (!found)
    {
        printf("No match.\n");
    }

    fclose(fbr);
    fclose(temp);
    remove(ricetta_name);
    rename(temp_name,ricetta_name);
    return 0;
}

Please format your code: add space(' ') after comma(','). And use standard function if possible.

Answer 2

1. Proper indentation would help you see the actual structure of the program. This is how the posted program looks with consistent indentation:

   #include <stdio.h>
   #include <stdlib.h>
   #include <string.h>
   
   int stringCmpi (char *s1,char *s2);
   
   struct _recipe
   {
       char name[40];
       int count_i;
       char ingredients[20][40];
       char diff[12];
       int timr;
       int calories;
       int count_p;
       char procedure[30][500];
   } recipe;
   
   int main()
   {
       FILE* fbr;
       FILE* temp;
       char ricetta_name[] = "ricette.bin";
       char temp_name[] = "temp.bin";
       fbr = fopen("ricette.bin", "rb");
       temp = fopen("temp.bin", "wb");
       int found = 0;
       char name_t[40];
       int i;
       char space = '\n';
   
       if(fbr == NULL)
       {
           printf("Couldn't open the file.\n");
           exit(1);
       }
       if(fbr == NULL)
       {
           printf("Couldn't open the file.\n");
           exit(1);
       }
   
       printf("Write the name of the recipe you want to delete:\n");
       fgets(name_t,sizeof(name_t),stdin);
       space = getchar();
       while(fread(&recipe,sizeof(recipe),1,fbr) && found == 0)
       {
           if(stringCmpi(name_t,recipe.name) == 0)
           {
               found = 1;
               printf("Match found. Recipe deleted.\n");
           }
   
           else
           {
               fwrite(&recipe,sizeof(recipe),1,temp);
           }
           if(!found)
           {
               printf("No match.\n");
           }
       }
   
       fclose(fbr);
       fclose(temp);
       remove(ricetta_name);
       rename(temp_name,ricetta_name);
       system("PAUSE");
       return 0;
   }
   
   int stringCmpi (char *s1,char *s2)
   {
       int i=0;
       for(i=0; s1[i]!='\0'; i++)
       {
           if( toupper(s1[i])!=toupper(s2[i]) )
               return 1;
       }
       return 0;
   }
   

2. The program is checking that fbr is not NULL twice. Probably the second check should be for temp.

3. The programs stops reading and copying when the recipe to be deleted is found; this means that the following records are not copied over.

4. The program complains that the recipe was not found for each recipe until the desired one is found.

Suggestions:

• Modify the while loop condition eliminating the && ! found, so that all records are copied except those matching the given name.

• Move the check for if (! found) outside the loop.

Bonus:

• The function stringCmpi() stops comparing at the end of string s1; which means that "abc" and "abcdef" will compare equal. You probably want to change return 0 to return s1 [i] != s2 [i].

Answer 3

The problem is here:

while(fread(&recipe,sizeof(recipe),1,fbr) && found == 0)

Once you find a recipe, you exit the loop, so the remaining elements are not copied to the new file. I recommend you delete the && found == 0 clause.

Furthermore:

if(!found)
{
    printf("No match.\n");
}

This is actually inside the while loop, so it will report No match multiple times. Please move it outside the loop.