how to luk good below program and perfect ?

Question

1.00/5 (1 vote)

See more:

This is Login SYstem Program

In this program any mistec plz tell

C++

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


void userlogin(void);

struct user{
    char username[10];
    char password[10];
}*pUser;

int main()
{
    userlogin ( );

    return 0;
}

void userlogin(void){
    FILE *fp;
    char uName[10], pwd[10];int i;char c;

    pUser=(struct user *)malloc(sizeof(struct user));

    printf("1. Login Through An Existing Account\n2. Create New account\n");
    scanf("%d",& i);
    //system("cls");
    switch(i){
        case 1:
            if ( ( fp=fopen("user.dat", "r+")) == NULL) {
                if ( ( fp=fopen("user.dat", "w+")) == NULL) {
                    printf ("Could not open file\n");
                    exit ( 1);
                }
            }
            printf("Username: ");
            scanf("%9s",uName);
            printf("Password: ");
            scanf("%9s",pwd);
            while ( fread (pUser, sizeof(struct user), 1, fp) == 1) {
                if( strcmp ( pUser-&gt;username, uName) == 0) {
                    printf ("Match username\n");
                    if( strcmp ( pUser-&gt;password, pwd) == 0) {
                        printf ("Match password\n");
                        //accessUser();
                    }
                }
            }
            break;

        case 2:
            do

            {
                if ( ( fp=fopen("user.dat", "a+")) == NULL) {
                    if ( ( fp=fopen("user.dat", "w+")) == NULL) {
                        printf ("Could not open file\n");
                        exit ( 1);
                    }
                }
                printf("Choose A Username: ");
                scanf("%9s",pUser-&gt;username);
                printf("Choose A Password: ");
                scanf("%9s",pUser-&gt;password);
                fwrite (pUser, sizeof(struct user), 1, fp);
                printf("Add another account? (Y/N): ");
                scanf(" %c",&c);//skip leading whitespace
            }while(c=='Y'||c=='y');
            break;
    }
    free ( pUser);//free allocated memory
    fclose(fp);
}

Posted 17-Nov-15 17:42pm

mark_777

Updated 17-Nov-15 17:55pm

PIEBALDconsult

v4

Add a Solution

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

George Jonsson · Accepted Answer · 2015-11-17T18:58:00

I suppose perfect is when the code works according to specification without any bugs or memory leaks. That usually take a few rounds.

One thing to think of is to try to make the code as readable as possible. That includes both the formatting of the code and the number of lines you have in a function.
For me it is more difficult to read code written like this

C++

if ( ( fp=fopen("user.dat", "a+")) == NULL) {
    if ( ( fp=fopen("user.dat", "w+")) == NULL) {
        printf ("Could not open file\n");
        exit ( 1);
    }
}

compared to this

C++

if ((fp = fopen("user.dat", "a+")) == NULL)
{
    if ((fp=fopen("user.dat", "w+")) == NULL)
    {
        printf("Could not open file\n");
        exit(1);
    }
}

This is of course a matter of taste, and the important thing is to be consistent and follow any guidelines in your organisation.

That said there a couple of things in your code that can be improved.
1. Always initialize variables to a default value.
Such as

C++

FILE *fp = NULL;

1.a. Use calloc instead of malloc

C++

pUser=(struct user *)malloc(sizeof(struct user));
pUser = (struct user*)calloc(1, sizeof(struct user));

calloc initializes the allocated memory to 0.

2. fopen("user.dat", "a+")
This statement actually creates the file if it doesn't exist, so the next if-statement is not necessary.

3. If you the fopen function fails you should check the errno variable for more information about what went wrong.

4. You can also see that you are using the same code block in both cases
So better make a function of like

C++

FILE* OpenFile(const char* fileName)
{
    if ((fp = fopen(fileName, "a+")) == NULL)
    {
        printf("Could not open file: '%s'\n", fileName);
        exit(1);
    }
)

5. switch statements tend to be difficult to read when you add more cases, so it is better to create functions for each case.

C++

switch(i)
{
    case 1: fp = OpenFile("user.dat");
            OpenAccount();
            break;
    
    case 2: fp = OpenFile("user.dat");
            CreateAccount();
            break;
    
    default: // Always, ALWAYS have a default statement
            printf("This case is not available\n");
            exit(1);
}

6. The password
Never store passwords as readable text.
Use a hash algorithm, like SHA1, and store the hashed value instead.
Then when the user login you calculate the password he/she enters and compare with the value you have stored.
See for example http://stackoverflow.com/questions/5189257/sha1-function-in-cpp-c[^]

That was all I had the energy to point out for now.