26
\$\begingroup\$

I want to build a cafe in C++. I want the user to answer each question with a yes or no. If they answer yes or no, I want to display their total price.

#include <iostream>
using namespace std;
int main()
{
    //Develop a billing statement for a store/restaurant 
    // Define variables 
    double donut, bagel, burrito, sandwhich, omelet, coffee, cappachino, smootie, water, spirit, total, tax;
    char answerType;
    bagel = 2.50;
    burrito = 3.50;
    donut = 1.00;
    sandwhich = 3.50;
    omelet = 1.25;
    coffee = 1.50;
    cappachino = 2.00;
    smootie = 3.25;
    water = 0.99;
    spirit = 1.00;
    total = 0.00;
    tax = 6.25;
    cout << "Welcome to Junelle's Cafe" << endl;
    // Introduction to my cafe

    //Customer knows the options before hand 
    //Customer enters yes (y) or no (n) 
    cout << "Enter a y (yes) or no (n) for every question asked." << endl;

//Ask user if they want a bagel 
    cout << "Would you like to buy a bagel for $2.50?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A bagel has been added to your order. Your total is " << total + bagel << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item2
    cout << "Would you like to buy a donut for $1.00?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A donut has been added to your order. Your total is " << total + donut+bagel << endl;

    }


    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item3 
    cout << "Would you like to buy a omelet for $1.25?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A omelet has been added to your order. Your total is " << total + donut + bagel+ omelet << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item 4
    cout << "Would you like to buy a burrito for $2.50?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A burrito has been added to your order. Your total is " << total + donut + bagel
            + omelet + burrito << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }

//item 5 
    cout << "Would you like to buy a sandwhich for $2.50?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A sandwhich has been added to your order. Your total is " << total + donut + bagel
            + omelet+ burrito+sandwhich << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item 6
    cout << "Would you like to buy a coffee for $1.50?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A coffee has been added to your order. Your total is " << total + donut + bagel 
            + omelet+ burrito + sandwhich + coffee<< endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }

//item 7 
    cout << "Would you like to buy a cappachino for $2.00?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A cappachino has been added to your order. Your total is " << total + donut + bagel 
            + omelet + burrito + sandwhich + coffee+ cappachino << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item 8
    cout << "Would you like to buy a water for $0.99?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A water has been added to your order. Your total is " << total + donut + bagel
            + omelet + burrito + sandwhich + coffee + cappachino + water << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item 9 
    cout << "Would you like to buy a spirit for $1.00?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A spirit has been added to your order. Your total is " << total + donut + bagel
            + omelet + burrito + sandwhich + coffee + cappachino + water + spirit<< endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }
//item 10 
    cout << "Would you like to buy a smootie for $1.25?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A smootie has been added to your order. Your total is " << total + donut + bagel 
            + omelet + burrito + sandwhich + coffee + cappachino + water + spirit + smootie << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }

    cout << "Thank you for visiting Junelle's Cafe" << endl;
    while (1);
    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Oh I remember when my C++ code was like this few months ago. Keep it practising and then you'll write much better code. Regards. \$\endgroup\$ – Xam Feb 13 '18 at 6:02
55
\$\begingroup\$

Don't. Repeat. Yourself.

That's an essential principle. You repeat the "do you want XY" cycle over and over manually. That's not only error prone, but also hard to maintain. For example, let's say you want to ask the customer how many items they want. Then you would have to add or change your input handling for every single item.

Maybe you want to use

 15 bagels
200 coffee
  1 water
  1 donut

It would get rather tedious to do that in your current program. We would need int bagelCount, coffeeCount, waterCount, coconoutCount…, you get the pattern.

So instead, we should try to handle all the items the same way in a single block of code, if possible. Which brings us to the next topic.

Reuse information

At the begin of your program, you have bagel = 2.50. However, you don't use that information to your advantage when you ask the customer whether they want to buy a bagel:

cout << "Would you like to buy a bagel for $2.50?:";
                                           ^^^^^^

That's again a DRY violation. If you change the bagel price, you have to change it at two places. That's error prone. Instead, reuse the information you already have at hand:

std::cout << "Would you like to buy a bagel for $" << bagel << ": ";

By the way, if a variable should never change its value, declare it const. Also, try to initialize your variables, with a value, whenever possible:

typedef double Price;

const Price bagelPrice = 2.50;
const Price coffeePrice = 1.50;

But we will change that later, so don't use that yet.

Don't trust the customer

What happens if the customer inputs 'a' instead of 'y' or 'n'? Well, you accept it and don't add anything.

However, if you asked a customer "Do you want a bagel?" and they answer "Cthulhu!", you'd probably ask them to either accept your offer or decline. A small

bool ask_yes_no() {
    /* exercise */
}

function can help you.

Namespaces

