Conversation
mooreaarond
left a comment
mooreaarond
left a comment
There was a problem hiding this comment.
I've added a few notes here to get you to think about things. One big thing I would call out is that you should always validate input to public methods. You never know how someone is going to use these methods, so you should always make sure they give you valid values. I also would want you to think a little more about how you interact with a vending machine as I don't think your main method here accurately reflects that
| { | ||
| public: | ||
| int coins_5; | ||
| int coins_10; |
There was a problem hiding this comment.
These probably shouldn't be public, so that someone can't just arbitrarily increase the values or set them to negative numbers
| currency() | ||
| { | ||
| coins_5=100; | ||
| coins_10=50; |
There was a problem hiding this comment.
These probably shouldn't be hard coded values
| coins_10=50; | ||
| } | ||
| }; | ||
| int id=0; |
There was a problem hiding this comment.
This id shouldn't be a global value, maybe a member of the vending machine class?
| class Product | ||
| { | ||
| int p_id; | ||
| float p_price; |
There was a problem hiding this comment.
You have to be careful using float for values representing money because of rounding issues. The classic example is that 0.1 can't be exactly represented by floating point numbers.
http://www.exploringbinary.com/why-0-point-1-does-not-exist-in-floating-point/
| int p_id; | ||
| float p_price; | ||
| public: | ||
| int p_count; |
There was a problem hiding this comment.
this should probably be private so that people have to use your increase/decrease_p_count methods and can't change this to invalid values
There was a problem hiding this comment.
Also, does it make sense for the Product itself to know how many of them there are? Does a bag of chips know how many other chips there are in the machine?
| cout <<"Insufficient change"; | ||
| } | ||
|
|
||
| void update_VM(float amount,int item_id) |
There was a problem hiding this comment.
validate amount > 0 and item_id is a valid id before indexing into items_list
| coins_10+=cnt_10; | ||
| coins_5+=cnt_5; | ||
| } | ||
| void pay_price(float bal) |
There was a problem hiding this comment.
How do they actually get the change back if this is void?
| { | ||
| Vending_Machine VM; | ||
| cout<< "Welcome!"; | ||
| cout<<"\nChoose your option\n 1.Request the price of an item\n2. Select and buy items\n3. Cancel request"; |
There was a problem hiding this comment.
Is this how a vending machine usually works? Typically you can select an item or add money in any order and different things happen based on the case
| { | ||
| cout<<"\nEnter the Item ID"; | ||
| cin>>p_id; | ||
| cout<<"Price: "<<VM.items_list[p_id]->get_price(); |
There was a problem hiding this comment.
you should verify that p_id is a valid value before indexing into an array.
| int cnt_10,cnt_5; | ||
| cin>>cnt_10>>cnt_5; | ||
| VM.add_coins(cnt_5,cnt_10); | ||
| VM.update_VM(pay,p_id); |
There was a problem hiding this comment.
Don't return item/change to the user?
2 participants
No description provided.