3
\$\begingroup\$

Task:

Write a program that presents the user w/ a choice of your 5 favorite beverages (Coke, Water, Sprite, ... , Whatever). Then allow the user to choose a beverage by entering a number 1-5. Output which beverage they chose.

★ If you program uses if statements instead of a switch statement, modify it to use a switch statement. If instead your program uses a switch statement, modify it to use if/else-if statements.

★★ Modify the program so that if the user enters a choice other than 1-5 then it will output "Error. choice was not valid, here is your money back."


#include <iostream>
using namespace std;

int main() {

int drink = 0;
string ifswitch;
string again = "y";


while( again == "y" || again == "Y")
{
    cout << "Would you like to use if or switch statements?: ";
    cin >> ifswitch;
    cout << endl;

    if(ifswitch == "switch" || ifswitch == "Switch")
    {
        cout << "Now using switch statements..." << endl << endl;

        cout << "Your choices of drinks are: " << endl;
        cout << "1 - Coke" << endl;
        cout << "2 - Sprite" << endl;
        cout << "3 - Water" << endl;
        cout << "4 - Orange Juice" << endl;
        cout << "5 - Apple Juice" << endl;

        cout << "What drink would you like (1/5): ";
        cin >> drink;
        cout << endl;

        switch(drink)
        {
            case 1 :
                cout << "You chose Coke." << endl;
                break;
            case 2 :
                cout << "You chose Sprite." << endl;
                break;
            case 3 :
                cout << "You chose Water." << endl;
                break;
            case 4 :
                cout << "You chose Orange Juice." << endl;
                break;
            case 5 :
                cout << "You chose Apple Juice." << endl;
                break;  
            default :
                cout << "Error. Choice was not valid, Here is your money back.";

        }   
    }
    else if(ifswitch == "if" || ifswitch == "If")
    {
        cout << "Now using if statements..." << endl << endl;

        cout << "Your choices of drinks are: " << endl;
        cout << "1 - Coke" << endl;
        cout << "2 - Sprite" << endl;
        cout << "3 - Water" << endl;
        cout << "4 - Orange Juice" << endl;
        cout << "5 - Apple Juice" << endl;

        cout << "What drink would you like (1/5): ";
        cin >> drink;
        cout << endl;

        if(drink == 1)
            cout << "You chose Coke." << endl;
        else if(drink == 2)
            cout << "You chose Sprite." << endl;
        else if(drink == 3)
            cout << "You chose Water." << endl;
        else if(drink == 4)
            cout << "You chose Orange Juice." << endl;
        else if(drink == 5)
            cout << "You chose Apple juice." << endl;
        else
        cout << "Error. Choice was not valid, Here is your money back.";        

    }   

    cout << "Would you like to go again? (Y/N)" << endl;
    cin >> again;
    cout << endl;

}

    cout << "Ending...";

}
\$\endgroup\$
4
\$\begingroup\$

I think you should indent main() a bit better, but that's probably because of copy/pasting it here.

So, having that said, your code is really ok in my opinion. There are just several things I find not so good.

Don't use using namespace std

Please see Why is using namespace std bad practice? for an explanation why it is bad to use it. You should avoid it most of the time, especially in header files! But in the source file, and because this is not a 100k line project, it might be ok :)

What happens when I enter SwItCh??

Well, it just says: Would you like to go again? (Y/N)\n without any message or something.

The default way to check case-insensitive input is to convert the input to all lowercase, and compare that:

std::cin >> ifswitch;
std::transform(ifswitch.begin(), ifswitch.end(), ifswitch.begin(), ::tolower);

if (ifswitch == "switch"); //...

I would say this is overkill for a single character, like again.

Use chars for single characters, not std::string

You don't need a whole string if you only need 1 character. This is the case with again, which stores only 1 character and expects only 1 character.

Don't endlessly flush the stream with std::endl

std::endl is a big performance bottleneck because it outputs '\n', but then uses std::flush, which flushes the stream. It is really unnecessary to do that every time you want a new line, so prefer using just \n:

std::cout << "Now using switch statements...\n\n";

On some platforms, you have to flush the stream to see the output. In this case, better be explicit and use std::flush.

Remove duplicate code

Duplicate code is a maintenance headache: If you want to add a sixth drink, you have to change it 2 times at 2 different places. What happens if you forget?