Unless you know what you're doing don't use using namespace std.

Abstractions

Now let's have a look at one item exchange

//Ask user if they want a bagel 
    cout << "Would you like to buy a bagel for $2.50?:";
    cin >> answerType;

    if (answerType == 'y')

    {

        cout << "A bagel has been added to your order. Your total is " << total + bagel << endl;

    }

    else if (answerType == 'n')

    {
        cout << "Your total is " << total << endl;

    }

All other items follow the same structure, so lets focus on this one. We see a pattern here:

  1. We say the name of the product and the price
  2. We ask whether they want to buy the product
  3. We add the item to the order and increase the total (this step is missing)
  4. We tell the customer their new total
  5. If we still have products, we select the next product and continue with 1.
  6. Otherwise we tell the customer the total.

Therefore, let us rewrite that dialog:

std::cout << "Would you like to buy a " << item_name
          << " for $" << item_price << ": ";

if(asks_yes_no()) { // see exercise above
    total += item_price;

    std::cout << "A " << item_name<< " has been added to your order. ";
}

std::cout << "Your total is $" << total << ".";

Now all we need is a way to get item_name and item_price and traverse all items. There are several ways:

  • we can use an associative collection, e.g. std::map<Name, Price>
  • we can couple the name and the price in a single Item and use a normal collection std::vector<Item>

The other review uses the latter, therefore let us have a look at the former, a std::map:

typedef std::string ItemName
typedef double Price;

const std::map<ItemName, Price> menu {
    {"bagel",  2.50 },
    {"coffee", 1.50 },
    {"sugar", 0.02 },
    {"full breakfast", 5.00}
};

We can now traverse all items in the menu easily:

// You might not know this kind of loop yet.
// It's basically going through the menu.
// The current pair of item and price is item_and_price,
// the .first-Member is the item, and the .second-Member
// is the price.
//
// The following holds throughout the loop:
//
//    menu[item_and_price.first] == item_and_price.second
for(const auto & item_and_price : menu) {
    const auto & item_name = item_and_price.first;
    const auto & item_price = item_and_price.second;

    std::cout << "Would you like to buy a " << item_name 
              << " for $" << item_price << ": ";

    if(asks_yes_no()) { // see exercise above
        total += item_price;

        std::cout << "A " << item_name << " has been added to your order. ";
    }

    std::cout << "Your total is $" << total << ".";
}

We can also keep tracks of the order:

typedef unsigned int Amount;

std::map<ItemName, Amount> order;

/* ... */

total += item_price;

order[item_name] = 1;

Small remark on the floating point values

Floating point arithmetic isn't exact. Instead of double, we should use an exact type and cents instead of a floating point type and dollars, e.g.

const Cents bagelPrice = 250;

For a small toy program, double is fine. But if you ever handle real money in your code, make sure that neither the customer, you, or the government pay too much or don't get enough money.

Usability

Last, but not least, you can show the menu to the user and have them pick the item they want to buy, e.g.:

Welcome to our shop. We have the following products at hand:

 1) Bagel     ($2.50)
 2) Coffee    ($1.50)
 3) Espresso  ($1.50)
 4) Breakfast ($5.20)
 5) Checkout

Your choice? [1-5]: 1

Amount of bagels: 20

Your order of 20 bagels has been registered. Your new total is $50.00.

---- ---- ----

 1) Bagel     ($2.50)
 2) Coffee    ($1.50)
 3) Espresso  ($1.50)
 4) Breakfast ($5.20)
 5) Checkout

You ordered 20 bagels. Your next choice [1-5]: 5

Your total is $50.00. Thanks for visiting and come again!

That way they don't have to go through all the products if they just want to buy 20 bagels.

\$\endgroup\$
  • 17
    \$\begingroup\$ A note on typedef - be very pragmatic about its use. The semantics for double are generally well understood. If I see a double - I know immediately what's going on and which operators are available. When I see a Price I'll have to read documentation to find out what's up, or worse I might make assumptions, because surely it's an unsigned long long? This gets more magnified by a (e.g.) typedef of std::vector<Price> that is called Prices. \$\endgroup\$ – Gerard Feb 5 '18 at 10:11
  • 8
    \$\begingroup\$ "Unless you know what you're doing don't use using namespace std" I'd just say not to use it at all. \$\endgroup\$ – Baldrickk Feb 5 '18 at 14:48
  • 2
    \$\begingroup\$ @Gerard Another sensible way would be to name the variable in a more descriptive manner. double price_per_hour would have the same information without the need to alias the type. \$\endgroup\$ – Filip Minx Feb 5 '18 at 15:29
  • 2
    \$\begingroup\$ Beware using floating point numbers as money values because of rounding or precision errors. double is probably one of your best options for c++, but there might be some caveats to be aware of: stackoverflow.com/q/149033/1474939 \$\endgroup\$ – Brian J Feb 5 '18 at 16:49
  • 1
    \$\begingroup\$ @Baldrickk I'm following the C++ Core Guidelines in that regard. "people who use using namespace std are supposed to know about std and about this risk." \$\endgroup\$ – Zeta Feb 6 '18 at 6:40
