Avoid if and switch statement and more generic approach

Question

I want to avoid the use of "if" and "switch" statements which deal with specific cases and instead taking a more generic approach.

Requirement: Based on a number of requirements which are detailed below, the status of an Order may be determined to be "Confirmed", "Closed" or "AuthorisationRequired".

Inputs:

IsRushOrder: bool
OrderType: Enum (Repair, Hire)
IsNewCustomer: bool
IsLargeOrder: bool

Outputs:

OrderStatus: Enum (Confirmed, Closed, AuthorisationRequired)

Requirements (Applied in priority order from top to bottom):

-Large repair orders for new customers should be closed
-Large rush hire orders should always be closed
-Large repair orders always require authorisation
-All rush orders for new customers always require authorisation
-All other orders should be confirmed

==========================================================================

Avoiding Obvious answer with IF statement

This violates O of SOLID principle. I am sure there is better way to do this than adding lots of IF statements

        public IOrder GetOrder(OrderInput orderInput)
        {

            // Apply rules based on the priority 
            if (orderInput.IsLargeOrder == true && orderInput.OrderType == OrderTypeEnum.Repair && orderInput.IsNewCustomer == true)
            {
                return new LargeRepairNewCustomerOrder();
            }

            if (orderInput.IsLargeOrder == true && orderInput.OrderType == OrderTypeEnum.Hire && orderInput.IsRushOrder == true)
            {
                return new LargeRushHireOrder();
            }

            if (orderInput.IsLargeOrder == true && orderInput.OrderType == OrderTypeEnum.Repair)
            {
                return new LargeRepairOrder();
            }

            if (orderInput.IsRushOrder == true && orderInput.IsNewCustomer == true)
            {
                return new AllRushNewCustomerOrder();
            }

            return new AllOtherOrders();
        }

One way i could think of implementing without using IF is by populating OrderStatus in collection and querying using LINQ This implementaion will get complicated quickly as we add more statuses.

public class OrderStatusAnalyzer
{
    protected static List<OrderStatus> OrderStatuses;

    public OrderStatusAnalyzer()
    {
        OrderStatuses = OrderStatusCollection.Get();
    }

    public string GetOrderStatusTypeByOrderInput(OrderInput orderInput)
    {
        var orderStatusTypes = from orderStatus in OrderStatuses
                               where orderStatus.IsNewCustomer == orderInput.IsNewCustomer
                                     && orderStatus.IsLargeOrder == orderInput.IsLargeOrder
                                     && orderStatus.IsRushOrder == orderInput.IsRushOrder
                                     && orderStatus.OrderType == orderInput.OrderType
                               orderby orderStatus.Priority ascending
                               select orderStatus.OrderStatusType;

        var statusTypesList = orderStatusTypes.ToList();
        var orderStatusType = !statusTypesList.Any() ? "VehicleRepairOrder.Order.AllOtherOrders" : statusTypesList[0];

        return orderStatusType;
    }
}

public static class OrderStatusCollection
{
    public static List<OrderStatus> Get()
    {
        // Note:
        // 1) If we have to avoid referencing null, then we can populate the data with probabilities. Populating data with probabilities will become hard Maintain as we add more status   
        // 2) this is also violating O(Open for extension and closed for modification) of SOLID principle 
        // 3) instead of passing null, if you pass actual unit test will fail
        var orderStatuses = new List<OrderStatus>
        {
            new OrderStatus
            {
                IsLargeOrder = true, IsNewCustomer = true, OrderType = OrderTypeEnum.Repair,
                IsRushOrder = null,
                Priority = 1, OrderStatusType = "VehicleRepairOrder.Order.LargeRepairNewCustomerOrder"
            },

            new OrderStatus
            {
                IsLargeOrder = true, IsNewCustomer = null, OrderType = OrderTypeEnum.Hire, IsRushOrder = true,
                Priority = 2, OrderStatusType = "VehicleRepairOrder.Order.LargeRushHireOrder"
            },

            new OrderStatus
            {
                IsLargeOrder = true, IsNewCustomer = null, OrderType = OrderTypeEnum.Repair,
                IsRushOrder = null,
                Priority = 3, OrderStatusType = "VehicleRepairOrder.Order.LargeRepairOrder"
            },

            new OrderStatus
            {
                IsLargeOrder = null, IsNewCustomer = true, OrderType = OrderTypeEnum.Any, IsRushOrder = true,
                Priority = 4, OrderStatusType = "VehicleRepairOrder.Order.AllRushNewCustomerOrder"
            },

        };

        return orderStatuses;
    }
}

Answer 1

It's not entirely clear why the 'no ifs' rule should be imposed. Even a set of principles like SOLID are meant to help solving particular problems. If you don't have those problems, you don't need SOLID. As stated, the problem is so simple that you don't need SOLID.

I'm guessing, however, that this question is a substitute for a larger problem, and that you'd like to be able to change rules independently of each other, which reminds me a bit of this article.

Depending on the actual constraints, you can address the concerns in several ways. One that came easily to my mind when I read the question is the Chain of Responsibility pattern.

You could start by defining an interface:

public interface IPolicy
{
    OrderStatus DecideStatus(
        bool isRushOrder,
        OrderType orderType,
        bool isNewCustomer,
        bool isLargeOrder);
}

Then you implement a set of concrete policies, e.g.:

public class LargeRepairOrderForNewCustomersPolicy : IPolicy
{
    public LargeRepairOrderForNewCustomersPolicy(IPolicy next)
    {
        Next = next;
    }

    public IPolicy Next { get; }

    public OrderStatus DecideStatus(
        bool isRushOrder,
        OrderType orderType,
        bool isNewCustomer,
        bool isLargeOrder)
    {
        if (isLargeOrder && orderType == OrderType.Repair && isNewCustomer)
            return OrderStatus.Closed;

        return Next.DecideStatus(isRushOrder, orderType, isNewCustomer, isLargeOrder);
    }
}

or:

public class LargeRushHireOrderPolicy : IPolicy
{
    public LargeRushHireOrderPolicy(IPolicy next)
    {
        Next = next;
    }

    public IPolicy Next { get; }

    public OrderStatus DecideStatus(
        bool isRushOrder,
        OrderType orderType,
        bool isNewCustomer,
        bool isLargeOrder)
    {
        if (isLargeOrder && isRushOrder && orderType == OrderType.Hire)
            return OrderStatus.Closed;

        return Next.DecideStatus(isRushOrder, orderType, isNewCustomer, isLargeOrder);
    }
}

To use them, compose the chain:

private IPolicy CreatePolicy()
{
    return new LargeRepairOrderForNewCustomersPolicy(
        new LargeRushHireOrderPolicy(
            new LargeRepairOrderPolicy(
                new RushOrderForNewCustomersPolicy(
                    new DefaultPolicy()))));
}

The object is now ready for use:

IPolicy policy = CreatePolicy();
var status = policy.DecideStatus(isRushOrder, orderType, isNewCustomer, isLargeOrder);

If you need to change the policies, you can edit individual policies, add new classes, or reorder the chain.

Answer 2

I'm going to answer based on your stated requirements, rather than your sample code, as the two seem to be in conflict with each other.

The most important thing about translating requirements like this to code, is to make sure that the code clearly and obviously implements the requirements.

Don't try and be clever, just be obvious. This means you're more likely to be correct, and you're definitely more maintainable as the requirements change.

For your case...

// Large repair orders for new customers should be closed
if (order.IsLargeOrder && order.OrderType == OrderType.Repair && order.IsNewCustomer)
{
    return OrderStatus.Closed;
}

// Large rush hire orders should always be closed
if (order.IsLargeOrder && order.IsRushOrder && order.OrderType == OrderType.Hire)
{
    return OrderStatus.Closed;
}

// Large repair orders always require authorisation
if (order.IsLargeOrder && order.OrderType == OrderType.Repair)
{
    return OrderStatus.AuthorisationRequired;
}

// All rush orders for new customers always require authorisation
if (order.IsRushOrder && order.IsNewCustomer)
{
    return OrderStatus.AuthorisationRequired;
}

// All other orders should be confirmed
return OrderType.Confirmed;

See how clearly the code matches the requirements, and how easy it is to find which bit of code to change if a requirement changes?

---

I'd say your problem isn't really related to the open/closed principle at all. Your version, with a hard-coded list of conditions, is no easier to modify than a method containing some if statements: it's a less obvious way of doing the same thing.

If you think about Martin's interpretation of the open/closed principle, at its heart it's about making sure that if you need to make modifications to a single requirement, you make them to a single place, and you don't need to hunt down and modify lots of dispirate bits of code.

For example, your use of an OrderStatus enum might in itself be a violation of the open/closed principle, depending on how it's used. If you have loads of separate bits of code which all do:

switch (orderStatus)
{
    case OrderStatus.Closed:
        // Do stuff
        break;
    ...
}

... then adding a new order status type would mean going and finding all of those switch statements and adding in the new condition, which would be a violation of the open/closed principle.

It's worth saying that you do need to take the open/closed principle with a pinch of salt. Meyer's version absolutely doesn't apply to languages like C#, where adding fields/methods isn't a binary breaking change. (Even if it did, to apply Meyer's open/closed principle to your code you'd make the GetOrderStatusTypeByOrderInput method virtual, and if requirements later changed you'd subclass OrderStatusAnalyzer and implement the new set of requirements there.)

The polymorphic interpretation doesn't apply to your case either: that says that interfaces are immutable ("closed to modification"), and that abstract base classes should be used instead. This used to apply to C#, where adding methods to an interface was a binary-breaking change (which is why things like SqlConnection are base classes rather than interfaces), but with DIM this no longer applies.

What's left for a language like C# is a version of Martin's interpretation, which is about making sure that if you need to make a targetted change, you don't need to touch dozens of different bits of code all over the codebase. Your requirements-implementing code itself doesn't violate this at all, as any requirements changes can be made by modifying a single method. Using the terms "open" and "closed" for this is a stretch, and I think people are just keen to keep using the term "SOLID" even as the original definition of the "open/closed principle" no longer applies to new languages, so they keep making up new rules which roughly (but not really) fit under the same name.

In fact, I'd go so far as to say your version with the OrderStatusCollection is actually more of a violation of the open/closed principle than the version in my answer! If you make any changes to OrderStatusCollection (such as adding a property, or changing a property type), you're also going to have to modify that horrendous linq query inside OrderStatusAnalyzer: you've gone from having one bit of code to modify, to having two bits! This is the thing that Martin's open/closed principle tells you to avoid.

---

With all of that said, the most important thing is to use common sense. SOLID are guiding principles, not strict laws. If you're actively making your code worse by trying to follow them religiously, you're doing something wrong.