3
\$\begingroup\$

Recently, I have appeared for coding exercise to design the Shopping Cart which should have following features:

  1. handles barcoded (like Chips) and non-barcoded item (like Banana 400gm)
  2. Can add item, remove item, increment & decrement quantity of item.
  3. Supports 2 For 1 offer for BarCoded Item.

I have completed the exercise and tried to follow the SOLID principle as much as I can. However, I can see there is lot of scope of improvement. I really appreciate if some one have a look at the code below and let me know the area of improvements.

  1. Booking.java

    package uk.co.supermarket;
    
    public class Basket {
    
        private List<CartItem> cartItemList = new ArrayList<>();
    
        private Float totalPriceBeforeDiscount = 0.0f;
    
        private Float totalPriceAfterDiscount = 0.0f;
    
        private Float totalDiscount = 0.0f;
    
        /* Key Value Pair of product code and Offer. */
        Map<String, Offer> offerMap = new HashMap<>();
    
        private DiscountStrategy discountStrategy;
    
        /**
         * @param offerMap
         * @param discountStrategy
         */
        public Basket(Map<String, Offer> offerMap, DiscountStrategy discountStrategy) {
            super();
            this.offerMap = offerMap;
            this.discountStrategy = discountStrategy;
        }
    
        public void addCartItem(CartItem cartItem) {
            int idxPos = cartItemList.indexOf(cartItem);
            if( idxPos != -1) {
                cartItem = cartItemList.get(idxPos);
                cartItem.addOne();
            }else {
                cartItemList.add(cartItem);
            }
            refreshTotal();
        }
    
        public int getTotalLineItem() {
            return cartItemList.size();
        }
    
        public Boolean removeCartItem(CartItem cartItem) {
            Boolean isLineItemRemoved = cartItemList.remove(cartItem);
            refreshTotal();
            return isLineItemRemoved;
        }
    
        /**
         * if cart item already in the basket then remove the item when there is only one quantity else reduce it by one.
         * if cart item not  
         **/
        public Boolean decreaseByOne(CartItem cartItem) {
            Boolean isLineItemReduced = false;
            int idxPos = cartItemList.indexOf(cartItem);
            if( idxPos != -1) {
                cartItem = cartItemList.get(idxPos);
                int qty = cartItem.reduceOne();
                if(qty == 0) {
                    cartItemList.remove(cartItem);
                }
                isLineItemReduced = true;
                refreshTotal(); 
            }
            return isLineItemReduced  ;
        }
    
        private void refreshTotal() {
            totalPriceBeforeDiscount = 0.0f;
            cartItemList.forEach(item -> totalPriceBeforeDiscount += item.getLineItemTotalBeforeDiscount());
            totalDiscount = discountStrategy.calculateDiscount(this.offerMap, this.cartItemList);
            totalPriceAfterDiscount = totalPriceBeforeDiscount - totalDiscount;
        }
    
        /**
         * @return the lineItemList
         */
        public List<CartItem> getCartItems() {
            return Collections.unmodifiableList(cartItemList);
        }
    
        /**
         * @return the totalPriceBeforeDiscount
         */
        public Float getTotalPriceBeforeDiscount() {
            return totalPriceBeforeDiscount;
        }
    
        public Float getTotalPriceAfterDiscount() {
            return totalPriceAfterDiscount;
        }
    
        public Float getTotalDiscount() {
            return totalDiscount;
        }
    
    }
    
  2. CartItem.java

    package uk.co.supermarket;
    public class CartItem {
    
        private final Product product;
    
        private Integer quantity;
    
        private Float weight ;
    
    
        /**
         * @param product
         */
        public CartItem(Product product, Integer quantity) {
            super();
            this.product = product;
            this.quantity = quantity;
        }
    
        /**
         * @param product
         */
        public CartItem(Product product) {
            this(product, 1); 
        }
    
        /**
         * @param product
         * @param weight
         */
        public CartItem(Product product, Float weight) {
            this(product); 
            this.weight = weight;
        }
    
    
        public Float getLineItemTotalBeforeDiscount() {
            switch(product.getProductType()) {
            case LOOSE : 
                return this.weight * product.getPrice().getUnitPrice();
            default :   
                return quantity * this.product.getPrice().getUnitPrice() ;
            }
        }
    
    
        /**
         * @return the product
         */
        public final Product getProduct() {
            return product;
        }
    
    
        /**
         * @return the quantity
         */
        public Integer addOne() {
            return quantity = quantity + 1;
        }
    
        /**
         * @return the quantity
         */
        public Integer reduceOne() {
            return quantity = quantity - 1;
        }
    
        /**
         * @return the quantity
         */
        public Integer getQuantity() {
            return quantity;
        }
    
    
        /* (non-Javadoc)
         * @see java.lang.Object#hashCode()
         */
        @Override
        public int hashCode() {
            final int prime = 31;
            int result = 1;
            result = prime * result + ((product == null) ? 0 : product.hashCode());
            return result;
        }
    
    
        /* (non-Javadoc)
         * @see java.lang.Object#equals(java.lang.Object)
         */
        @Override
        public boolean equals(Object obj) {
            if (this == obj)
                return true;
            if (obj == null)
                return false;
            if (getClass() != obj.getClass())
                return false;
            CartItem other = (CartItem) obj;
            if (product == null) {
                if (other.product != null)
                    return false;
            } else if (!product.equals(other.product))
                return false;
            return true;
        }
    
    }
    
  3. Product.java

