checking characters

[DJL]

not sure what is wrong with my program.

if I enter a password that is to short then I get back the to short message.
however when I enter the new password it automatically tells me to use uppercase letter.

It seems like its not actually running through the full loop.

The program is suppose to check passwords and make sure they have at least 1 upper case, 1 lower case, 1 number, 1 symbol, and are not shorter than 6 characters long.

I have been messing with this for a couple hours and cant seem to get it right.

 #include <iostream>
#include <cctype>
#include <string>
#include <iomanip>

using namespace std;

bool PasswordCheck(char password[]);

int main()
{

	char password[12];
		
	cout << "Enter your password" << endl;

    cin.getline(password, 12);

	if (strlen(password) > 6 && strlen(password) < 12)
	{
		cout << "Your password is not long enough";
        cout << endl;
		cin >> password;
	}
	if (PasswordCheck(password))
		cout << "Good job";
	        else {
                    cout << endl;
		            cout << "Password invlaid";

                    cout << endl; 
                    cout << endl;
                    cout << "Enter a password ";
		            cin >> password;
	                                }


cout << endl;
cout << endl;
cout << endl;

return 0;
}

bool PasswordCheck(char password[])
{
	int count;

	for(count = 0; count < 12; count++)
	{
		if (isupper(password[count]))
            cout <<  " Password must contain an upper case ";
			return false;
	}
	for (count = 0; count < 12; count++)
	{
		if (islower(password[count]))
            cout << "Password must contain a lower case ";
			return false;
	}
	for (count = 0; count < 12; count++)
	{
		if (isdigit(password[count]))
            cout << "Password must contain a number ";
			return false;
	}
    for (count = 0; count < 12; count++)
    { 
        if (isalpha(password[count]))
            cout << "Password must contian a letter ";
            return false;
    }
    
	return password;
		

[TheToaster]

You need to rewrite your entire passwordCheck function. Delete the entire thing and start over. It is abysmal. In fact, just start the entire program over again. There is just so much wrong with it. Here are just a few I noticed on first inspection.

You store the password in an array of 12, which means that the user can only enter 11 characters, since a null terminator is automatically the 12th character, before the rest of the input is ignored. This means that your test in line 19 WILL ALWAYS fail, and even after failing, you simply ask the user for input again, and the user can simply enter the same incorrect password again and it will accept it since there is no test afterward that prohibits it. So this is completely useless.

Secondly, the PasswordCheck function is really bad. The return type doesn't match, there are FOUR unneccesary loops, and the if statements do absolutely nothing because they just check if the current character contains an upper/lower/digit/alpha, then PRINTS AN ERROR MESSAGE IF IT DOES, EVEN THOUGH THAT IS WHAT IS EXPECTED, and then returns a false value on the very first iteration of the loop (i.e it doesn't iterate more than once). So pretty much, the function will always return false whenever it enters any one of these loops.

You also don't use std::string, which I highly highly recommend over character arrays.

[Handy Andy]

Hello DJL,

The first thing I see is you have written a C++ program with C code and failed with the C code. You are missing the proper header file for the C functions that you are using.

"string" is not the same as or will replace "cstring", the C++ version of, "string.h". Neither of these C header files should be in the program.

"password" should be a "std::string" not a C string.

The if statement on line 19 is not correct. You are saying that is the length of "password" is 7 to 11 characters it is to long. I believe that you want to check if "password" is less than 6.

Line 17 uses "cin.getline" and lines 23 and 34 use formatted "cin >>". This may not work the way that you are thinking.

Watch your indenting it is not consistent.

It may just be a copy thing, but you are missing the closing } on the function.

While I get the program loaded up think about you want or need to do. Also you can pose anything that you may have left out of the instructions for the program.

Andy

[thmm]

are not shorter than 6 characters long.

if (strlen(password) > 6 && strlen(password) < 12)
{
  cout << "Your password is not long enough";
  // ...
}

The condition is length >= 6. It doesn't say anything about 12.
Do you want to write a C or C++ program?

[thmm]

I would implement the password check function like this - assumed you have to use this old c-style strings:

// Passwords  are valid when they have at least 1 upper case, 
// 1 lower case, 1 number, 1 symbol, 
// and are not shorter than 6 characters long.

bool PasswordCheck(char password[], size_t size)
{
  const int MinPasswordLength = 6;

  int num_uppercase = 0, num_lowercase = 0, 
      num_digit = 0, num_symbol = 0;

  size_t len = strlen(password);
  if (len < MinPasswordLength)
    return false;

  for (size_t i = 0; i < size; ++i)
  {
    if (islower(password[i]))
      ++num_lowercase;
    else if (isupper(password[i]))
      ++num_uppercase;
    else if (isdigit(password[i]))
      ++num_digit;
    // else if not sure what symbol is
  }
  return num_lowercase > 0 && num_uppercase > 0 &&
            num_digit > 0 && num_symbol > 0;
}

[Handy Andy]

@Learner2,

Like you I had the same idea last night.Then realized that there is no requirement to count how many times a category hits. It only need to count 1. I decided that defining the variables as "bool"s is a better choice. Then I thought of putting them in "main" and passing by reference, so they can be used later.

Same concept except I used a "std::string" and not a C style "char" array. Either will work.

bool PasswordCheck(std::string password, bool& lcLetter, bool& ucLetter, bool& num, bool& symbol, bool& space)
{
	for (size_t index = 0; index < password.size(); index++)
	{
		if (islower(password[index])) lcLetter = true;
		else if (isupper(password[index])) ucLetter = true;
		else if (isdigit(password[index])) num = true;
		else if (ispunct(password[index])) symbol = true;  // <--- !"#$%&'()*+,-./:;<=>?@[]^_'{|\}~
		else if (isspace(password[index])) space = false;  // <--- Optional.
	}

	return lcLetter && ucLetter && num && symbol && space;
}

Now the function does one thing and one thing only.

Back in main I added:

if (PasswordCheck(password, lcLetter, ucLetter, num, symbol, space))
	cout << "\n  Good job\n";
else
{
	cout << "\n     Password invlaid!\n";

	if (!lcLetter)  // <--- Optional, but does say what is missing.
		std::cout << "\n     Password must contain at least 1 lower case letter.";

Not only does it tell you that the password is invalid, but what is missing.

I get an output that looks like this:

 Enter a password.
 Password should be at least 6 characters and no more than 12 characters.
 Password should contain:
 1 lower case letter, 1 upper case letter, 1 number and 1 symbol (!"#$%&'()*+,-./:;<=>?@[]^_'{ | \ }~).

 Enter your password: asd1ghjk

     Password invlaid!

     Password must contain at least 1 upper case letter.
     Password must contain at least 1 symbol.

 Press Enter to continue:


 Enter a password.
 Password should be at least 6 characters and no more than 12 characters.
 Password should contain:
 1 lower case letter, 1 upper case letter, 1 number and 1 symbol (!"#$%&'()*+,-./:;<=>?@[]^_'{ | \ }~).

 Enter your password: asd1 ghjk

     Password invlaid!

     Password must contain at least 1 upper case letter.
     Password must contain at least 1 symbol.
     Password must not contain any spaces.

 Press Enter to continue:

See what you think.

Andy

[thmm]

@Handy Andy,
your solution is more sophisticated and I like it better than mine
There is really no need to count the number of digits...