Skip to main content
2 of 5
added 113 characters in body
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65
  1. As a first step, fix the indentation.
  2. As a second step, eliminate superfluous blocks, and use a block if a loop / if / else contains anything longer.
  3. Next, look at your comments. They just restate the code badly, and are thus worse than useless. Comments should be used to explain why something is done, or to express pre- and post-conditions as well as return-values, if it cannot be done acceptably in code.
  4. Don't abbreviate names more than neccessary to make them useable. Descriptive names are essential to making code self-explaining.
  5. Add a space around every binary operator, and after a comma.
  6. Avoid over-long lines. Horizontal scrolling, especially combined with vertical scrolling, kills readability.
  7. Extract the evaluation of the password into its own function for reusability.
  8. Test every function which can fail for failure.
  9. Don't read an unknown amount of data into a finite buffer. Buffer-overflows are bad.
  10. Don't pass a pointer to a pointer to char where a pointer to char is expected.
  11. Be aware that passing anything but the value of an unsigned char or EOF to the character-classification-functions isdigit and isupper is undefined behavior. Cast to unsigned char.
  12. Make sure to end your output with a newline, or the next prompt in the terminal will be in a weird position.
  13. You don't use anything from <conio.h> so don't include it.
  14. As an aside, don't you also want a lower-case letter?

Applying all that yields:

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

static int validPassword(const char* password) {
    int hasUpper = 0, hasDigit = 0, hasDollar = 0 /*, hasLower = 0*/;
    const unsigned char* p = (const unsigned char*)password;
    while(*p) {
        if(isupper(*p))
            hasUpper = 1;
        /*else if(islower(*p))
            hasLower = 1;*/
        else if(isdigit(*p))
            hasDigit = 1;
        else if(*p == '$')
            hasDollar = 1;
    }
    return hasUpper && hasDigit && hasDollar /* && hasLower*/;
}

int main() {
    char password[32];

    puts("Please enter a password having an upper case letter, a number"
        " and also a $ sign: ");
    if(scanf("%31s", password) != 1) {
        puts("Could not get the password.");
        return EXIT_FAILURE;
    }
    if(strlen(password) > 30) {
        puts("Password is too long.");
        return EXIT_FAILURE;
    }
    if(!validPassword(password)) {
        puts("Password must contain an upper case letter, a number and a $ sign.");
        return EXIT_FAILURE;
    }
    puts("Nice Password!");
    return 0;
}
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65