package uk.co.supermarket;

public class Product {

    private String productCode;

    private ProductType productType;

    private Price price;


    /**
     * @param productCode
     * @param productType
     * @param price
     */
    public Product(String productCode, ProductType productType, Price price) {
        super();
        this.productCode = productCode;
        this.productType = productType;
        this.price = price;
    }

    /**
     * @return the price
     */
    public Price getPrice() {
        return price;
    }


    public String getProductCode() {
        return productCode;
    }


    /**
     * @return the productType
     */
    public ProductType getProductType() {
        return productType;
    }

    /* (non-Javadoc)
     * @see java.lang.Object#hashCode()
     */
    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((productCode == null) ? 0 : productCode.hashCode());
        result = prime * result + ((productType == null) ? 0 : productType.hashCode());
        return result;
    }

    /* (non-Javadoc)
     * @see java.lang.Object#equals(java.lang.Object)
     */
    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Product other = (Product) obj;
        if (productCode == null) {
            if (other.productCode != null)
                return false;
        } else if (!productCode.equals(other.productCode))
            return false;
        if (productType != other.productType)
            return false;
        return true;
    }



}
  1. ProductType.java
package uk.co.supermarket;

public enum ProductType {

    BARCODED, LOOSE;
}

  1. Discount.java
package uk.co.supermarket.offers;

import uk.co.supermarket.CartItem;

public interface Discount {
    float applyDiscount(CartItem lineItem);
}

  1. DiscountType.java
package uk.co.supermarket.offers;

public enum DiscountType {

    TWO_FOR_ONE(new PercenatageDiscount(0.50f)),
    THREE_FOR_ONE(new PercenatageDiscount(0.33f));

    private final Discount discount;

    public Discount getDiscount() {
        return discount;
    }

    private DiscountType(Discount discount) {
        this.discount = discount;
    }

}

  1. Offer.java
package uk.co.supermarket.offers;

import uk.co.supermarket.Product;

public class Offer {

    private final Product product;
    private final DiscountType discountType;
    private final int quantity;

    /**
     * @param product
     * @param quantity
     * @param discountType
     */
    public Offer(Product product, int quantity, DiscountType discountType) {
        super();
        this.product = product;
        this.quantity = quantity;
        this.discountType = discountType;
    }
    /**
     * @return the product
     */
    public Product getProduct() {
        return product;
    }
    /**
     * @return the quantity
     */
    public int getQuantity() {
        return quantity;
    }
    /**
     * @return the discount
     */
    public DiscountType getDiscountType() {
        return discountType;
    }

    /* (non-Javadoc)
     * @see java.lang.Object#toString()
     */
    @Override
    public String toString() {
        return "Offer [product=" + product + ", quantity=" + quantity + ", discountType=" + discountType + "]";
    }



}

  1. PercenatageDiscount.java
package uk.co.supermarket.offers;

import uk.co.supermarket.CartItem;

public class PercenatageDiscount implements Discount{

    private float discountPercent ;

    public float getDiscountPercent() {
        return discountPercent;
    }

    public PercenatageDiscount(float discountPercent) {
        super();
        assert discountPercent <= 1;
        this.discountPercent = discountPercent;
    }

    @Override
    public String toString() {
        return "Discount [ discountPercent=" + discountPercent + "]";
    }

    @Override
    public float applyDiscount(CartItem lineItem) {
        return lineItem.getLineItemTotalBeforeDiscount() - (lineItem.getLineItemTotalBeforeDiscount() * ( discountPercent));
    }

}

  1. DiscountStrategy.java
package uk.co.supermarket;

import java.util.List;
import java.util.Map;

import uk.co.supermarket.offers.Offer;

