From 2c323e7f0336bc63b5095f29f24d190e674ab9e9 Mon Sep 17 00:00:00 2001 From: Tianqin Yu Date: Tue, 29 Oct 2024 03:02:59 -0400 Subject: [PATCH] CodeSmells: Object oriented violation in `Order/calculatePrice()` (#61) Co-authored-by: Tianrui Qi --- Code_Smells/README.md | 59 ++++++++++++++++++++++++++++++- Code_Smells/src/GiftCardItem.java | 3 +- Code_Smells/src/Item.java | 21 +++++++++++ Code_Smells/src/Order.java | 24 +------------ Code_Smells/src/TaxableItem.java | 15 +++++++- 5 files changed, 96 insertions(+), 26 deletions(-) diff --git a/Code_Smells/README.md b/Code_Smells/README.md index 22b0755..29b1124 100644 --- a/Code_Smells/README.md +++ b/Code_Smells/README.md @@ -7,7 +7,7 @@ java -cp bin main ## Code Smells -### Long method in `Order/calculateTotalPrice()` - +### Long method in `Order/calculateTotalPrice()` The method `calculateTotalPrice` in `Order.java` has a complex logic to determine the price of each kind of item, i.e., price after percentage discount, amount discount, and tax. We can move price calculation logic into a seperate method called `calculatePrice` and call it repeatedly for each item in `calculateTotalPrice` method. @@ -45,3 +45,60 @@ public double calculateTotalPrice() { ``` where `calculatePrice` is a private method with a single function that calculates the price of an item. + + +### Object oriented violation in `Order/calculatePrice()` + +Currently, in `Order/calculatePrice()`, we are using complex logic to determine the price of each item. This is a violation of object-oriented principles as we are not using polymorphism to determine the price of each item. +We can move this calculate price logic into the item class itself and its subclasses. + +For original code in `Order.java` + +```java +private double calculatePrice(Item item) { + double price = item.getPrice(); + // logic to calculate price after discount + // logic to calculate price after quantity + // logic to calculate price after tax + return price; +} +``` + +we move logic to `Item.java` + +```java +public double calculatePrice() { + double final_price = this.getPrice(); + // logic to calculate price after discount + // logic to calculate price after quantity + return final_price; +} +``` + +For special items that need tax, we override this method and include the logic to calculate tax, i.e., in `TaxableItem/calculatePrice()` + +```java +@Override +public double calculatePrice() { + double final_price = super.calculatePrice(); + // logic to calculate price after tax + return final_price; +} +``` + +Then, in `Order/calculateTotalPrice()`, we can simply call `calculatePrice()` method of each item. + +```java +public double calculateTotalPrice() { + double total = 0.0; + for (Item item : items) { + total += item.calculatePrice(); + } + // ... logic applying on total price + return total; +} +``` + +## Contributors + +All members of the group contributed to this code smell detection and reafactoring equally. \ No newline at end of file diff --git a/Code_Smells/src/GiftCardItem.java b/Code_Smells/src/GiftCardItem.java index ea4c5a1..f85ef81 100644 --- a/Code_Smells/src/GiftCardItem.java +++ b/Code_Smells/src/GiftCardItem.java @@ -1,7 +1,8 @@ public class GiftCardItem extends Item { + // constructor + public GiftCardItem(String name, double price, int quantity, DiscountType discountType, double discountAmount){ super(name, price, quantity, discountType, discountAmount); } - } diff --git a/Code_Smells/src/Item.java b/Code_Smells/src/Item.java index 6cfbef8..0ca283a 100644 --- a/Code_Smells/src/Item.java +++ b/Code_Smells/src/Item.java @@ -5,6 +5,8 @@ class Item { private DiscountType discountType; private double discountAmount; + // constructor + public Item(String name, double price, int quantity, DiscountType discountType, double discountAmount) { this.name = name; this.price = price; @@ -13,6 +15,25 @@ public Item(String name, double price, int quantity, DiscountType discountType, this.discountAmount = discountAmount; } + // methods + + public double calculatePrice() { + double final_price = this.getPrice(); + switch (this.getDiscountType()) { + case PERCENTAGE: + final_price -= this.getDiscountAmount() * this.getPrice(); + break; + case AMOUNT: + final_price -= this.getDiscountAmount(); + break; + default: + break; + } + return final_price * this.getQuantity(); + } + + // getter and setter + public String getName() { return name; } diff --git a/Code_Smells/src/Order.java b/Code_Smells/src/Order.java index 218f84b..261f3ae 100644 --- a/Code_Smells/src/Order.java +++ b/Code_Smells/src/Order.java @@ -14,7 +14,7 @@ public Order(List items, String customerName, String customerEmail) { public double calculateTotalPrice() { double total = 0.0; for (Item item : items) { - total += calculatePrice(item); + total += item.calculatePrice(); } if (hasGiftCard()) { total -= 10.0; // subtract $10 for gift card @@ -25,28 +25,6 @@ public double calculateTotalPrice() { return total; } - private double calculatePrice(Item item) { - double price = item.getPrice(); - switch (item.getDiscountType()) { - case PERCENTAGE: - price -= item.getDiscountAmount() * price; - break; - case AMOUNT: - price -= item.getDiscountAmount(); - break; - default: - // no discount - break; - } - price = price * item.getQuantity(); - if (item instanceof TaxableItem) { - TaxableItem taxableItem = (TaxableItem) item; - double tax = taxableItem.getTaxRate() / 100.0 * item.getPrice(); - price += tax; - } - return price; - } - public void sendConfirmationEmail() { String message = "Thank you for your order, " + customerName + "!\n\n" + "Your order details:\n"; diff --git a/Code_Smells/src/TaxableItem.java b/Code_Smells/src/TaxableItem.java index 237e71d..b1593fe 100644 --- a/Code_Smells/src/TaxableItem.java +++ b/Code_Smells/src/TaxableItem.java @@ -1,14 +1,27 @@ - public class TaxableItem extends Item { private double taxRate = 7; + // constructor + public TaxableItem(String name, double price, int quantity, DiscountType discountType, double discountAmount){ super(name, price, quantity, discountType, discountAmount); } + // methods + + @Override + public double calculatePrice() { + double final_price = super.calculatePrice(); + double tax = this.getTaxRate() / 100.0 * this.getPrice(); + return final_price + tax; + } + + // getter and setter + public double getTaxRate(){ return taxRate; } + public void setTaxRate(double rate) { if(rate>=0){ taxRate = rate;