random access and sequential binary files and functions issues

overlord21 · 09-29-2008, 01:36 PM

here i have the function that is supposed to delete a selected account and display the change in the file, but this instead removes all the account and leave only one. Can anyone help

Code:

 void account_delete(char *filename)
{
     
     
     
     int count=0,i=0,g,h;
     char NAME[20];
     account t[20];
     FILE *file=fopen(filename,"rb");
     if(file!=NULL)
     {
                     
                                    
                          while(fread(&t[i],sizeof (account),1,file)==1)   
                            {   
                                i++;                           
                                count++; 
                            }                
     
                     
     
     }
     printf("give the name of account to delete");
     scanf("%s",&NAME);
     for( g=0;g<count;g++)
       {if(strcmp(t[g].name,NAME)==0)
           {  for(h=g;h<count;h++)
                {
                     t[h]=t[h+1];
                    }
            }    
        }  
        count--;  
     
        for(i=0;i<count;i++)
          { 
               account_overwrite(filename,t[i]);//this  open the file in "wb" and write to content of t[i] in the file. This one still works with other functions.
                }             
         
          fclose(file);                                          
          view_account(filename);
     
     
     }

**laserlight** · 09-29-2008, 02:03 PM

Firstly, you really do need to improve your indentation, e.g.,

Code:

void account_delete(char *filename)
{
    int count = 0, i = 0, g, h;
    char NAME[20];
    account t[20];

    FILE *file = fopen(filename, "rb");
    if (file != NULL)
    {
        while (fread(&t[i], sizeof(account), 1, file) == 1)
        {
            i++;
            count++;
        }
    }

    printf("give the name of account to delete");
    scanf("%s", &NAME);
    for (g = 0; g < count; g++)
    {
        if (strcmp(t[g].name, NAME) == 0)
        {
            for (h = g; h < count; h++)
            {
                t[h] = t[h+1];
            }
        }
    }
    count--;

    for (i = 0; i < count; i++)
    {
        //this open the file in "wb" and write to content of t[i] in the file. This one still works with other functions.
        account_overwrite(filename, t[i]);
    }

    fclose(file);
    view_account(filename);
}

The comment for account_overwrite() is strange. If account_overwrite() opens the file with the "wb" mode, then each time it is called, it truncates the file to zero length, and then writes the account provided to the file. This could explain why you only have one account left.

Rather, I suggest that you have a store_accounts() function (or something like that) with a signature like this:

Code:

void store_accounts(const char *filename, const account accounts[], size_t count);

Then you can open the file with "wb" mode and write the entire accounts array to the file.

Likewise, instead of having to write the code to retrieve the accounts from file into an array in each function, you can have a retrieve_accounts() function with a signature like this:

Code:

void retrieve_accounts(const char *filename, account accounts[], size_t count);

When implementing this function, be careful to avoid buffer overflow by ensuring that you only read up to count number of accounts. Your current code does not do this. When you are ready to handle dynamic memory allocation and so remove this restriction on array length, you can then change retrieve_accounts() to:

Code:

void retrieve_accounts(const char *filename, account *accounts, size_t *count);

In which case count will contain the number of accounts retrieved, and accounts will be dynamically allocated and so will need to be freed.

overlord21 · 09-29-2008, 02:43 PM

Code:

void view_account(char *filename)
{
     int count=0;
     int i=0;
     account t[30];
     FILE *file=fopen(filename,"rb");
     if(file!=NULL)
     {
                     
                                    
                          while(fread(&t[i],sizeof (account),1,file)==1)   
                            {   
                                i++;                           
                                count++; 
                            }                
     
                     
     
     }
     printf(  "Acc.Name    |     Password|       Privilege");
	 for(i=0;i<count;i++)
     {
         printf("\n   %s   |   %s          |   %s\n ",t[i].name,t[i].password,t[i].previleges);    
             
             }
     
     
     
     
     
     
     }

here's the view account code is anything wrong with it?

**laserlight** · 09-30-2008, 12:10 AM

Quote:

here's the view account code is anything wrong with it?

One problem is that you forgot to close the file after reading from it. Again, notice that this code is used:

Code:

if (file != NULL)
{
    while (fread(&t[i], sizeof(account), 1, file) == 1)
    {
        i++;
        count++;
    }
}

Clearly, you use the above code in the account_delete() function as well. If you had a retrieve_accounts() function as I proposed, then you would just use that function (and the closing of the file would be done correctly in that function). Your view_account() function would focus on printing the accounts, not on reading them. This follows the principle that functions should do one thing and do it well.

Incidentally, fread() can read multiple accounts in one call, so I would expect code like this:

Code:

size_t retrieve_accounts(const char *filename, account *accounts, size_t capacity)
{
    size_t size = 0;
    FILE *file = fopen(filename, "rb");
    if (file != NULL)
    {
        size = fread(accounts, sizeof(account), capacity, file);
        fclose(file);
    }
    return size;
}

Then to use it, you might write:

Code:

account accounts[30];
size_t count = retrieve_accounts(filename, accounts, 30);

The magic number 30 itself could be replaced by a global named constant, e.g., ACCOUNT_MAX, upon which you would not need the capacity parameter.

09-29-2008, 02:03 PM	#2
laserlight C++ Witch Join Date: Oct 2003 Location: Singapore Posts: 10,352	Firstly, you really do need to improve your indentation, e.g., Code: void account_delete(char filename) { int count = 0, i = 0, g, h; char NAME[20]; account t[20]; FILE file = fopen(filename, "rb"); if (file != NULL) { while (fread(&t[i], sizeof(account), 1, file) == 1) { i++; count++; } } printf("give the name of account to delete"); scanf("%s", &NAME); for (g = 0; g < count; g++) { if (strcmp(t[g].name, NAME) == 0) { for (h = g; h < count; h++) { t[h] = t[h+1]; } } } count--; for (i = 0; i < count; i++) { //this open the file in "wb" and write to content of t[i] in the file. This one still works with other functions. account_overwrite(filename, t[i]); } fclose(file); view_account(filename); } The comment for account_overwrite() is strange. If account_overwrite() opens the file with the "wb" mode, then each time it is called, it truncates the file to zero length, and then writes the account provided to the file. This could explain why you only have one account left. Rather, I suggest that you have a store_accounts() function (or something like that) with a signature like this: Code: void store_accounts(const char filename, const account accounts[], size_t count); Then you can open the file with "wb" mode and write the entire accounts array to the file. Likewise, instead of having to write the code to retrieve the accounts from file into an array in each function, you can have a retrieve_accounts() function with a signature like this: Code: void retrieve_accounts(const char filename, account accounts[], size_t count); When implementing this function, be careful to avoid buffer overflow by ensuring that you only read up to count number of accounts. Your current code does not do this. When you are ready to handle dynamic memory allocation and so remove this restriction on array length, you can then change retrieve_accounts() to: Code: void retrieve_accounts(const char filename, account accounts, size_t *count); In which case count will contain the number of accounts retrieved, and accounts will be dynamically allocated and so will need to be freed. __________________ C + C++ Compiler: MinGW port of GCC Build + Version Control System: SCons + Bazaar Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
laserlight is offline

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode