Café in C++ program
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
25
down vote
favorite
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;
c++ beginner
add a comment |Â
up vote
25
down vote
favorite
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;
c++ beginner
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.
â Xam
Feb 13 at 6:02
add a comment |Â
up vote
25
down vote
favorite
up vote
25
down vote
favorite
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;
c++ beginner
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;
c++ beginner
edited Feb 5 at 12:17
Richard the Spacecat
1033
1033
asked Feb 5 at 3:58
lucky girl
12623
12623
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.
â Xam
Feb 13 at 6:02
add a comment |Â
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.
â Xam
Feb 13 at 6:02
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.
â Xam
Feb 13 at 6:02
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.
â Xam
Feb 13 at 6:02
add a comment |Â
4 Answers
4
active
oldest
votes
up vote
55
down vote
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:
- We say the name of the product and the price
- We ask whether they want to buy the product
- We add the item to the order and increase the total (this step is missing)
- We tell the customer their new total
- If we still have products, we select the next product and continue with 1.
- 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 collectionstd::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.
17
A note ontypedef
- be very pragmatic about its use. The semantics fordouble
are generally well understood. If I see adouble
- I know immediately what's going on and which operators are available. When I see aPrice
I'll have to read documentation to find out what's up, or worse I might make assumptions, because surely it's anunsigned long long
? This gets more magnified by a (e.g.)typedef
ofstd::vector<Price>
that is calledPrices
.
â Gerard
Feb 5 at 10:11
8
"Unless you know what you're doing don't useusing namespace std
" I'd just say not to use it at all.
â Baldrickk
Feb 5 at 14:48
2
@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.
â Filip Minx
Feb 5 at 15:29
2
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
â Brian J
Feb 5 at 16:49
1
@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."
â Zeta
Feb 6 at 6:40
 |Â
show 4 more comments
up vote
22
down vote
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 InventoryItem
s. 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 iterator
s 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!
4
std::pair<>
is a particularly bad class is it offers no useful information about what it contains. I avoid it and uses oftuple
as much as possible because it makes code unreadable.
â user1118321
Feb 5 at 6:26
7
Let me put that in another way (still need coffee): is there any advantage forInventoryItem
being aclass
withprivate
fields instead of being astruct
with proper named fields?
â Zeta
Feb 5 at 6:30
2
Mainly that this was written for a beginner and I think it's a bad habit to get into making most fieldspublic
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.
â user1118321
Feb 5 at 6:35
6
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.
â Nic Hartley
Feb 5 at 7:53
8
I'd suggest usingstd::vector
instead ofstd::array
. This way, if the items change in the future, there's no need to change the declaration. The fact that there are10
items is not intrinsic to the program.
â Mark H
Feb 5 at 8:06
 |Â
show 5 more comments
up vote
18
down vote
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.
2
Small remark: this is essentially the reason why I usedtypedef double Price
immediately in my review. One can change it to a proper type for financial calculations easily.
â Zeta
Feb 5 at 9:48
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
2
@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.
â Toby Speight
Feb 5 at 13:50
1
@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.
â Andreas H.
Feb 5 at 14:34
1
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
 |Â
show 5 more comments
up vote
1
down vote
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()
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.
add a comment |Â
4 Answers
4
active
oldest
votes
4 Answers
4
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
55
down vote
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:
- We say the name of the product and the price
- We ask whether they want to buy the product
- We add the item to the order and increase the total (this step is missing)
- We tell the customer their new total
- If we still have products, we select the next product and continue with 1.
- 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 collectionstd::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.
17
A note ontypedef
- be very pragmatic about its use. The semantics fordouble
are generally well understood. If I see adouble
- I know immediately what's going on and which operators are available. When I see aPrice
I'll have to read documentation to find out what's up, or worse I might make assumptions, because surely it's anunsigned long long
? This gets more magnified by a (e.g.)typedef
ofstd::vector<Price>
that is calledPrices
.
â Gerard
Feb 5 at 10:11
8
"Unless you know what you're doing don't useusing namespace std
" I'd just say not to use it at all.
â Baldrickk
Feb 5 at 14:48
2
@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.
â Filip Minx
Feb 5 at 15:29
2
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
â Brian J
Feb 5 at 16:49
1
@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."
â Zeta
Feb 6 at 6:40
 |Â
show 4 more comments
up vote
55
down vote
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:
- We say the name of the product and the price
- We ask whether they want to buy the product
- We add the item to the order and increase the total (this step is missing)
- We tell the customer their new total
- If we still have products, we select the next product and continue with 1.
- 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 collectionstd::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.
17
A note ontypedef
- be very pragmatic about its use. The semantics fordouble
are generally well understood. If I see adouble
- I know immediately what's going on and which operators are available. When I see aPrice
I'll have to read documentation to find out what's up, or worse I might make assumptions, because surely it's anunsigned long long
? This gets more magnified by a (e.g.)typedef
ofstd::vector<Price>
that is calledPrices
.
â Gerard
Feb 5 at 10:11
8
"Unless you know what you're doing don't useusing namespace std
" I'd just say not to use it at all.
â Baldrickk
Feb 5 at 14:48
2
@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.
â Filip Minx
Feb 5 at 15:29
2
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
â Brian J
Feb 5 at 16:49
1
@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."
â Zeta
Feb 6 at 6:40
 |Â
