Password login program loops forever and displays incorrect results

Question

I'm trying to create a password login program in C++. So there is some mistake in looping.

If the entered password satisfied all the conditions means it had to come out of loop but it doesn't end. So if anyone know means explain me properly. I just have started to learn programming.

Look at the output first. It is showing it must include uppercase and digits and I enter some correct password and when I enter the wrong password knowingly, it is showing that it's good password, which is unexpected for me.

Here's what I've attempted to do:

#include <iostream>
#include <string.h>
#include <ctype.h>

using namespace std;

int main()
{
    int alp = 0, i, num = 0, j;
    char a[10];

    do
    {
        cout << "\nEnter a password:";
        cin >> a;

        if (strlen(a) > 8)
        {
            for(i = 0; i <= sizeof(a); i++)
            {
                if(isupper(a[i]))
                    alp++;
                else if(isdigit(a[i]))
                    num++;
            }
            if (alp > 0 && num > 0)
                cout << "\nGood password\n";
            else
                cout << "Your password must include atleast one digit and one uppercase\n";
        }
        else
            cout << "\nYour password must have atleast 8 characters";
    } while(true);

    return 0;
}

Here's the output:

Enter a password:harry

Your password must have atleast 8 characters
Enter a password:harrypot  

Your password must have atleast 8 characters
Enter a password:harrypott
Your password must include atleast one digit and one uppercase

Enter a password:Harry1817t 

Good password

Enter a password:harrypott

Good password

Enter a password: // forever

Any help on this is appreciated.

why do you expect the loop to end? `while(true)` basically means "loop forever" — 463035818_is_not_an_ai, Jul 06 '20 at 14:33
A `while(true)` loop without a `break` statement will loop forever. — ForceBru, Jul 06 '20 at 14:33
While(true) starts an endless loop. Remove it and you are fine. — andrew28349, Jul 06 '20 at 14:34
`} while (alp == 0 || num == 0);` Also, don't forget to reset `alp = num = 0;` each iteration. — 001, Jul 06 '20 at 14:41
Note: `cin>>a;` what happens when user enters a string larger than 9 characters? You could use `std::string` or `std::istream::getline()`. — 001, Jul 06 '20 at 14:48
yeah there may be a chance of that . ok sir i'll think of it.maybe we can use dynamic memory allocation like that. Thank you for responding — Saravanan R, Jul 06 '20 at 14:53
Also, take a look at `for(i = 0; i <= sizeof(a); i++)` -- this should be `for(i = 0; i < strlen(a); i++)` otherwise you will be read in uninitialized memory giving you weird results. — Andy, Jul 06 '20 at 15:25

RedEyesBlueDragon · Answer 1 · 2020-07-06T14:42:01.663

0

You should add "break;" statment to where you want to break the loop after like

  if(alp>0 && num>0){
        cout<<"\nGood password\n";
        break;
        }

Or add some condition which break the loop

       while(i<1){
            if(alp>0 && num>0){
                 cout<<"\nGood password\n";
                 i=2;
                 }
       }

edited Jul 06 '20 at 14:42

answered Jul 06 '20 at 14:36

RedEyesBlueDragon

79
4

1

You need to add some `{` and `}` into your answer... – ChrisMM Jul 06 '20 at 14:40
Thanks for answering sir .I try this – Saravanan R Jul 06 '20 at 14:48
The answer seems to be incomplete, nowhere mentioned to reset the variables on each iteration to prevent malfunction on next iteration. – Rohan Bari Jul 06 '20 at 15:06

score 0 · Answer 2 · answered Jul 06 '20 at 14:53

Try using break when the password is good and also bad stuff will happen if the input is bigger than 9 characters, use std::string instead of char array something like this:

#include<iostream>
#include<string>
using namespace std;

