HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Inventories of iPods for different countries

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
countriesinventoriesipodsdifferentfor

Problem

We were given two countries, namely Brazil and Argentina. Both had one inventory each with 100 iPods in them. The cost of an iPod in Brazil was 100/unit and in Argentina it was 50/unit. But to get iPods form Argentina, you also had to pay 400/10units. Now they give us some number and we had to find out which purchase would be cheaper.

#include
#include
#include
#include
class country
{
  public:
    int name;
    int items,unit;
    float cost,price;

  public:
    country();
    void purchase(std::string,int);
    void setData(int,int,float,float, int);
};
country::country()
{
}
void country::setData(int name,int items,float cost,float price,int unit)
{
  name=name;
  items=items;
  cost=cost;
  price=price;
  unit=unit;
}
void country::purchase(std::string a, int b)
{
  float custCost=0;
  std::cout>a;
  cout>b;
  if(a=="brazil")
    con=0;
  else con=1;
  t[con].purchase(a,b);

  getch();
  return 0;
}


It works but I am not totally satisfied with my design as I feel it's too cumbersome. Specifically, if I change the price etc., code changes have to be made.

Solution

Before looking at your design, a few trivial points that you can correct,

#include
#include
#include


  • Avoid c style header usage. Instead use the c++ header style, and namespaces.



-
Instead of conio.h/getch() is not really portable. Avoid it.

-
Your setData method should really be the constructor. Using setData to populate the structure is a c habit. Also be sure to use the member initializers for construction.

You really want to keep the members private

class country {
  private:
    int name;
    int items,unit;
    float cost,price;

    bool try_purchase(std::string,int);

  public:
    country(int,int,float,float, int);
    void do_purchace(std::string a);
};
country::country(int name,int items,float cost,float price,int unit)
          :name(name),items(items),cost(cost),price(price){}

// false indicates that the purchase failed.
bool country::try_purchase(std::string a, int b) {
  if(b > items) return false;
  items = items - b;
  return true;
}
void country::do_purchace(std::string a) {
  int b;
  std::cout>b;
  if (try_purchase(a,b)) 
    std::cout>a;
  int con = (a=="brazil") ? 0 : 1;
  t[con].do_purchace(a);
}


Avoid incorporating io statements with your logic. Refactor purchase so that logic is separated to another method.

Regarding your assertion that code changes have to be made if you change price, I do not see why. You can read the information to construct a country from a file and thus avoid hard coding it in the main file. However, this does not change the design as a whole.

Code Snippets

#include<iostream>
#include<string>
#include<cstdlib>
class country {
  private:
    int name;
    int items,unit;
    float cost,price;

    bool try_purchase(std::string,int);

  public:
    country(int,int,float,float, int);
    void do_purchace(std::string a);
};
country::country(int name,int items,float cost,float price,int unit)
          :name(name),items(items),cost(cost),price(price){}

// false indicates that the purchase failed.
bool country::try_purchase(std::string a, int b) {
  if(b > items) return false;
  items = items - b;
  return true;
}
void country::do_purchace(std::string a) {
  int b;
  std::cout<<"enter the items needed: ";
  std::cin>>b;
  if (try_purchase(a,b)) 
    std::cout<<name<<":"<<items<<":"<<cost * b<<std::endl;
}
int main()
{
  country t[] = {country(0,100,100,0,0), country(1,100,50,400,20)};
  std::string a;
  std::cout<<"enter the country: ";
  std::cin>>a;
  int con = (a=="brazil") ? 0 : 1;
  t[con].do_purchace(a);
}

Context

StackExchange Code Review Q#12763, answer score: 4

Revisions (0)

No revisions yet.