remove conditional statements when dealing with inheritance

Question

I want to avoid conditionals and apply some technique to make code more maintainable and easily extensible when working with inherited classes.

In this example we have.

MarketOrder and LimitOrder both extending from base class Order.

TradeServiceImpl with method getUnitPrice that calculate the price that 2 orders will be trade.

This method has several conditionals to select the correct price for different types of orders. Additionally, it is converting to derived classes.

The problem with this method is that if I want to add 5 new Orders I will have 7*7=49 if statements, in some of them I will need to convert to a derived class that I might confuse with another one, which looks very easy to write buggy code and gives me the smell I should redesign.

Could I get some ideas please?

// Domains.

public class Order{

private OrderTypeEnum orderType; 
// can be buy or sell

public OrderTypeEnum getOrderType() {
    return orderType;
}
public void setOrderType(OrderTypeEnum orderType) {
    this.orderType = orderType;
}
}

public class LimitOrder extends Order{

private Long unitPrice;

public Long getUnitPrice() {
    return unitPrice;
}
public void setUnitPrice(Long unitPrice) {
    this.unitPrice = unitPrice;
}
}

public class MarketOrder extends Order{

}

// Service.

public class TradeServiceImpl implements TradeService{

private TradeRepository tradeRepository;    


public Long getUnitPrice(Order buyOrder, Order sellOrder){

    if(buyOrder instanceof MarketOrder && sellOrder instanceof MarketOrder){
        return tradeRepository.getLastPrice();
    }

    if(buyOrder instanceof MarketOrder && sellOrder instanceof LimitOrder){
        return ((LimitOrder)sellOrder).getUnitPrice();
    }

    if(buyOrder instanceof LimitOrder && sellOrder instanceof MarketOrder){
        return ((LimitOrder)buyOrder).getUnitPrice();
    }

    if(buyOrder instanceof LimitOrder && sellOrder instanceof LimitOrder){
        return ((LimitOrder)buyOrder).getUnitPrice() + ((LimitOrder)sellOrder).getUnitPrice() /2;
    }

    return 0L;
}
}

Answer 1

To begin with, there is two cases which can be coalesced into one: When the orders are different:

 if(buyOrder instanceof MarketOrder && sellOrder instanceof LimitOrder){
        return ((LimitOrder)sellOrder).getUnitPrice();
    }

    if(buyOrder instanceof LimitOrder && sellOrder instanceof MarketOrder){
        return ((LimitOrder)buyOrder).getUnitPrice();
    }

To:

if(!buyOrder.getClass().equals(LimitOrder.getClass())){
        return ((LimitOrder)sellOrder).getUnitPrice();
    }

On the other hand, you can use orders classes as indexes of a map containing function objects. That way you can expand your funcionality by just adding elements to this map.

You can encapsulate calculation algorithms using anonymous inner classes implementing an interface like:

public interface IFunction {
    public Long execute(Order oA, Order oB);
}

And decide which behaviour executed accessing this map using order classes:

Map<Class, Map<Class, IFunction>> opClass2calcAlgorithm = new HashMap();

IFunction market_market = new IFunction() {

            @Override
            public Long execute(Order a, Order b) {
                return tradeRepository.getLastPrice();
            }
        };

IFunction market_limit = new IFunction() {

            @Override
            public Long execute(Order a, Order b) {
                return ((LimitOrder)a).getUnitPrice();
            }
        };

Map<Class, IFunction> marketMap = new HashMap();
marketMap.put(MarketOrder.class, market_market);
marketMap.put(LimitOrder.class, market_limit);
opClass2calcAlgorithm.put(marketMap);

Finally, your getUnitPrice method can be implemented like this:

public Long getUnitPrice(Order buyOrder, Order sellOrder){

    long ret = 0L;

    Map<Class, IFunction> firstLevel = opClass2calcAlgorithm.get(buyOrder.getClass());
    if(firstLevel == null) return ret;
    IFunction calcAlg = firstLevel.get(sellOrder.getClass());
    if(calcAlg == null) return ret;

    ret = calcAlg.execute(buyOrder, sellOrder);

    return ret;
}

Answer 2

First of all, you need not cast the buyOrder/sellOrder to a particular class type before getting the price (because of polymorphism). i.e.

public Long getUnitPrice(Order buyOrder, Order sellOrder){

    if(buyOrder instanceof MarketOrder && sellOrder instanceof MarketOrder){
        return tradeRepository.getLastPrice();
    }

    if(buyOrder instanceof MarketOrder && sellOrder instanceof LimitOrder){
        return sellOrder.getUnitPrice();
    }

    if(buyOrder instanceof LimitOrder && sellOrder instanceof MarketOrder){
        return buyOrder.getUnitPrice();
    }

    if(buyOrder instanceof LimitOrder && sellOrder instanceof LimitOrder){
        return (buyOrder.getUnitPrice() + sellOrder.getUnitPrice()) /2;
    }

    return 0L;
}

Secondly, for the calculation logic, you can use a factory pattern to return appropriate calculator based on the order types. Unfortunately, I don't have enough context about your code to suggest the exact code. I would suggest you search for factory pattern and strategy patterns online. They should help you in designing a better strategy for your code.