Please note this is a follow up question to Implement a Vending machine
Task:
Design a vending machine which does the following:
Accepts coins of 1, 5, 10, 25, 50 cents or notes of 1 and 2
User selections: Candy, snacks, nuts, Coke, Pepsi and soda
Allow user by canceling the request, returns the product and change,
Also I have been asked to use pointers and memory management.
Edit :
I Have tried to implement Singularity principle
Data Encapsulation & data abstraction
Also I have tried to implement the open/closed principle.
Also, regarding the money class: it is important because it might help to add more types of currency if possible. e.g.: accepting Euros and Dollars
Product class
#include <iostream>
class product
{
private :
const int MAX_STOCK = 10;
std::string *name = new std::string;
int *price = new int;
int *remaining_stock = new int;
public :
product(std::string n, int p)
{
*price = p;
*name = *name + n;
*remaining_stock = MAX_STOCK;
}
~product()
{
delete price;
delete name;
delete remaining_stock;
}
void deduct_remaining_stock()
{
*remaining_stock = *remaining_stock - 1;
}
int return_remaining_stock()
{
return *remaining_stock;
}
int return_price()
{
return *price;
}
};
Money class
class money
{
private :
int *max = new int;
int *accepted_values = new int[10];
public :
money()
{
}
money(int *a, int how_many)
{
*max = how_many;
for (int i=0; i<how_many; i++)
{
*(accepted_values + i) = a[i];
}
}
int get_max()
{
return *max;
}
int check(int value, int loop_max)
{
bool present = 0;
for(int i=0; i< loop_max; i++)
{
if (value == *(accepted_values + i))
{
present = 1;
}
}
if(!present)
{
value = 0;
}
return value;
}
~money()
{
delete max;
delete accepted_values;
}
};
Vending machine class:
class Vending_machine
{
private :
int *amount_deposited = new int;
int accepted_coins[5] = {1,5,10,25,50};
int accepted_notes[2] = {1,2};
product *candy = new product("Candy",10);
product *snack = new product("Snack",50);
product *nuts = new product("Nuts",90);
product *coke = new product("Coke",25);
product *soda = new product("Soda",45);
money *coins = new money(accepted_coins, 5);
money *notes = new money(accepted_notes, 2);
public :
Vending_machine()
{
*amount_deposited = 0;
}
Vending_machine(bool reset)
{
if (reset == true)
{
*amount_deposited = 0;
product *candy = new product("Candy",10);
product *snack = new product("Snack",50);
product *nuts = new product("Nuts",90);
product *coke = new product("Coke",25);
product *soda = new product("Soda",45);
}
}
~Vending_machine()
{
delete candy;
delete snack;
delete nuts;
delete coke;
delete soda;
delete coins;
delete notes;
delete amount_deposited;
}
int get_money(int choice,int value)
{
switch(choice)
{
case(1):
*amount_deposited = coins->check(value, coins->get_max());
break;
case(2):
*amount_deposited = notes->check(value, notes->get_max()) * 100;
break;
default:
*amount_deposited = 0;
}
return *amount_deposited;
}
int compare_availability(product *a)
{
int remaining_stock = a->return_remaining_stock();
int price = a->return_price();
if(remaining_stock !=0 && *amount_deposited >= price)
{
a->deduct_remaining_stock();
*amount_deposited -= price;
return(1);
}
else if (*amount_deposited < price)
{
return(2);
}
else if (remaining_stock == 0)
{
return(3);
}
else
{
return(4);
}
}
int get_selection(int selection)
{
switch(selection)
{
case (1):
return(compare_availability(candy));
break;
case (2):
return(compare_availability(snack));
break;
case (3):
return(compare_availability(nuts));
break;
case (4):
return(compare_availability(coke));
break;
case (5):
return(compare_availability(soda));
break;
default:
return(5);
}
}
int return_money()
{
int temp_return = *amount_deposited;
*amount_deposited = 0;
return temp_return;
}
int return_remaining_quantity(int select)
{
switch(select)
{
case(1):
return candy->return_remaining_stock();
break;
case(2):
return snack->return_remaining_stock();
break;
case(3):
return nuts->return_remaining_stock();
break;
case(4):
return coke->return_remaining_stock();
break;
case(5):
return soda->return_remaining_stock();
break;
default:
return(0);
}
}
};
A few functions for main and main
void print_remaining_quantity(Vending_machine *temp)
{
std::cout << "********************************************************************************";
std::cout << "\n\nAvailable stock :"
<< "\n\t\t1 Candy : " << temp->return_remaining_quantity(1)
<< "\n\t\t2 Snack : " << temp->return_remaining_quantity(2)
<< "\n\t\t3 Nuts : " << temp->return_remaining_quantity(3)
<< "\n\t\t4 Coke : " << temp->return_remaining_quantity(4)
<< "\n\t\t5 Soda : " << temp->return_remaining_quantity(5)
<< "\n\n********************************************************************************";
}
int print_money_returned(Vending_machine *temp)
{ float balance = float(temp->return_money());
if (balance > 0.0)
{
std::cout << "\nTotal amount $ " << balance/100.0 << " has been returned\n";
}
}
int main()
{
int main_select, main_coin_note, main_val, i=1;
char change_your_mind;
Vending_machine *a = new Vending_machine(1);
while(i!=0)
{
std::cout << "\nCoin(1) or Note(2) : ";
std::cin >> main_coin_note;
std::cout << "\nEnter your denomination :";
std::cin >> main_val;
if (main_coin_note ==0 && main_val ==0)
{
Vending_machine *a = new Vending_machine(1);
std::cout << "Machine reseted !! \n";
print_remaining_quantity(a);
}
else
{
if (a->get_money(main_coin_note, main_val) == 0)
{
std::cout << "\n Wrong denomination, Value returned !";
}
else
{
print_remaining_quantity(a);
std::cout << "\n\tEnter what product you want :" << std::endl
<< "\t\t1 for Candy" << std::endl
<< "\t\t2 for Snack" << std::endl
<< "\t\t3 for Nuts" << std::endl
<< "\t\t4 for Coke" << std::endl
<< "\t\t5 for Soda\n\t : ";
std::cin >> main_select;
std::cout << "\n You want to cancel request?(Y/N) :";
std::cin >> change_your_mind;
if (change_your_mind == 'Y' || change_your_mind == 'y')
{
print_money_returned(a);
}
else
{
switch(a->get_selection(main_select))
{
case 1:
std::cout << "\nProduct has been dispatched";
break;
case 2:
std::cout << "\nInsufficient funds";
break;
case 3:
std::cout << "\nSorry! We are out of Stock!";
break;
case 4:
std::cout << "\nSorry! Unknown Error";
break;
case 5:
std::cout << "\nWrong Selection! Please insert a valid selection";
break;
default:
std::cout << "\nSorry! Unknown Error";
}
print_money_returned(a);
}
}
}
}
}
-
\$\begingroup\$ sorry did not notice that \$\endgroup\$Ram Subramaniam– Ram Subramaniam2017年12月04日 18:28:27 +00:00Commented Dec 4, 2017 at 18:28
-
2\$\begingroup\$ Before anything else...remove all those dynamically allocated objects... \$\endgroup\$Adriano Repetti– Adriano Repetti2017年12月04日 18:42:35 +00:00Commented Dec 4, 2017 at 18:42
-
\$\begingroup\$ OK, you've added std::, but you completely ignored anything about memory allocation that I said. This is the same thing Ben Steffan mentioned in his review yesterday. Put memory allocation into the constructors! \$\endgroup\$pacmaninbw– pacmaninbw ♦2017年12月04日 19:18:30 +00:00Commented Dec 4, 2017 at 19:18
-
\$\begingroup\$ so my compiler was throwing an error on that \$\endgroup\$Ram Subramaniam– Ram Subramaniam2017年12月04日 19:23:54 +00:00Commented Dec 4, 2017 at 19:23
-
\$\begingroup\$ @AdrianoRepetti about the dynamically allocated objects i have been asked to do it this way for the assignment that is the only reason why i did not remove it \$\endgroup\$Ram Subramaniam– Ram Subramaniam2017年12月04日 19:24:33 +00:00Commented Dec 4, 2017 at 19:24
1 Answer 1
Dynamic Allocation
Firstly, why all those objects on the heap?! There is no need for these pointers, simply remove them. If you did need one, in 90% of cases std::shared_ptr
will suffice over a raw pointer.
Now you can use the compiler's automatically generated default destructor as well.
Initialisation Lists
Getting rid of those nasty pointers, you should now use an initialisation list for constructors when all you are doing is passing the arguments to initialise members of your class:
product(const std::string name, const int price) : _name(name), _price(price) {}
and for our data members we would have,
private:
std::string _name;
int _price;
int _remaining_stock{10};
An alternative is to still have your constant maximum stock but use the constexpr
keyword:
constexpr int MAX_STOCK{10};
Notice we are making use of the uniform initialisation syntax.
Const Correctness
Your return functions or 'getters' simply return values and do not modify the state of the class. As such these are constant member functions so for example,
int return_price() const{
return _price;
}
The deduct function can also have its body reduced to simply _remaining_stock--;
Avoid C Arrays`
For accepted_values
, I would go with a std::array
, since the size is known at compile time:
std::array<int, 10> accepted_values;
In the future, always prefer vectors over arrays, when size is not known a priori.
Prefer \n to std::endl
Using std::endl
flushes the stream, which you don't need to do. You want a new line: \n
.