22
\$\begingroup\$

This is a very straightforward way to implement this, and it's very easy to read. That's great! But it might have occurred to you that there's an easier way. Here are some thoughts on simplifying it.

Use Classes

Notice how you have several items that all have the same properties. They all have a name and a price. So it would make sense to combine those into a class. A simple class might be something like this:

class InventoryItem {
    public:
        InventoryItem (const std::string& itemName, const double itemPrice);
        ~InventoryItem();

        std::string getName() const;
        double getPrice() const;

    private:
        std::string name;
        double price;
};

The constructor of the class would simply set the name field to the passed in item name, and the price field to the passed in item price:

InventoryItem (const std::string& itemName, const double itemPrice) : 
    name (itemName), 
    price(itemPrice) 
{
}

The get functions would just return the values:

std::string getName() const
{
    return name;
}

Once you have a class for this, you can create objects by doing something like this:

InventoryItem newItem("burrito", 3.50);

Use Arrays

Since you have several items and you want to do similar operations on them, you can put them into an array. An array holds multiple items of the same type. So in this case, we'd want an array of InventoryItems. We can create one like this:

#include <array>

int main()
{
    std::array<InventoryItem, 10>   inventory {
        InventoryItem("bagel", 2.50),
        InventoryItem("burrito",3.50),
        InventoryItem("donut", 1.00),
        InventoryItem("sandwhich", 3.50),
        InventoryItem("omelet", 1.25),
        InventoryItem("coffee", 1.50),
        InventoryItem("cappachino", 2.00),
        InventoryItem("smootie", 3.25),
        InventoryItem("water", 0.99),
        InventoryItem("spirit", 1.00)
    };

}

