What am I doing wrong?? (simple)

Ok so I am quite sure everyone will find this stupidly simple but I am trying to learn C++ and am still in the MOST basic steps, just made it to Loop section in Jumping Into C++ E-book. Anyways here is my question that I need to answer (I am stuck on a practice problem):

Write a menu program that lets the user select from a list of options, and if the input is not one of the options, reprint the list...

*pertinent info: it works without the OR operator so essentially if "1" is the only option...

Code:

#include <iostream>
#include <string>


using namespace std;


int main()
{
    string num;
    cout << "Please choose from the options: 1,2,3: ";
    cin >> num;
    while ( (num != ("1")) || (num != ("2")) || (num != ("3")))
    {
        cout << "Wrong option--try again (1,2,3): ";
        cin >> num;
    }
}

Thank you in advance to any help received!

Consider this expression:

Code:

(num != ("1")) || (num != ("2")) || (num != ("3"))

What does it evaluate to if num == "0"? What about num == "1"? What about num == "2"?

Originally Posted by laserlight

Consider this expression:

Code:

(num != ("1")) || (num != ("2")) || (num != ("3"))

What does it evaluate to if num == "0"? What about num == "1"? What about num == "2"?

0 would result in all being true thus the loop would run again.
1 would result in (false, true, true) run again.
2 would result in (true, false, true) run again.

Darn it ok it always evaluates to true no matter what, I see why that would be a problem, no matter what it will always run :/. I fail. I fixed it this way... Thank you very much for your help!!!

Code:

#include <iostream>
#include <string>


using namespace std;


int main()
{
    string num;
    cout << "Please choose from the options: 1,2,3: ";
    cin >> num;
    while (1)
    {
        if (num == "1" || num == "2" || num == "3")
        {
            break;
        }
        else
        {
            cout << num <<" was not an option please try again.";
            cin >> num;
        }
    }
    cout << "Thank you!";
}

While loops run on inverted logic. That is, it loops while the answer is wrong.
The easiest way to create a complex condition is IMO to think about when a loop must end, then invert that logic.
Similarly, when reading a condition, invert it first and then you know when it will end. Give it a try.

Thanks Elysia!

Originally Posted by rTeapot

Code:

#include <iostream>
#include <string>


using namespace std;


int main()
{
    string num;
    cout << "Please choose from the options: 1,2,3: ";
    cin >> num;
    while (1)
    {
        if (num == "1" || num == "2" || num == "3")
        {
            break;
        }
        else
        {
            cout << num <<" was not an option please try again.";
            cin >> num;
        }
    }
    cout << "Thank you!";
}

This is more simply written as:

Code:

while ( !(num == "1" || num == "2" || num == "3") )
    {
        cout << num <<" was not an option please try again.";
        cin >> num;
    }

To simplyfy it further, you can apply DeMorgan’s Rules for distributing the negation:

Code:

while ( num != "1" && num != "2" && num != "3" )
    {
        cout << num <<" was not an option please try again.";
        cin >> num;
    }

Which literally reads "while num is not equal to 1, and num is not equal to 2, and num is not equal to 3".

Personally, I'd reduce it even further

Code:

while (num.size() == 1 && num[0] >= '1' && num[0] <= '3')

True, this takes slightly more typing, but it offers other advantages such readability (arguably more clear that it's testing for input in a range) and being more easily extended to more than 3 cases.

Thank you everyone for your quick replies and help! To grumpy and King Mir, thank you for showing me how you would shorten it, I feel like writing shorter code comes with more experience because currently I find it really hard to initially write that way... I have to write it the long way it seems and then try to think of ways to shorten it... Like the one below it gets the job done but its much longer than it should be, at least I have a feeling it can be shortened a lot haha, I will keep working at it...

1. Take the "menu program" you wrote earlier and break it out into a series of calls to functions for
each of the menu items. Add the calculator and "100 bottles of beer" as two different functions
that can be called.
2. Make your calculator program perform computations in a separate function for each type of
computation.

Code:

#include <iostream>
#include <string>


using namespace std;


double add (double x, double y)
{
    return x + y;
}
double mult (double x, double y)
{
    return x * y;
}
double sub (double x, double y)
{
    return x - y;
}
double div (double x, double y)
{
    return x / y;
}


string selection;


int main()
{
    while (selection != "Calculator" && selection != "99")
    {
        cout << "Please pick a selection from the following menu by typing it out and hitting enter:" <<endl;
        cout << "Calculator" <<endl;
        cout << "99" <<endl;
        cin >> selection;
    }


    if (selection == "Calculator")
    {
        string op;
        double x;
        double y;


        cout << "Please enter an arithmetic operator (+,-,*,/): ";
        cin >> op;


        cout << "Please enter a number: ";
        cin >> x;


        cout << "Please enter another number: ";
        cin >> y;


        if ( op == "+")
        {
            cout << "The sum is: " << add(x,y);
        }
        if ( op == "-")
        {
            cout << "The difference is: " << sub(x,y);
        }
        if ( op == "*")
        {
            cout << "The product is: " << mult(x,y);
        }
        if ( op == "/")
        {
            cout << "The dividend is: " << div(x,y);
        }
    }


    if (selection == "99")
    {
        for ( int beer = 99; beer > 0; beer--)
        {
        cout << beer << " Bottles of Beer on the wall, " << beer << " Bottles of Beer, take one down and pass it around, " << beer - 1 << " Bottles of Beer on the wall." <<endl;
        }
    }
}

Personally, I don't think the add, mult, sub and div functions help. You might as well use the operators directly in main. These functions could help if you map the strings (e.g., "+") to the function pointers and then do a map lookup to perform the operation, but that might be a little more advanced than what you intend right now.

Instead, I suggest moving the entire calculation section to another function. Also, since op cannot be "+" and "-" at the same time, it makes sense to use an if/else chain instead of just a series of if statements.

Thanks for the input laserlight, I agree that the way I did it was a bit bulky but I am a little new to this so that's what I came up with, still working at getting my writing minimalistic and streamlined...