patternjavaMinor
Super Market Pricing Model
Viewed 0 times
supermarketmodelpricing
Problem
I have modelled the task of super market pricing as per the below task.
http://codekata.com/kata/kata01-supermarket-pricing/
Please review my code and suggest any improvements.
```
class Product
{
String Name; //Name of the product
int unit_price;
UnitType unit_type;
boolean groupOffer; //Is this product eligible for a group offer?
int buy,get; //If so..what are the values of buy and get..
public Product(String name, int unit_price, UnitType unit_type, boolean groupOffer) {
// TODO Auto-generated method stub
Name = name;
this.unit_price = unit_price;
this.unit_type = unit_type;
this.groupOffer = groupOffer;
}
public Product(String name, int unit_price, UnitType unit_type,boolean groupOffer, int buy, int get) {
this(name,unit_price,unit_type,groupOffer);
this.buy = buy;
this.get = get;
}
}
enum UnitType
{
NUMERIC, //to represent non-breakable units.
POUND,
KG,
GRAM;
}
public class SuperMarket {
public static void main(String args[])
{
//Configuring the price of products.
Product pen = new Product("Pen",5,UnitType.NUMERIC,Boolean.FALSE);
Product potato = new Product("Potato",2,UnitType.POUND,Boolean.FALSE);
Product soda = new Product("Soda",1,UnitType.NUMERIC,Boolean.TRUE,4,1);
//Calculating the prices
System.out.println(getPrice(pen,4));
System.out.println(getPrice(pen,4.1f)); //Pen cannot be a fraction
System.out.println(getPrice(potato,3));
System.out.println(getPrice(potato,3.1f));//Potato can be a fraction
System.out.println(getPrice(soda,10)); // Eligible for group offer
}
//Get the price of products
public static float getPrice(Product product,float quantity)
{
if(product.unit_type == UnitType.NUMERIC)
{
if(quantity != Math.round(quantity))
return -1000;
}
float price = p
http://codekata.com/kata/kata01-supermarket-pricing/
Please review my code and suggest any improvements.
```
class Product
{
String Name; //Name of the product
int unit_price;
UnitType unit_type;
boolean groupOffer; //Is this product eligible for a group offer?
int buy,get; //If so..what are the values of buy and get..
public Product(String name, int unit_price, UnitType unit_type, boolean groupOffer) {
// TODO Auto-generated method stub
Name = name;
this.unit_price = unit_price;
this.unit_type = unit_type;
this.groupOffer = groupOffer;
}
public Product(String name, int unit_price, UnitType unit_type,boolean groupOffer, int buy, int get) {
this(name,unit_price,unit_type,groupOffer);
this.buy = buy;
this.get = get;
}
}
enum UnitType
{
NUMERIC, //to represent non-breakable units.
POUND,
KG,
GRAM;
}
public class SuperMarket {
public static void main(String args[])
{
//Configuring the price of products.
Product pen = new Product("Pen",5,UnitType.NUMERIC,Boolean.FALSE);
Product potato = new Product("Potato",2,UnitType.POUND,Boolean.FALSE);
Product soda = new Product("Soda",1,UnitType.NUMERIC,Boolean.TRUE,4,1);
//Calculating the prices
System.out.println(getPrice(pen,4));
System.out.println(getPrice(pen,4.1f)); //Pen cannot be a fraction
System.out.println(getPrice(potato,3));
System.out.println(getPrice(potato,3.1f));//Potato can be a fraction
System.out.println(getPrice(soda,10)); // Eligible for group offer
}
//Get the price of products
public static float getPrice(Product product,float quantity)
{
if(product.unit_type == UnitType.NUMERIC)
{
if(quantity != Math.round(quantity))
return -1000;
}
float price = p
Solution
-
-
-
-
Currency
It is recommended to use BigDecimal class while dealing with Currency or monetary values as it provides better handling of floating point numbers and their operations.
-
I'd rename
-
I'd implement the body of your first constructor with just:
-
And the one of the second with:
-
I'd add a space after the commas in parameter and argument lists for easier reading.
-
I'd change
-
It's convention to separate name and parenthesis in statements like
-
I'd simplify:
to:
String Name; – Variable names begin with lowercase by convention: String name;.-
unit_price; unit_type; – Variable names are camel case by convention: unitPrice; unitType;-
boolean groupOffer – It's convention to start boolean variables with is..., has..., can..., must...: boolean isEligibleForGroupOffer. Such you also don't need the comment.-
int unit_price; int buy,get;- Your amounts are integers only? Really? Not
floatorBigDecimal:
Currency
It is recommended to use BigDecimal class while dealing with Currency or monetary values as it provides better handling of floating point numbers and their operations.
buyandgetare verbs that fit better to a method name. Do you meanbuyingPriceandsellingPrice?
- More than one variable declaration per line is discouraged.
-
I'd rename
UnitType to SellingUnitand NUMERIC to PIECES.-
I'd implement the body of your first constructor with just:
this(name, unitPrice, sellingUnit, isEligibleForGroupOffer, 0, 0);-
And the one of the second with:
this.name = name;
this.unitPrice = unitPrice;
this.sellingUnit = sellingUnit;
this.isEligibleForGroupOffer= isEligibleForGroupOffer;
this.buyingPrice = buyingPrice;
this.sellingPrice = sellingPrice;-
I'd add a space after the commas in parameter and argument lists for easier reading.
-
I'd change
static ... getPrice(...) to a non-static member of Product with then a header of public BigDecimal getPriceFor(float quantity).-
It's convention to separate name and parenthesis in statements like
if (...) with a space to easily distinguish them from method invocations.-
I'd simplify:
float price = product.unit_price * quantity;
//To decrease the price of free items.
if(product.groupOffer) {
price = (quantity/(product.buy+product.get)) * product.buy * product.unit_price;
}
return price;to:
return isEligibleForGroupOffer // To decrease the price of free items.
? quantity / (buyingPrice + sellingPrice) * buyingPrice * unitPrice
: unitPrice * quantity;Code Snippets
this(name, unitPrice, sellingUnit, isEligibleForGroupOffer, 0, 0);this.name = name;
this.unitPrice = unitPrice;
this.sellingUnit = sellingUnit;
this.isEligibleForGroupOffer= isEligibleForGroupOffer;
this.buyingPrice = buyingPrice;
this.sellingPrice = sellingPrice;float price = product.unit_price * quantity;
//To decrease the price of free items.
if(product.groupOffer) {
price = (quantity/(product.buy+product.get)) * product.buy * product.unit_price;
}
return price;return isEligibleForGroupOffer // To decrease the price of free items.
? quantity / (buyingPrice + sellingPrice) * buyingPrice * unitPrice
: unitPrice * quantity;Context
StackExchange Code Review Q#109472, answer score: 6
Revisions (0)
No revisions yet.