public class DiscountStrategy {

    public Float calculateDiscount(final Map<String, Offer> offerMap, final List<CartItem> set) {

        float totalDiscount = 0.0f;

        //apply discount if the offer matches with line item
        for(CartItem item : set) {
            Offer offer = offerMap.get(item.getProduct().getProductCode());
            if(offer != null 
                    && item.getProduct().getProductCode().equals(offer.getProduct().getProductCode())
                    && item.getQuantity() >= offer.getQuantity()) {
                        //This is the logic to handle the case when cart item quantity is more than offer 
                        // eg. when cart item are 3 and offer is 2 For 1 
                        int mod = item.getQuantity() % offer.getQuantity();
                        int quantityOnWhichOfferIsApplied = mod == 0 ? item.getQuantity() : item.getQuantity() - mod;
                        totalDiscount += offer.getDiscountType().getDiscount().applyDiscount(new CartItem(item.getProduct(), quantityOnWhichOfferIsApplied));
            }
        }
        return totalDiscount;
    }
}

  1. BasketTest.java
package uk.co.supermarket;

import static org.junit.Assert.assertTrue;

import java.util.HashMap;
import java.util.Map;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import uk.co.supermarket.offers.DiscountType;
import uk.co.supermarket.offers.Offer;

public class BasketTest {

    private Basket basket = null;
    private DiscountStrategy discountStrategy = null;
    private Map<String,Offer> map = null;

    private Price price_5 =  new Price(5.0f);
    private Price price_10 =  new Price(10.0f);
    private Price price_15 =  new Price(15.0f);
    private Price price_20 =  new Price(20.0f);

    private Product bean_can = new Product("BEAN_CAN",ProductType.BARCODED,price_10);
    private Product coke_bottle = new Product("COKE_BOTTLE", ProductType.BARCODED,price_15);
    private Product coke_can = new Product("COKE_CAN", ProductType.BARCODED,price_5);
    private Product orange = new Product("ORANGE", ProductType.LOOSE,price_10);
    private Product apple = new Product("APPLE", ProductType.LOOSE,price_15);
    private Product banana = new Product("BANANA", ProductType.LOOSE,price_20);

    @Before
    public void setup() {
        discountStrategy = new DiscountStrategy();
         map = new HashMap<>();
         basket = new Basket(map,discountStrategy);
    }

    @After
    public void cleanUp() {
        map.clear();
    }