int main()
{
    int alp=0,num=0;
    string a;
    do
    {
        cout<<"\nEnter a password:";
        cin>>a;
        if(a.length()>8)
        {
            for(int i=0; i<a.length(); ++i)
            {
                if(isalpha(a[i]))
                {
                        ++alp;
                }
                else if(isdigit(a[i]))
                {
                        ++num;
                }
            }
            if(alp>0 && num>0)
            {
                cout<<"\nGood password\n";
                break;
            }
            else
            {
                cout<<"Your password must include atleast one digit and one uppercase\n";
            }
        }
        else
        {
            cout<<"\nYour password must have atleast 8 characters";
        }
    }while(true);

    return 0;
}

Rohan Bari · Accepted Answer · 2020-07-06T15:13:12.337

There are several defects in your code:

The while(true) is a non-terminating loops, but no break is added.
The alp and num weren't reset at the end of the loop iteration.
Since the alp and num weren't reset, they're incremented in each iteration.

A few notes on the code:

You don't need to include string.h or ctype.h after iostream.
In C++, you may use std::string class to manipulate better with strings by using a number of helpful class functions (e.g. if you want to get the string length, then use variable.length(), etc.)
You shouldn't use:
```
using namespace std;
```
In large programs, they may cause ambiguity. Why is using namespace std considered bad practice?
The variable j is unused and redundant declaration.

Enhanced version of your code is as follows:

#include <iostream>
#include <string>

int main(void) {
    // important declaration
    std::string password;
    char temp;
    int alpha = 0, digit = 0;

    do {
        std::cout << "Enter a password: ";
        std::cin >> password;

        // getting the length
        size_t len = password.length();

        // conditions begins, checks if the password is greater than 8 (9 or more)
        if (!(len > 8)) {
            std::cout << "Password must be greater than 8 chars." << std::endl;
            continue;
        }

        // testing each letter
        for (size_t i = 0; i < len; i++) {
            temp = password[i];
            // if the char is an alphabet, alpha++
            if (isalpha(temp)) alpha++;

            // or a digit, digit++
            if (isdigit(temp)) digit++;
        }

        // verifying if digit and alpha are greater than 0    
        if (alpha > 0 && digit > 0)
            break;
        else {
            std::cout << "The password must have an alphabetic letter and a digit." << std::endl;
            alpha = digit = 0;
        }
    } while (true);

    // prints when out of loop reaches
    std::cout << "Password was set successful." << std::endl;

    return 0;
}

Output (sample test cases):

Enter a password: asdf // --------------------------------- < 8 chars
Password must be greater than 8 chars.

Enter a password: safasdfasdfasdf // ---------------------- > 8 chars, but no numeric
The password must have an alphabetic letter and a digit.

Enter a password: 34523523452345 // ----------------------- > 8 chars, but no alpha
The password must have an alphabetic letter and a digit.

Enter a password: asdfa34 // ------------------------------ alpha + numeric, but < 8 chars
Password must be greater than 8 chars.

Enter a password: asfasdfdsaf2 // ------------------------- a good password
Password was set successful.

Thank You bro .In my college they taught me using only "using namespace std".so that i use .But you are using a different technique.std::cout like that it is new so is there any different in that. — Saravanan R, Jul 07 '20 at 14:48
@SaravananR if you use `using namespace std;` then you don't need to use `std::` anywhere in the code. But in big programs, this will cause ambiguity if a global variable and a local variable or a built-in variable has the same name. — Rohan Bari, Jul 07 '20 at 14:51
Thanks for your response bro .so I'm new to stack overflow so if i have any doubts like this can i ask you bro — Saravanan R, Jul 07 '20 at 14:55
Bro ,In this code you have used size_t len= password.length() — Saravanan R, Jul 07 '20 at 15:06
is this any default variable or datatype or something .can you explain — Saravanan R, Jul 07 '20 at 15:07
@SaravananR `size_t` means `unsigned long long` and in some systems `unsigned long`. And it's a `typedef` definition and expands to what I mentioned in the first sentence. — Rohan Bari, Jul 07 '20 at 15:19

Password login program loops forever and displays incorrect results

3 Answers3