Thread: What am I doing wrong?? (simple)

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"?

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.

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.