    @Test
    public void testAddSingleBarCodedLineItem() {

        CartItem barCodedItem = new CartItem(bean_can);
        basket.addCartItem(barCodedItem);

        assertTrue(1 == basket.getTotalLineItem());
        assertTrue(10.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(10.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());
    }

    @Test
    public void testAddMultipleQuantityOfSingleBarCodedProduct() {
        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(bean_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);

        assertTrue(1 == basket.getTotalLineItem());
        assertTrue(3 == basket.getCartItems().get(0).getQuantity());
        assertTrue(30.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(30.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());
    }

    @Test
    public void testAddMultipleQuantityofSingleBarCodedProductWithRemovingOneUnit() {
        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(bean_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);
        CartItem barCodedItem_4 = new CartItem(new Product("BEAN_CAN", ProductType.BARCODED,new Price(10.0f)));

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);
        basket.addCartItem(barCodedItem_4);

        assertTrue(1 == basket.getTotalLineItem());
        assertTrue(4 == basket.getCartItems().get(0).getQuantity());
        assertTrue(40.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(40.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

        basket.decreaseByOne(barCodedItem_4);

        assertTrue(1 == basket.getTotalLineItem());
        assertTrue(3 == basket.getCartItems().get(0).getQuantity());
        assertTrue(30.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(30.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

    }

    @Test
    public void testAddMultipleQuantityofSingleBarCodedProductWithRemovingCartItem() {

        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(bean_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);
        CartItem barCodedItem_4 = new CartItem(bean_can);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);
        basket.addCartItem(barCodedItem_4);

        //checking product with right quantity are added 
        assertTrue(1 == basket.getTotalLineItem());
        assertTrue(4 == basket.getCartItems().get(0).getQuantity());
        assertTrue(40.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(40.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

        // remove the product altogether
        basket.removeCartItem(barCodedItem_4);

        assertTrue(0 == basket.getTotalLineItem());
        assertTrue(basket.getCartItems().isEmpty());
        assertTrue(0.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(0.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

    }

    @Test
    public void testAddMultipleQuantityofMultipleBarCodedProduct() {

        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(bean_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);
        CartItem barCodedItem_4 = new CartItem(bean_can);

        CartItem barCodedItem_5 = new CartItem(coke_bottle);
        CartItem barCodedItem_6 = new CartItem(coke_bottle);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);
        basket.addCartItem(barCodedItem_4);
        basket.addCartItem(barCodedItem_5);
        basket.addCartItem(barCodedItem_6);

        //checking product with right quantity are added 
        assertTrue(2 == basket.getTotalLineItem());
        assertTrue(4 == basket.getCartItems().get(0).getQuantity());
        assertTrue(2 == basket.getCartItems().get(1).getQuantity());
        assertTrue(70.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(70.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

    }

    @Test
    public void testAddMultipleQuantityofMultipleBarCodedProductWithReducingOneUnitEach() {

        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(bean_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);
        CartItem barCodedItem_4 = new CartItem(bean_can);

        CartItem barCodedItem_5 = new CartItem(coke_bottle);
        CartItem barCodedItem_6 = new CartItem(coke_bottle);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);
        basket.addCartItem(barCodedItem_4);
        basket.addCartItem(barCodedItem_5);
        basket.addCartItem(barCodedItem_6);

        //checking product with right quantity are added 
        assertTrue(2 == basket.getTotalLineItem());
        assertTrue(4 == basket.getCartItems().get(0).getQuantity());
        assertTrue(2 == basket.getCartItems().get(1).getQuantity());
        assertTrue(70.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(70.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

        //reducing the quantity for both products
        basket.decreaseByOne(barCodedItem_4);
        basket.decreaseByOne(barCodedItem_5);

        assertTrue(2 == basket.getTotalLineItem());
        assertTrue(3 == basket.getCartItems().get(0).getQuantity());
        assertTrue(1 == basket.getCartItems().get(1).getQuantity());
        assertTrue(45.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(45.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

    }

    @Test
    public void testAddMultipleQuantityofMultipleBarCodedProductWithRemovingOneCartItem() {

        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(bean_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);
        CartItem barCodedItem_4 = new CartItem(bean_can);


        CartItem barCodedItem_5 = new CartItem(coke_bottle);
        CartItem barCodedItem_6 = new CartItem(coke_bottle);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);
        basket.addCartItem(barCodedItem_4);
        basket.addCartItem(barCodedItem_5);
        basket.addCartItem(barCodedItem_6);

        //checking product with right quantity are added 
        assertTrue(2 == basket.getTotalLineItem());
        assertTrue(4 == basket.getCartItems().get(0).getQuantity());
        assertTrue(2 == basket.getCartItems().get(1).getQuantity());
        assertTrue(70.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(70.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

        //reducing the quantity for both products
        basket.decreaseByOne(barCodedItem_4);

        assertTrue(2 == basket.getTotalLineItem());
        assertTrue(3 == basket.getCartItems().get(0).getQuantity());
        assertTrue(2 == basket.getCartItems().get(1).getQuantity());
        assertTrue(60.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(60.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

    }

    @Test
    public void testAddSingleWeightedProduct() {
        CartItem weightedItem = new CartItem(orange, 0.20f);

        basket.addCartItem(weightedItem);

        assertTrue(1 ==  basket.getTotalLineItem());
        assertTrue(2.0f ==  basket.getTotalPriceBeforeDiscount());
        assertTrue(2.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());
    }

    @Test
    public void testAddMultipleWeightedProduct() {
        CartItem looseItem_1 = new CartItem(orange, 0.20f);
        CartItem looseItem_2 = new CartItem(apple, 0.40f);

        basket.addCartItem(looseItem_1);
        basket.addCartItem(looseItem_2);

        assertTrue(2 ==  basket.getTotalLineItem());
        assertTrue(1 == basket.getCartItems().get(0).getQuantity());
        assertTrue(1 == basket.getCartItems().get(1).getQuantity());
        assertTrue(8.0f ==  basket.getTotalPriceBeforeDiscount());
        assertTrue(8.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());
    }

    @Test
    public void testAddMultipleBarCodedAndMultipleWeightedLineItems() {
        CartItem barCodedItem_1 = new CartItem(bean_can);
        CartItem barCodedItem_2 = new CartItem(coke_bottle);
        CartItem barCodedItem_3 = new CartItem(coke_can);
        CartItem weightedItem_1 = new CartItem(orange, 0.4f);
        CartItem weightedItem_2 = new CartItem(apple, 0.5f);
        CartItem weightedItem_3 = new CartItem(banana, 2.5f);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);
        basket.addCartItem(barCodedItem_3);

        basket.addCartItem(weightedItem_1);
        basket.addCartItem(weightedItem_2);
        basket.addCartItem(weightedItem_3);

        assertTrue(6 == basket.getTotalLineItem());
        assertTrue(91.5f == basket.getTotalPriceBeforeDiscount());
        assertTrue(91.5f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());
    }

    @Test
    public void testApply2For1Offer() {
        map.put("COKE_CAN", new Offer(coke_can,2,DiscountType.TWO_FOR_ONE));
        map.put("BEAN_CAN", new Offer(bean_can,3,DiscountType.THREE_FOR_ONE));

        CartItem barCodedItem_1 = new CartItem(coke_can);
        CartItem barCodedItem_2 = new CartItem(coke_can);
        CartItem barCodedItem_3 = new CartItem(bean_can);
        CartItem barCodedItem_4 = new CartItem(bean_can);
        CartItem barCodedItem_5 = new CartItem(bean_can);
        CartItem barCodedItem_6 = new CartItem(bean_can);

        basket.addCartItem(barCodedItem_1);
        basket.addCartItem(barCodedItem_2);

        basket.addCartItem(barCodedItem_3);
        basket.addCartItem(barCodedItem_4);
        basket.addCartItem(barCodedItem_5);
        basket.addCartItem(barCodedItem_6);

        assertTrue(2 == basket.getTotalLineItem());
        assertTrue(2 == basket.getCartItems().get(0).getQuantity());
        assertTrue(4 == basket.getCartItems().get(1).getQuantity());
        assertTrue(50.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(Math.abs(basket.getTotalPriceAfterDiscount() - 24.900002f) < 0.0001f);
        assertTrue(Math.abs(basket.getTotalDiscount()- 25.099998) < 0.0001f);

    }

    @Test
    public void testRemoveSingleBarCodedProduct() {
        CartItem barCodedItem_1 = new CartItem(bean_can);

        basket.addCartItem(barCodedItem_1);

        assertTrue(1 == basket.getTotalLineItem());
        assertTrue(1 == basket.getCartItems().get(0).getQuantity());
        assertTrue(10.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(10.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

        basket.decreaseByOne(barCodedItem_1);

        assertTrue(0 == basket.getTotalLineItem());
        assertTrue(basket.getCartItems().isEmpty());
        assertTrue(0.0f == basket.getTotalPriceBeforeDiscount());
        assertTrue(0.0f == basket.getTotalPriceAfterDiscount());
        assertTrue(0.0f == basket.getTotalDiscount());

    }
}

```
\$\endgroup\$
  • 2
    \$\begingroup\$ Don't use float for prices since calculation results can become quite inaccurate. Better use BigDecimal. See also stackoverflow.com/questions/12496555/… \$\endgroup\$ – Joachim Rohde Jul 19 at 12:16
  • \$\begingroup\$ Thanks. Indeed I should be using BigDecimal. I will make the correction. Is there anything from the design perspective I am missing here ? Any thoughts ? \$\endgroup\$ – NIGAGA Jul 19 at 13:26
  • 1
    \$\begingroup\$ The Basket class doesn't extend anything so there is no need for the super() call in the constructor. \$\endgroup\$ – d33j Aug 1 at 6:33
1
\$\begingroup\$

The best way to see how the system is architected is to look at the tests. It is great that you have provided such tests with the BasketTest class! So I will review your code by reviewing the BasketTestclass.

  • Initializing class fields to null is unnecessary and duplicate as they are automatically initialized to null (default value)
  • setup() would be a better place to setup your products.
  • Don't use field and variable names like map, list. They need to speak to whomever is reading the code. I have no idea what your map represents.
  • Make name of the products like "COKE_CAN" constants and replace all occurrences in the code with the new constants.
  • Without looking into the implementation, I cannot tell what the 2nd parameter means: new Offer(coke_can, 2, DiscountType.TWO_FOR_ONE). I would like this code to be improved to be readable with less effort.
  • Use assertArrayEquals(float[] expecteds, float[] actuals, float delta) for comparing floats.
  • Improve testApply2For1Offer() test. There you put elements to the map but it is not clear how the map interacts with the rest of the code.
  • Every basked needs to have DiscountStrategy and offers in form of Map<String, Offer>. This does not seem right. I would think that there is only one Discounts object that can calculate a discount or final price for a given Basket. This is the biggest issues I see here, the design of the Basket.

Overall you did great with your design! I like it.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your review. Excluding the last point, I have updated the code base for the review comment The last one require some thought before I do any change. I agree basket does not need to know about Offer. I think Offer should sit inside the product. \$\endgroup\$ – NIGAGA Aug 13 at 23:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.