I would like to understand why the loop will not loop if i != 1234 and ask to "please try again".

#include <iostream>

int main() {
    //i want to create a program to stop after three attempts

    int i = 1234;

    for (i = 0; i < 3; i  ) {

        std::cout << "enter pin ";
        std::cin >> i;

        if (i == 1234) {
            std::cout << "Thank you" << "\n";
        }
         
        else if (i < 1000 || i > 9999) {
            std::cout << "Invalid" << "\n";
        }

        else {
            std::cout << "incorrect, please try again" << "\n";

        }

    }
}

CodePudding user response：

This has several mistakes. First, you "blow away" the value of i by replacing it with input:

std::cin >> i;

In general, reusing the same variable for multiple things is a bad practice because it can lead to defects like that one. One variable should only be used for one thing.

Also, you don't break out of the loop in the case where they enter the correct value.

CodePudding user response：

you need to use a different variable for your input value. if you input anything 3 or greater then the loop will end.

CodePudding user response：

First, a mini-code review:

#include <iostream>

int main() {
    //i want to create a program to stop after three attempts

    int i = 1234;  // Should have a different name

    for (i = 0; i < 3; i  ) {

        std::cout << "enter pin ";
        std::cin >> i;  // Overwrites value of i, throwing loop off

        if (i == 1234) {
            std::cout << "Thank you" << "\n";  // Still repeats
        }
        // I subjectively don't like these blank lines.
        // I prefer if/else blocks to be contiguous.
        else if (i < 1000 || i > 9999) {
            std::cout << "Invalid" << "\n";  // No need to separate these
        }

        else {
            std::cout << "incorrect, please try again" << "\n";

        }

    }
}

Overall, not awful compared to some of the other beginner code I've seen here.

The issue is that you are trying to use the variable i for many different purposes. Anything you write/use in programming should be singular in purpose. You have three separate tasks: the stored PIN, the loop counter, and the user's input. You should have three separate variables.

Finally, you don't say whether or not 0000 is a valid PIN or not. Typically it is, even if the security of it is awful. However, reading the PIN as an int doesn't make this possible. So let's use a std::string instead.

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

bool is_all_digits(const std::string& val) {
    for (const auto& c : val) {
        if (!std::isdigit(c)) {
            return false;   
        }
    }
    
    return true;
}

int main()
{
    std::string pin{"1234"};

    // Altered number of iterations to exercise all possible scenarios.
    for (int i = 0; i < 4;   i) {
        std::string input;
        std::cout << "Enter PIN: ";
        std::getline(std::cin, input);
        
        // Error check first
        // First, is the length correct?
        if (input.length() != 4) {
            std::cout << "Invalid. Try again.\n\n";
            continue;  // Go to next loop iteration; no need to
                       // make the other checks.
        }
        
        // input must have length of 4 to reach here
        if (!is_all_digits(input)) {
            std::cout << "Please enter a valid PIN.\n\n";
            continue;
        }
        
        // input must have length of 4 and be all digits to reach here
        if (input == pin) {
            std::cout << "Thank you.\n";
            break;  // User entered correct PIN, we get out of the loop.
        } else {
            // Not bothering with continue; if this branch is taken, the loop
            // restarts automatically.
            std::cout << "Incorrect.\n\n";
        }
    }
}

Output:

Enter PIN: cut & dry
Invalid. Try again.

Enter PIN: l337
Please enter a valid PIN.

Enter PIN: 5678
Incorrect.

Enter PIN: 1234
Thank you.

If you are allowed to pretend that 0000 is not valid, then you can continue to use ints. The logic above will only need minor tweaks.