That's why it is better to remove code duplication using functions:

int chooseDrink() 
{
    std::cout << "Your choices of drinks are: \n";
    std::cout << "1 - Coke\n";
    std::cout << "2 - Sprite\n";
    std::cout << "3 - Water\n";
    std::cout << "4 - Orange Juice\n";
    std::cout << "5 - Apple Juice\n";

    std::cout << "What drink would you like (1/5): ";

    int drink = 0;
    std::cin >> drink;

    std::cout << '\n';

    return drink;
}

Duplicate strings?

Like code, you have duplicate string literals. It has the exact same problem has with code: you have to change the string at 2 places, instead of 1. In a typical project, you would use a map, string constants or load them from file. Doesn't really matter for such a small project, you don't have 1k strings :)

Output a newline for the last output

My terminal looks like this if you forget to output the last newline:

$ ./vendingmachine
Would you like to use if or switch statements?: if

Now using if statements...

Your choices of drinks are: 
1 - Coke
2 - Sprite
3 - Water
4 - Orange Juice
5 - Apple Juice
What drink would you like (1/5): 2

You chose Sprite.
Would you like to go again? (Y/N)
n

Ending...$

Now I'm gonna enter the next command on the same line as Ending..., which is not really ideal. Consider outputting a newline.

\$\endgroup\$
  • 1
    \$\begingroup\$ It's important to note that the tolower/toupper approach, while enough for simple tasks like these, does not cover all of Unicode. \$\endgroup\$ – Rhymoid Oct 7 '16 at 15:07
  • \$\begingroup\$ @Rhymoid Well, true. But OP also uses std::string, instead of std::wstring, which hints on a lack of unicode support as not important. \$\endgroup\$ – Rakete1111 Oct 7 '16 at 15:22
  • \$\begingroup\$ You wouldn't store UTF-8 data in std::wstring. \$\endgroup\$ – Rhymoid Oct 7 '16 at 15:40
  • \$\begingroup\$ @Rhymoid Um, yes you would \$\endgroup\$ – Rakete1111 Oct 7 '16 at 16:11
  • 1
    \$\begingroup\$ Why? UTF-8 is a variable length encoding based on units of 8 bits, so wchar_t units would be a waste. Besides that, wchar_t is insufficient to capture a whole codepoint (up to 21 bits). It's useless except for UTF-16 (which has no legitimate reason to exist, IMO). \$\endgroup\$ – Rhymoid Oct 7 '16 at 18:05
4
\$\begingroup\$
  1. Do not use namespace std;

  2. Put your code into functions if possible. For example the Question for the user should be a single function.

    void printOptions() {
         std::cout << "Your choices of drinks are: " << "\n";
         std::cout << "1 - Coke" << "\n";
         std::cout << "2 - Sprite" << "\n";
         std::cout << "3 - Water" << "\n";
         std::cout << "4 - Orange Juice" << "\n";
         std::cout << "5 - Apple Juice" << "\n";
         std::cout << "What drink would you like (1/5): ";
    

    }

    Note that i use "\n" instead of std::endl as this doesnt flush the stream.

  3. Use a dedicated input function

    int chooseBeverage() {
        int beverage;
        std::cin >> beverage;
        return beverage;
    }
    
  4. Now define a function that takes the choice and give the beverage.

    enum choice {
        NONE = 0;
        COKE,
        SPRITE,
        WATER,
        ORANGE_JUICE,
        APPLE_JUICE
    };
    
    void serveBeverageIf(choice beverage) {
         if(beverage == COKE)
            std::cout << "You chose Coke." << "\n";
         else if(beverage == SPRITE)
            std::cout << "You chose Sprite." << "\n";
         else if(beverage == WATER)
            std::cout << "You chose Water." << "\n";
         else if(beverage == ORANGE_JUICE)
            std::cout << "You chose Orange Juice." << "\n";
         else if(beverage == APPLE_JUICE)
            std::cout << "You chose Apple juice." << "\n";
         else
            std::cout << "Error. choice was not valid, here is your money back.";
     }
    
     void serveBeverageSwitch(choice beverage) {
         switch(beverage)
         case COKE:
            std::cout << "You chose Coke." << "\n";
            break;
         case SPRITE:
            std::cout << "You chose Sprite." << "\n";
            break;
         case WATER:                
            std::cout << "You chose Water." << "\n";
            break;
         case ORANGE_JUICE:                
            std::cout << "You chose Orange Juice." << "\n";
            break;
         case APPLE_JUICE:                
            std::cout << "You chose Apple juice." << "\n";
            break;
         default:
            std::cout << "Error. choice was not valid, here is your money back.";
            break;
         }
     }
    
  5. What you could also do is use a third option utilizing a map.

    void serveBeverageMap(choice beverage) {
    map<choice, std::string> nameMap = {
        std::make_pair(COKE, "Coke"),
        std::make_pair(SPRITE, "Sprite"),
        std::make_pair(WATER, "Water"),
        std::make_pair(ORANGE_JUICE, "Orange juice"),
        std::make_pair(APPLE_JUICE, "Apple juice")            
    };
    if(nameMap.find(beverage) == nameMap.end()) {
       std::cout << "Error. choice was not valid, here is your money back.";
    } else { 
        std::cout << You chose " << nameMap[beverage] << ".";
    }
    }
    
