How to compare char array to cstring and loop program

Question

My program is supposed take a user input to create a password. There's specific criteria.

The user should be able to retry up five times to create a "good" password. If they make a "good" password, the program should just end. If they don't make a password within the 5 tries, then the program should end. I tried using do-while and a for loop, but it won't break or end the program.

I have a text file with 5 passwords. The user can't use those 5 passwords. I put the passwords into an char array. I'm not sure how to compare the passwords in the file to the user input.

I can only use cstrings for this program. I'm not allowed to use any strings.

"prevPswds.txt"

2347UCDa!
PassUCD97~
@489sqUCD
123AaaUCD$%
UCDAsue1,

main.cpp:

#include <iostream>
#include <cstring>
#include <cctype>
#include <fstream>
//#include <string.h>

using namespace std;

int main()
{
    //Initialize variables
    const int size = 15;
    char* check = NULL;

    char checkUCD[] = "UCD";
    char specialChar[] = { '~', '!', '@', '#', '$', '%', '^', '&', '*', '-', '_', '?' };

    char password[size];

    int counter=0;

    bool length = false;
    bool uppercase = false;
    bool lowercase = false;
    bool number = false;
    bool special = false;
    bool ucd = false;
    bool previous = false;

    bool lower = false;
    bool upper = false;
    bool num = false;

    //bool done = false;

    ifstream file("prevPswds.txt");
    const int arraySize = 150;
    char myArray[arraySize];
    char current_char;
    int count = 0;

 //Read in file
  for (int k = 0; k < arraySize; k++)
  {
     if (file.is_open())
      {
          int c = 0;
          while (!file.eof())
          {
              file.get(myArray[c]);
              c++;
              count++;
          }
      }
  }



  for (int i = 0; i < 5; i++)
  {
    cout << "----CREATE A PASSWORD----" << endl;
    cout << "Password requires:" << endl;
    cout << "-8 to 12 characters" << endl;
    cout << "-At least 1 uppercase letter" << endl;
    cout << "-At least 1 lowercase letter" << endl;
    cout << "-At least 1 number" << endl;
    cout << "-At least 1 special character (~, !, @, #, $, %, ^, &, *, -, _, ?)" << endl;
    cout << "-'UCD' \n" << endl;

    cout << "Password cannot include:" << endl;
    cout << "-Lowercase letters  'l,' 'i,' 'o,' or 'z'" << endl;
    cout << "-Uppercase letters 'I,' 'O,' or 'S'" << endl;
    cout << "-Numbers '0,' '1,' or '5'" << endl;
    cout << "-------------------------" << endl;



        //Get user input
        cout << "Please enter a password." << endl;
        cin.getline(password, size);
        cout << endl;
        counter++;
        


        //Check requirements
        
        if (strlen(password) < 8 || strlen(password) > 12)
        {
            cout << "-Password must be 8 - 12 characters." << endl;
        }
        else
        {
            length = true;
        }

        for (int i = 0; i < size; i++)
        {

            if (isupper(password[i])) //Check for uppercase
            {
                uppercase = true;
            }

            if (islower(password[i])) //Check for lowercase
            {
                lowercase = true;
            }

            if (isdigit(password[i])) //Check for numbers
            {
                number = true;
            }

            if (password[i] != 'l' || password[i] != 'i' || password[i] != 'o' || password[i] != 'z') //Check for exceptions
            {
                lower = true;
            }

            if (password[i] != 'I' || password[i] != 'O' || password[i] != 'S') //Exceptions
            {
                upper = true;
            }

            if (password[i] != '0' || password[i] != '1' || password[i] != '5') //Exceptions
            {
                num = true;
            }
        }

        for (int i = 0; i < size; i++) //Check for special characters
        {
            if (specialChar[i])
            {
                if (ispunct(password[i]))
                {
                    special = true;
                }
            }
        }


        check = strstr(password, checkUCD); //Check for 'UCD'
        if (check)
        {
            ucd = true;
        }


        //Give feedback and suggestion
        if (uppercase == false)
        {
            cout << "Password must contain at least 1 uppercase letter." << endl;
        }

        if (lowercase == false)
        {
            cout << "Password must contain at least 1 lowercase letter." << endl;
        }

        if (number == false)
        {
            cout << "Password must contain at least 1 number." << endl;
        }

        if (special == false)
        {
            cout << "Password must contain at least 1 special character." << endl;
        }

        if (ucd == false)
        {
            cout << "Password must contain 'UCD.'" << endl;
        }

        if (lower == false)
        {
            cout << "Password cannot contain 'l', 'i', 'o', or 'z.'" << endl;
        }

        if (upper == false)
        {
            cout << "Password cannot contain 'I', 'O', or 'S.'" << endl;
        }

        if (num == false)
        {
            cout << "Password cannot contain '0', '1' or '5.'" << endl;
        }
  }

  if (length == true || uppercase == true || lowercase == true || number == true || special == true || ucd == true || previous == true || lower == true || upper == true || num == true)
  {
    return 1;
  }
}

Answer 1

This is getting a bit long, so I have tried to add some small headers so you can jump to the next issue easier.

First thing first: Why is “using namespace std;” considered bad practice? Do yourself a favor and stop using it. It will be much harder to change if you let it become an ingrown habit, and at some point you will run into trouble and be forced to change.