show 4 more comments
up vote
55
down vote
up vote
55
down vote
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:
- We say the name of the product and the price
- We ask whether they want to buy the product
- We add the item to the order and increase the total (this step is missing)
- We tell the customer their new total
- If we still have products, we select the next product and continue with 1.
- 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 collectionstd::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.
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:
- We say the name of the product and the price
- We ask whether they want to buy the product
- We add the item to the order and increase the total (this step is missing)
- We tell the customer their new total
- If we still have products, we select the next product and continue with 1.
- 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 collectionstd::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.
edited Feb 6 at 8:40
answered Feb 5 at 6:47
Zeta
14.3k23267
14.3k23267
17
A note ontypedef
- be very pragmatic about its use. The semantics fordouble
are generally well understood. If I see adouble
- I know immediately what's going on and which operators are available. When I see aPrice
I'll have to read documentation to find out what's up, or worse I might make assumptions, because surely it's anunsigned long long
? This gets more magnified by a (e.g.)typedef
ofstd::vector<Price>
that is calledPrices
.
â Gerard
Feb 5 at 10:11
8
"Unless you know what you're doing don't useusing namespace std
" I'd just say not to use it at all.
â Baldrickk
Feb 5 at 14:48
2
@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.
â Filip Minx
Feb 5 at 15:29
2
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
â Brian J
Feb 5 at 16:49
1
@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."
â Zeta
Feb 6 at 6:40
 |Â
show 4 more comments
17
A note ontypedef
- be very pragmatic about its use. The semantics fordouble
are generally well understood. If I see adouble
- I know immediately what's going on and which operators are available. When I see aPrice
I'll have to read documentation to find out what's up, or worse I might make assumptions, because surely it's anunsigned long long
? This gets more magnified by a (e.g.)typedef
ofstd::vector<Price>
that is calledPrices
.
â Gerard
Feb 5 at 10:11
8
"Unless you know what you're doing don't useusing namespace std
" I'd just say not to use it at all.
â Baldrickk
Feb 5 at 14:48
2
@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.
â Filip Minx
Feb 5 at 15:29
2
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
â Brian J
Feb 5 at 16:49
1
@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."
â Zeta
Feb 6 at 6:40
17
17
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
.â Gerard
Feb 5 at 10:11
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
.â Gerard
Feb 5 at 10:11
8
8
"Unless you know what you're doing don't use
using namespace std
" I'd just say not to use it at all.â Baldrickk
Feb 5 at 14:48
"Unless you know what you're doing don't use
using namespace std
" I'd just say not to use it at all.â Baldrickk
Feb 5 at 14:48
2
2
@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.â Filip Minx
Feb 5 at 15:29
@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.â Filip Minx
Feb 5 at 15:29
2
2
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â Brian J
Feb 5 at 16:49
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â Brian J
Feb 5 at 16:49
1
1
@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."
â Zeta
Feb 6 at 6:40
@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."
â Zeta
Feb 6 at 6:40
 |Â
show 4 more comments
up vote
22
down vote
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 InventoryItem
s. 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 iterator
s 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!
4
std::pair<>
is a particularly bad class is it offers no useful information about what it contains. I avoid it and uses oftuple
as much as possible because it makes code unreadable.
â user1118321
Feb 5 at 6:26
7
Let me put that in another way (still need coffee): is there any advantage forInventoryItem
being aclass
withprivate
fields instead of being astruct
with proper named fields?
â Zeta
Feb 5 at 6:30
2
Mainly that this was written for a beginner and I think it's a bad habit to get into making most fieldspublic
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.
â user1118321
Feb 5 at 6:35
6
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.
â Nic Hartley
Feb 5 at 7:53
8
I'd suggest usingstd::vector
instead ofstd::array
. This way, if the items change in the future, there's no need to change the declaration. The fact that there are10
items is not intrinsic to the program.
â Mark H
Feb 5 at 8:06
 |Â
show 5 more comments
up vote
22
down vote
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 InventoryItem
s. 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 iterator
s 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!
4
std::pair<>
is a particularly bad class is it offers no useful information about what it contains. I avoid it and uses oftuple
as much as possible because it makes code unreadable.
â user1118321
Feb 5 at 6:26
7
Let me put that in another way (still need coffee): is there any advantage forInventoryItem
being aclass
withprivate
fields instead of being astruct
with proper named fields?
â Zeta
Feb 5 at 6:30
2
Mainly that this was written for a beginner and I think it's a bad habit to get into making most fieldspublic
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.
â user1118321
Feb 5 at 6:35
6
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.
â Nic Hartley
Feb 5 at 7:53
8
I'd suggest usingstd::vector
instead ofstd::array
. This way, if the items change in the future, there's no need to change the declaration. The fact that there are10
items is not intrinsic to the program.
â Mark H
Feb 5 at 8:06
 |Â
show 5 more comments
up vote
22
down vote
up vote
22
down vote
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 InventoryItem
s. 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 iterator
s 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!
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 InventoryItem
s. 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 iterator
s 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!
answered Feb 5 at 4:49
user1118321
10.2k11144
10.2k11144
4
std::pair<>
is a particularly bad class is it offers no useful information about what it contains. I avoid it and uses oftuple
as much as possible because it makes code unreadable.
â user1118321
Feb 5 at 6:26
7
Let me put that in another way (still need coffee): is there any advantage forInventoryItem
being aclass
withprivate
fields instead of being astruct
with proper named fields?
â Zeta
Feb 5 at 6:30
2
Mainly that this was written for a beginner and I think it's a bad habit to get into making most fieldspublic
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.
â user1118321
Feb 5 at 6:35
6
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.
â Nic Hartley
Feb 5 at 7:53
8
I'd suggest usingstd::vector
instead ofstd::array
. This way, if the items change in the future, there's no need to change the declaration. The fact that there are10
items is not intrinsic to the program.
â Mark H
Feb 5 at 8:06
 |Â
show 5 more comments
4
std::pair<>
is a particularly bad class is it offers no useful information about what it contains. I avoid it and uses oftuple
as much as possible because it makes code unreadable.
â user1118321
Feb 5 at 6:26
7
Let me put that in another way (still need coffee): is there any advantage forInventoryItem
being aclass
withprivate
fields instead of being astruct
with proper named fields?
â Zeta
Feb 5 at 6:30
2
Mainly that this was written for a beginner and I think it's a bad habit to get into making most fieldspublic
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.
â user1118321
Feb 5 at 6:35
6
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.
â Nic Hartley
Feb 5 at 7:53
8
I'd suggest usingstd::vector
instead ofstd::array
. This way, if the items change in the future, there's no need to change the declaration. The fact that there are10
items is not intrinsic to the program.
â Mark H
Feb 5 at 8:06
4
4
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.â user1118321
Feb 5 at 6:26
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.â user1118321
Feb 5 at 6:26
7
7
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?â Zeta
Feb 5 at 6:30
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?â Zeta
Feb 5 at 6:30
2
2
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.â user1118321
Feb 5 at 6:35
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.â user1118321
Feb 5 at 6:35
6
6
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.â Nic Hartley
Feb 5 at 7:53
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.â Nic Hartley
Feb 5 at 7:53
8
8
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.â Mark H
Feb 5 at 8:06
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.â Mark H
Feb 5 at 8:06
 |Â
show 5 more comments
up vote
18
down vote
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.
2
Small remark: this is essentially the reason why I usedtypedef double Price
immediately in my review. One can change it to a proper type for financial calculations easily.
â Zeta
Feb 5 at 9:48
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
2
@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.
â Toby Speight
Feb 5 at 13:50
1
@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.
â Andreas H.
Feb 5 at 14:34
1
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
 |Â
show 5 more comments
up vote
18
down vote
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.
2
Small remark: this is essentially the reason why I usedtypedef double Price
immediately in my review. One can change it to a proper type for financial calculations easily.
â Zeta
Feb 5 at 9:48
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
2
@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.
â Toby Speight
Feb 5 at 13:50
1
@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.
â Andreas H.
Feb 5 at 14:34
1
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
 |Â
show 5 more comments
up vote
18
down vote
up vote
18
down vote
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.
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.
answered Feb 5 at 9:40
Andreas H.
57635
57635
2
Small remark: this is essentially the reason why I usedtypedef double Price
immediately in my review. One can change it to a proper type for financial calculations easily.
â Zeta
Feb 5 at 9:48
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
2
@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.
â Toby Speight
Feb 5 at 13:50
1
@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.
â Andreas H.
Feb 5 at 14:34
1
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
 |Â
show 5 more comments
2
Small remark: this is essentially the reason why I usedtypedef double Price
immediately in my review. One can change it to a proper type for financial calculations easily.
â Zeta
Feb 5 at 9:48
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
2
@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.
â Toby Speight
Feb 5 at 13:50
1
@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.
â Andreas H.
Feb 5 at 14:34
1
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
2
2
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.â Zeta
Feb 5 at 9:48
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.â Zeta
Feb 5 at 9:48
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
âÂÂ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.
â Konrad Rudolph
Feb 5 at 12:07
2
2
@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.
â Toby Speight
Feb 5 at 13:50
@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.
â Toby Speight
Feb 5 at 13:50
1
1
@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.
â Andreas H.
Feb 5 at 14:34
@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.
â Andreas H.
Feb 5 at 14:34
1
1
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
@MartinYork 0.01 is 1 / 100, which isnâÂÂt a sum of powers of 2 (or am I daft?)
â Konrad Rudolph
Feb 5 at 19:49
 |Â
show 5 more comments
up vote
1
down vote
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()
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.
add a comment |Â
up vote
1
down vote
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()
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.
add a comment |Â
up vote
1
down vote
up vote
1
down vote
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()
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.
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()
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.
answered Feb 8 at 19:39
mreff555
1662
1662
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186781%2fcaf%25c3%25a9-in-c-program%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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.
â Xam
Feb 13 at 6:02