k1st
k1st

Reputation: 101

This program doesn't seem to update the value and verify for the new value

Here is a code i wrote for pin verification, but i cannot make the system verify the new pin. In this program, input is taken from user and is verified using the pre-defined pin. If the user fails to verify the code, then a new code is presented which is to be the new pin.

#include<stdio.h>
#include<stdlib.h>
#include<strings.h>
#include<windows.h>

int pin();  

int main()
{
    int pin();
    {
        int pin,new_pin,old_pin;
        {
            old_pin=1111;
            printf("Enter your pin: ");
            scanf("%d", &pin);

            if(pin==old_pin)
            {
                printf("PIN succefully verified \n");
            }
            else if(pin!=old_pin)
            {
                printf("PIN couldn't be verified. \n");

                printf("Press 1 to generate a new pin, 2 to re-enter or 0 to exit\n");
                scanf("%d", &new_pin);
                if (new_pin==1)
                {
                    printf("Your new pin is: \n");
                    //generating random 4numbered pin

                    int i,random;
                    for(i=0; i<3; i++)
                        random= (rand() );
                    printf("%d\n", random);

                    main();
                    random=new_pin;
                    new_pin=old_pin;
                }
                else if (new_pin==2) {
                    main();
                }
                else{
                    printf("Exiting the program....\n");
                    exit;
                }
            }
        }
    }
    return(0);
}

Upvotes: 1

Views: 110

Answers (1)

user6889435
user6889435

Reputation:

What's wrong there?

First of all, there is need to revisit the code. You reference to function called pin(), that source code we don't know. Next, we run into tiny conflict of names (int pin;, int pin();). There are way too many code blocks. Variables are named poorly (new_pin isn't new pin, old_pin isn't old). Loop for generating pin is useless as you override variable without storing old randoms. Multiline strings without \ in printf's (and others) are forbidden (I spotted them somewhere near Your original code). exit is missing paranthesis. That's about syntax, code optimization and naming quality.

The code is recursively calling main, and that's certainly not a good idea (unless You know, what You are doing!)

How to fix the code.

I've formatted it and removed int pin(); declarations as they were redudant. Complexity of code blocks was reduced and unnecessary library imports were stripped. The for loop was simplified (as it was redudant). Code looks now like this:

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

int main(void) {
    int pin, new_pin, old_pin = 1111;
    printf("Enter your pin: ");
    scanf("%d", &pin);
    if (pin == old_pin)
        printf("PIN succefully verified.\n");
    else if (pin != old_pin) {
        printf("PIN couldn't be verified.\n");
        printf("Press 1 to generate a new pin, 2 to re-enter or 0 to exit\n");
        new_pin = getchar();
        if (new_pin == '1') {
            int i, random;
            printf("Your new pin is: %4d\n" rand() % 9999);
            main();
        } else if (new_pin=='2')
            main();

        else {
            printf("Exiting the program....\n");
            return 0;
        }
    }
}

Now, let's implement the functionality was made incorrectly. New pin mechanism was broken so hard, there is no sense in explaining it. It's better to get brand new one. Values were staticized so they won't disappear after creating new local variable set is made.

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

int main(void) {
    static int pin, new_pin, validPin = 1111;
    printf("Enter your pin: ");
    scanf("%d", &pin);
    if (pin == validPin)
        printf("PIN succefully verified.\n");
    else if (pin != validPin) {
        printf("PIN couldn't be verified.\n");
        printf("Press 1 to generate a new pin, 2 to fall back or any other key to exit.\n");
        new_pin = getchar();
        if (new_pin == '1') {
            validPin = rand() % 9999;
            printf("Your new pin is: %4d.\n", validPin);
            main();
        } else if (new_pin=='2')
            main();

        else {
            printf("Exiting the program.\n");
            return 0;
        }
    }
}

Now, let's revisit main recursion antipattern and divide the code into subprocedures.

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

int proc() {
    /* ... the code in partially unmodified form goes here. */
}

int main(void) {
    while(!proc());
}

Finally, let's mess around with variable scope to reduce it to the absolute minimum and put exit message at the end of main. Code at the end looks like this:

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

int proc() {
    int pin;
    static int validPin = 1111;

    printf("Enter your pin: ");
    scanf("%d", &pin);
    if (pin == validPin)
        printf("PIN succefully verified.\n");
    else if (pin != validPin) {
        int command;
        printf("PIN couldn't be verified.\n");
        printf("Press 1 to generate a new pin, 2 to fall back or any other key to exit.\n");
        command = getchar();
        if (command == '1') {
            validPin = rand() % 9999;
            printf("Your new pin is: %4d.\n", validPin);
            return 0;
        } else if (command == '2')
            return 0;
        else 
            return 1;
    }

    return 1;
}

int main(void) {
    while(!proc());
    printf("Exiting the program.\n");
}

As some online C compilers may incorrectly react to getchar(), just use scanf(), like You actually did:

        int command;
        printf("PIN couldn't be verified.\n");
        printf("Press 1 to generate a new pin, 2 to fall back or 0 to exit:\n");
        scanf("%d", &command);
        if (command == 1) {
            validPin = rand() % 9999;
            printf("Your new pin is: %4d.\n", validPin);
            return 0;
        } else if (command == 0)
            return 1;
          else
            return 0;

Appendix

My final code could have been improved by exploding god-procedure proc() into pin verifier and command handlers, but the application is simple enough to keep things concise. In your learning curve, this application example might be a nice piece of code to train refactoring skills.

I hope my review helped you.

Upvotes: 1

Related Questions