When you have items in an array and you want to do the same thing to each item, you can iterate over them in a loop. An array is a type of "container" because it holds other objects. All containers supplied by the standard template library (that's the std:: prefix stuff) supply objects called iterators that allow you to step through each item in the container. You simply call container.begin() to get the iterator that points to the first item in the container, then increment it to get the next one. You can keep incrementing it until it is equal to container.end(), and then you know you don't have any more to process. So let's try that with the above array of inventory items:

for (std::array<InventoryItem, 10>::iterator nextItem = inventory.begin();
     nextItem != inventory.end();
     nextItem++)
{
    std::cout << "Would you like to buy a " << nextItem->getName();
    std::cout << " for " << nextItem->getPrice() << "?\n";

    std::cin >> answerType;

    if (answerType == 'y')
    {
        std::cout << "A " << nextItem->getName();
        std::cout << " has been added to your order. Your total is ";
        std::cout << total << "\n";
    }
    else
    {
        std::cout << "Your total is " << total << "\n";
    }
}

Now notice something odd about the total? It's not getting updated properly. I didn't add the cost of the item into the total. The reason I didn't is because I want you to look at how you did it in your version. In your first if statement, if the user chooses "y", you print out total + bagel. But notice that you didn't actually update total, you only printed it. You need to actually assign the new price to total. So you need to do something like this:

total = total + bagel;

Or in my version with the array and loop, it would just be:

total = total + nextItem->getPrice();

Notice also that in your version, for the second if statement, when the user chooses "y" you print the price of a bagel and a donut. But what if the user chose "n" for bagel and "y" for donut? You'll print the wrong price!

So if you keep track of the total as you go, and update it after every "y" choice, then it should always have the right value, and you won't need to manually add in all the other choices.

And you can end it just as you end your version:

cout << "Thank you for visiting Junelle's Cafe" << endl;
while (1);
return 0;

Thanks for letting us drop in!

\$\endgroup\$
  • 4
    \$\begingroup\$ std::pair<> is a particularly bad class is it offers no useful information about what it contains. I avoid it and uses of tuple as much as possible because it makes code unreadable. \$\endgroup\$ – user1118321 Feb 5 '18 at 6:26
  • 7
    \$\begingroup\$ Let me put that in another way (still need coffee): is there any advantage for InventoryItem being a class with private fields instead of being a struct with proper named fields? \$\endgroup\$ – Zeta Feb 5 '18 at 6:30
  • 2
    \$\begingroup\$ Mainly that this was written for a beginner and I think it's a bad habit to get into making most fields public by default. It encourages writing hard-to-debug code. It's also likely that a simple class like this will grow to include more fields in the future. I mainly intended it to be a basic example of what a real class is likely to look like. \$\endgroup\$ – user1118321 Feb 5 '18 at 6:35
  • 6
    \$\begingroup\$ I'd argue that you don't really need classes -- std::map<std::string, float> itemPrices is just as meaningful, faster to do lookups in, and requires less code to use. It's also more than sufficient for now; if more features (e.g. item-specific discounts) are required, you can always refactor the code. \$\endgroup\$ – Fund Monica's Lawsuit Feb 5 '18 at 7:53
  • 8
    \$\begingroup\$ I'd suggest using std::vector instead of std::array. This way, if the items change in the future, there's no need to change the declaration. The fact that there are 10 items is not intrinsic to the program. \$\endgroup\$ – Mark H Feb 5 '18 at 8:06
19
\$\begingroup\$

In addition to the answers above

Never ever use floating point in financial calculations

This is not good as floating point additions are not exact and round-off error propagates in a non-intuitive way. It is very likely that one looses or gains 1 cent when doing more complex calculations.

It may work for your simple example but notice that accounting systems usually work with a fixed point representation.

Since C++ does not support fixed point the simplest workaround is to use integers and store all prices in cent.

\$\endgroup\$
  • 2
    \$\begingroup\$ Small remark: this is essentially the reason why I used typedef double Price immediately in my review. One can change it to a proper type for financial calculations easily. \$\endgroup\$ – Zeta Feb 5 '18 at 9:48
  • \$\begingroup\$ “Never ever use floating point in financial calculations” — I’ve heard this advice a lot, and used to dispense it myself. But then I’ve also heard that banks do use floating points internally to represent currency since there are never cases where the lack of precision spills over into whole pennies/cents/…. So there are (plausible) claims that this isn’t a problem in practice. \$\endgroup\$ – Konrad Rudolph Feb 5 '18 at 12:07
  • 2
    \$\begingroup\$ @Konrad - I think the full form of the advice (like most programming advice) ends with "unless you understand and accept all the consequences". IME, financial software authors know (or should know) when to use exact (fixed-point) arithmetic (such as adding prices for transactions) and when to use floating point (e.g. when computing interest payments) and (critically) when to convert between them. And nothing says that you can't use your cent/penny/전/分/øre/kopek/... as the unit for your floating-point, so it's not quite the dichotomy it appears to be. \$\endgroup\$ – Toby Speight Feb 5 '18 at 13:50
  • 1
    \$\begingroup\$ @TobySpeight: Perhaps the most important point of using fixed-point is that it forces the programmer to think about rounding after/of each operation. I guess this is also important even for interest calculation, adding prices and so on. Reproducibility is hard to achieve when one has a higher precision of intermediate results. \$\endgroup\$ – Andreas H. Feb 5 '18 at 14:34
  • 1
    \$\begingroup\$ @MartinYork 0.01 is 1 / 100, which isn’t a sum of powers of 2 (or am I daft?) \$\endgroup\$ – Konrad Rudolph Feb 5 '18 at 19:49
2
\$\begingroup\$

There is already some great advice but I noticed some of it used STL which you likely won't learn for a while. You can greatly simplify your program by using a class to incorporate the functionality of the cafe.

something like this:

struct Product
{
    static float total;
    static constexpr float tax = 6.25;
    char desc[16];
    float price;
    bool pitchItem(); 
}productList[10] =
{
    "bagel",2.50,
    "burrito",3.50,
    "donut", 1.00,
    "sandwich", 3.50,
    "omlet", 1.25,
    "coffee", 1.50,
    "cappachino", 2.00,
    "smoothie", 3.25,
    "water", .99,
    "spirit",1.00
};

This product struct contains all the information you need. Static members are members of the class rather than the object, so they can be used to store global or aggregate information. Methods do not need parameters in this case because they assume you are referring to the desc and price values in scope.

In this example all the sizes are static. This isn't the most efficient way to program for space saving purposes but anything more would be overkill and I'm purposely staying away from STL and dynamic memory because I'm guess you haven't learned that yet.

The pitchItem() method removes redundant typing by doing something like this:

bool Product::pitchItem()
{
    bool goodAnswer = true;
    char answer;
    std::cout << "Would you like to buy a " << desc << "?: ";
    std::cin >> answer;
    if(answer == 'y' || answer == 'Y')
    {
        total += price;
        std::cout << "A "<< desc << " has been added to your order. Your total is $"<< std::fixed << std::setprecision(2) <<total << "\n"; 
    }
    else if(answer == 'n' || answer == 'N')
    {
        std::cout << "Your total is $" << total << "\n";
    }
    else
    {
        std::cout << "Sorry, I must have misunderstood you.\n";
        goodAnswer = false;
    }
    return goodAnswer;
}

Doing something like this allows you to iterate through all of the choices with a for loop. Status of the question is returned so you can do something like this to make sure the input was valid:

    if(!productList[i].pitchItem())
    {
        i--;
    }

Hope that helps.

\$\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.