CodeSmells: Object oriented violation in Order/calculatePrice() (#61)

Co-authored-by: Tianrui Qi <[email protected]>

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. 

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);
     }
-
 }

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;
     }

Code_Smells/src/Order.java

@@ -14,7 +14,7 @@ public Order(List<Item> 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";

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;