Secondly, you have also run somewhat afoul of: Why is iostream::eof inside a loop condition (i.e. while (!stream.eof())) considered wrong? I think that your code actually does work, but I also think that it is a bit by chance.

    for (int k = 0; k < arraySize; k++)
    {
        if (file.is_open())
        {
            int c = 0;
            while (!file.eof())
            {
                file.get(myArray[c]);
                c++;
                count++;
            }
        }
    }

The outer loop starts at k=0, then we arrive at the inner loop, where you read all the characters in the file - and then you read one character more, because eof is only set when you try to read beyond the end (with get). However, since you don't try to process that character here, you don't notice that the last read failed - though your count may be off by one.

Once the inner loop terminates, you then go back to the outer loop and set k=1. But since you have already read the whole file, the inner loop is simply skipped. And so on for k=2 to arraySize.

As you can see, the file reading has some problems, so don't do it like that. :)

Another consideration is that you probably don't want a single array of characters mashed together, it may be more desirable to have the five old passwords individually - and they are nicely all on their own line in the file.

My suggestion is that since you have 5 old passwords (can this number change?) that can be up to 12 or size (can this number change?) characters long, then you change myArray to char myArray[5][size]{}. Then use getline to read the old passwords, like so:

    for (int k = 0; k < 5 && file.getline(myArray[k], size); k++)
    {}

I think this will make your compare job easier, as you only have to loop over the five arrays, and compare myArray[k][i] == password[i].

Third issue is your checks. I would simplify them - unless you have a good reason for wanting to be able to see which ones failed later. Instead of:

            if (isupper(password[i])) //Check for uppercase
            {
                uppercase = true;
            }
            if (islower(password[i])) //Check for lowercase
            {
                lowercase = true;
            }
...
        if (uppercase == false)
        {
            cout << "Password must contain at least 1 uppercase letter." << endl;
        }
        if (lowercase == false)
        {
            cout << "Password must contain at least 1 lowercase letter." << endl;
        }
...
    if (length == true || uppercase == true || lowercase == true || number == true || special == true || ucd == true || previous == true || lower == true || upper == true || num == true)
    {
        return 1;
    }

Change it to:

            if (!isupper(password[i])) //Check for uppercase
            {
                passwordIsGood = false;
                cout << "Password must contain at least 1 uppercase letter." << endl;
            }
            if (!islower(password[i])) //Check for lowercase
            {
                passwordIsGood = false;
                cout << "Password must contain at least 1 lowercase letter." << endl;
            }
...
        if (passwordIsGood)
        {
            return 1;
        }

It will make your code more unified, and easier to read.

Fourth: It will also make it easier to fix the bug that you don't reset the booleans on each "create password" loop. You set f.ex. length = false when you declare the variable, then length = true after checking that it is correct. But then if that password fails on some other issue length is never reset, so the next password can be the wrong length but length is already true so the check fails.

The fifth problem is that you have placed your check if outside the password loop. The structure of your program is:

    for (int i = 0; i < 5; i++)
    {
        cout << "----CREATE A PASSWORD----" << endl;

... lots of lines ...

    }

    if (length == true || uppercase == true || lowercase == true || number == true || special == true || ucd == true || previous == true || lower == true || upper == true || num == true)
    {
        return 1;
    }

So you always loop five times through "CREATE A PASSWORD" before you check if the requirements are fulfilled.

My recomendation here is to refactor.

Move all your password checking into a function of its own:

bool checkPassword(char* password, int size)
{
    if (strlen(password) < 8 || strlen(password) > 12)
    {
        cout << "-Password must be 8 - 12 characters." << endl;
        return false;
    }

    for (int i = 0; i < size; i++)
    {

        if (!isupper(password[i])) //Check for uppercase
        {
            cout << "Password must contain at least 1 uppercase letter." << endl;
            return false;
        }
        ...
    }
    ...
    return true;
}

Then your loop becomes:

    for (int i = 0; i < 5; i++)
    {
        cout << "----CREATE A PASSWORD----" << endl;
...
        //Get user input
        cout << "Please enter a password." << endl;
        cin.getline(password, size);
        cout << endl;
        counter++;

        //Check requirements
        bool passwordIsGood = checkPassword(password, size);

        if (passwordIsGood)
        {
            return 1;
        }
    }

Now it suddenly is much easier to see what is going on.

The sixth problem is that some of your checks don't do what you want. Specifically the ones like this:

if (password[i] != 'l' || password[i] != 'i' || password[i] != 'o' || password[i] != 'z')

Note that only one of the conditions in an OR has to be true for the OR to be true. If a=1, then a!=1 is false, but a!=2 is true. This means that a != 1 || a != 2 is true. a would have to be both 1 and 2 at the same time for a != 1 || a != 2 to be false.

One option for getting the behavior you want is to change the check to:

if (!(password[i] == 'l' || password[i] == 'i' || password[i] == 'o' || password[i] == 'z'))

That became way longer than I had intended... I hope it helps you at least some. :)

Answer 2

if (length == true || uppercase == true || lowercase == true || number == true || special == true || ucd == true || previous == true || lower == true || upper == true || num == true)
  {
    //return 1; take this line out and place after all {}
    i = 5; //should end the for loop
  }
}
return 1;