while(user == gullible)

Question

The goal for this project was to:

Write a program that continues to asks the user to enter any number other than 5 until the user enters the number 5. Then tell the user "Hey! you weren't supposed to enter 5!" and exit the program.

★ Modify the program so that after 10 iterations if the user still hasn't entered 5 will tell the user "Wow, you're more patient then I am, you win." and exit.

Requires:

• variables, data types, and numerical operators
• basic input/output
• logic (if statements, switch statements)
• loops (for, while, do-while)

(I'm not doing the 2 stars one yet)

My questions are:

1. How can I optimize my code in every possible way? (efficiency, readability, etc)
2. How can I improve the code syntax?

#include <iostream>  
using std::cout;
using std::cin; 

int main()
{

    int UserNumber = 0;
    int k = 1;
    while (k < 11)
    {
        cout << "\nEnter any number other than 5: ";
        cin >> UserNumber;

        if (UserNumber == 5)
        {
            cout << "\n\n\nHey! You weren't supposed to enter 5!\n\n\n";
            exit(0);
        }

        else if (k == 10)
        {
            cout << "Wow, you're more patient then I am, you win.\n\n\n";
            exit(0);
        }
        k++;
    }

}

Answer 1

A simple rule I (and other) try to apply is : define things in the smallest possible scope.

For instance, int UserNumber = 0; can be moved inside the loop.

Similarly, k can be moved once you've made your loop a for loop : for(int k = 1; k < 11; k++).

Then, computer people love counting from 0. If you write : for(int k = 0; k < 10; k++), I am used to this and I know straight-away that there will be 10 iterations. If you re-index your loop, you'll need to convert else if (k == 10) into a else if (k == 9).

Then, you'll realise that there is no need for this check, you can just move this out of the loop.

Answer 2

Given that the objective of the program is to waste time, I'm not sure that time efficiency is the thing to optimize for :P

As far as anything else goes though there's room for improvement. I think that a for loop makes more sense in this particular case.

for(int k = 0; k < 10; k++)
{
    cout << "\nEnter any number other than 5: ";
    cin >> UserNumber;

    if (UserNumber == 5)
    {
        cout << "\n\n\nHey! You weren't supposed to enter 5!\n\n\n";
        exit(0);
    }
}

cout << "Wow, you're more patient then I am, you win.\n\n\n";
exit(0);

Answer 3

• UserNumber shouldn't be capitalized, it's not class.

• Just for readability sake I'll consider defining constants instead of magic numbers

  #define FORBIDDEN_INPUT 5
  #define MAX_ATTEMPTS 10
  

• k may be better named (maybe attempt)?

• I don't like empty line in if-else statement, I want to know that they should be read together, and I know that, when they are close to each other vertically.

Answer 4

One more thing, which nobody seems to have touched on. This is a somewhat stylistic matter, but it's also style that helps convey intent. Remember that programming is 1% for the compiler and 99% for your fellow programmers, including a later-time yourself.

Normally, if you assign a value on declaration, it's because the initial value of the variable is somehow special or important. Maybe it's a reference counter, or a pointer that needs to point someplace special, or whatever. In C++, if you don't explicitly assign a value on declaration, then the initial value of the variable is indeterminate. This is important if you do something with the value before any further assignment. So, for example, in a class,

int some_value = 0;

is potentially very different from:

int some_value;

if before you make any explicit assignment you perform some operation that reads the value of it, like perhaps:

some_value++;

More generally, in principle

int x = y;

should be equally valid and not look any more odd when written on the longer form of:

int x;
x = y;

In your case, there is nothing special about the initial value of 0. The initial value of the variable is thrown away before you make any comparison involving the variable, or even more generally, read the value of the variable.

When I read your code, it looks like the initial value 0 should be somehow special or important.

You wouldn't write, for some values of x and 0

int x;
x = 0;
cout << "Give me a number: ";
cin >> x;

so based on this line of reasoning, there is equally little reason to write:

int x = 0;
cout << "Give me a number: ";
cin >> x;

which does exactly the same thing (and which is pretty much what you are doing).

Since the initial value isn't in any way important, I would suggest that you just remove the assignment entirely, keeping only the variable declaration:

int UserNumber;

---

I would also suggest using a different name for that variable. Perhaps UserInput capitalized to your liking? To me, UserNumber sounds more like some sort of ID number for the user, than a number which the user inputs.

Answer 5

A couple of things other answers haven't touched upon.

• Your main function is missing its return statement.
• exit(int status) should be avoided for normal program flow, use return instead.

Answer 6

What happens if the user input isn't a number? std::cin >> UserNumber will set the error indicator of that stream; we should test it, and reset the state if necessary.

std::exit is declared by <cstdlib>, so include that header and qualify the function name. More simply, we could just return directly from main(); no need for the the function call.