\$\endgroup\$
  • \$\begingroup\$ Don't forget the case if I input SwItCh :) \$\endgroup\$ – Rakete1111 Oct 7 '16 at 12:26
  • \$\begingroup\$ Thanks for the help, i have not yet learnt what a map is, could i possibly get an example and how i could benefit from it? \$\endgroup\$ – Taylor Robinson-Powell Oct 7 '16 at 13:14
  • \$\begingroup\$ I would say a map is more concise than the other two approaches. The three functions above are equivalent which should serve as a simple usecase. \$\endgroup\$ – miscco Oct 7 '16 at 13:31
1
\$\begingroup\$

Use curly braces {} in an if/if-else clause, even if the statements are just one lines.

This makes it easier and more failproof to extend a block, preventing bugs such as the infamous goto fail SSL bug.

Depending on your preference for indentation and bracketing, this doesn't even have to lead to extra lines (well, just one, at the end). For example:

if (drink == 1) {
    cout << "You chose Coke." << endl;
} else if (drink == 2) {
    cout << "You chose Sprite." << endl;
} else if (drink == 3) {
    cout << "You chose Water." << endl;
} else if (drink == 4) {
    cout << "You chose Orange Juice." << endl;
} else if (drink == 5) {
    cout << "You chose Apple juice." << endl;
} else {
    cout << "Error. Choice was not valid, Here is your money back.";        
}
\$\endgroup\$
0
\$\begingroup\$

Here's a way to abuse the instructions to avoid having to write two versions, by using neither switch nor cascaded if:

#include <cstdlib>
#include <iostream>
#include <string>
#include <vector>

static const std::vector<std::string> drinks = {
    "Coke",
    "Sprite",
    "Water",
    "Orange Juice",
    "Apple Juice"
};

void print_drinks(auto& o, std::vector<auto> drinks)
{
    int i = 0;
    for (auto drink: drinks)
        o << ++i << " - " << drink << "\n";
    o.flush();
}

int main()
{
    print_drinks(std::cout, drinks);

    std::cout << "What drink would you like? (1-" << drinks.size() << ")" << std::endl;
    size_t n;
    if (!(std::cin >> n) || !n || n > drinks.size()) {
        std::cerr << "Error. Choice was not valid, Here is your money back." << std::endl;
        return EXIT_FAILURE;
    }

    // We now have a valid selection
    const std::string drink = drinks[n-1];
    std::cout << "You chose " << drink << "." << std::endl;

    // Implicit EXIT_SUCCESS
}

I don't suggest you actually submit this for your homework, but you may learn something from studying it.

  • Instead of repeating the names of the drinks everywhere they are needed, we store them in one place. This ensures consistency between the menu that's printed and the output after selecting (e.g. in your code, you have both Apple Juice and Apple juice). It also makes it possible to add or remove drinks without changing any of the 'logic' part of the code.
  • I've taken out the surrounding loop, according to the philosophy of "do one thing well". You can easily use a shell to repeatedly execute the program:

    while ./select_drink; do :; done
    

    or

    while true; do ./select_drink; done
    

    or

    do ./select_drink;
    while read -p "Would you like to go again? [y/N] " -N 1 again \
        && [[ $again =~ [yY] ]]
    

    (this last, in Bash 3 or later, is most like your code).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.