Design Pattern for Classifying a set of Data

Question

I am working on some legacy code that involves Orders placed by Customers. There is a new requirement to classify each Order by Business Unit. The Business Unit Name will be stored in a new column on the Order table.

The Order is not an actual object. I have an Order Number, which I can use to retrieve two different XML documents from a database. Well, one XML document that represents the entire Order, and one XML document that represents a Product Definition, which is retrieved using a Product ID from the Order XML Document. The Product XML documents all conform to the same schema.

I have a set of rules to apply on values obtained from the XML documents to determine the Business Unit classification.

I am trying to avoid using a lot of if/else statements because it seems messy and not very maintainable. All I need is a string containing the Business Unit Name. The method needs to be called in two separate flows, so I have put it in a class.

Here is what the code looks like to return the Business Unit Name:

public class BusinessUnitName
{

    public static string GetBusinessUnitName(string orderNumber)
    {
        var order = getOrder(orderNumber);
        var orderType = order.SelectSingleNode("/Order/Type").InnerText;

        if (orderType.Equals("Remake"))
        {
            var remakeReason = order.SelectSingleNode("/Order/RemakeReason").InnerText;
            if (remakeReason.Equals("SHIPDMG") || remakeReason.Equals("WRNGITM"))
            {
                return "Shipping";
            }
            else
            {
                return "Customer Service";
            }
        }
        else if (orderType.Equals("Design"))
        {
            List<string> illustrationProductTypes = new List<string> { "1204", "1205", "1206", "1207" };
            List<string> customProductTypes = new List<string> { "9204", "9205", "9206", "9207" };

            var firstProductId = order.SelectSingleNode("/Order/Products/Product[1]/Id").InnerText;
            var firstProductDefinition = getProductDefinition(firstProductId);
            var firstProductType = firstProductDefinition.SelectSingleNode("/Product/Type").InnerText;

            if (illustrationProductTypes.Contains(firstProductType))
            {
                return "Design-Illustration";
            }
            else if (customProductTypes.Contains(firstProductType))
            {
                return "Design-Custom";
            }
            else
            {
                return "Design-Other";
            }                
        }
        else if (orderType.Equals("Samples"))
        {
            var firstProductId = order.SelectSingleNode("/Order/Products/Product[1]/Id").InnerText;
            var firstProductDefinition = getProductDefinition(firstProductId);
            var departmentCode = firstProductDefinition.SelectSingleNode("/Product/DepartmentCode").InnerText;
            if (departmentCode.Equals("JWL"))
            {
                return "Jewelry";
            }
            else if (departmentCode.Equals("SPPLS"))
            {
                return "Craft Supplies";
            }
        }
    }

    private static XmlDocument getOrder(string orderNumber)
    {
        XmlDocument order = new XmlDocument();
        /* execute code to retrieve XML from database 
           and load into order */
        return order;
    }

    private static XmlDocument getProductDefinition(string productId)
    {
        XmlDocument productDefinition = new XmlDocument();
        /* execute code to retrieve XML from database 
           and load into productDefinitionXmlDocument */
        return productDefinition;
    }
}

This is pretty similar to the actual code, but the list of if/else statements is about three times as long. Each rule just consists of a combination of values in XML documents.

I do not know if I am on the right path, but I was considering using the Strategy pattern. I could create an IBusinessUnitStrategy interface with an ExecuteRules method that returns a bool value indicating if a combination of rules is a match, and a field indicating the Business Unit Name. I would then need to implement the IBusinessUnitStrategy for each Business Unit Name.

I would need another class that generates a list of IBusinessUnitStrategy instances for each concrete Business Unit class, then calls the ExecuteRules method sequentially on each Business Unit class until it one of them returns true, and then return the value of the Business Unit Name field for that class.

Would this approach make sense/be somewhat along a better path design-wise, or is it overkill?

Answer 1

As others have suggested in comments, one way to go, if possible, is to change the data model so that it makes processing easier, but you may not be able to do that.

So I'm going to suggest a way to deal with that complexity. First, create a helper method that turns your order (an XmlDocument) into a suitable data structure - just collect the properties you need.

// Think of this as being a data bag
public class OrderData   // not a great name, I know 
{
    public int OrderNumber { get; set; }
    public string OrderType { get; set; }
    public string RemakeReason { get; set; }

    // you may want to include properties from a related table,  
    // and possibly populate these in advance
    public string FirstProductType { get; set; }  

    //...
}

Basically, take all the code that you already use to extract these values, and move it to that helper method, but instead of having a bunch of local variables, populate the properties of an OrderData instance.

private OrderData getOrderData(XmlDocument order) { /* ... */ }

Now, the approach described below is not particularly complicated, but it does incorporate ideas from a couple of patterns, so bear with me.

What you want is a method that takes in an OrderData, and returns a classifying string (Business Unit Name), but you want it to be flexible and maintainable. The idea behind my suggestion is that along with the OrderData, you pass in an object (or just a delegate) that represents a collection of classification rules (I'll refer to these as "classifiers" from now on). So in one sense, its like the Strategy pattern. But in another sense, it also acts like the Composite pattern, because you would have a collection of classifiers that would itself act as a classifier - it would have the same interface (or the same signature, if it's a delegate) as each of its children. And then you'd have a number of individual classifiers. I.e., you could have one that recognizes "Shipping", another that recognizes "Customer Service", and another that recognizes "Design-Illustration", and so on. (Note that you can group some of these under the same classifier, if you feel that doing so makes sense. E.g. you can think of a classifier operating on the level of the Order Type, but you can do it in a more granular way if you want.)

Then you would simply pass the OrderData object to the composite classifier, which would in turn pass it to its children, one by one, until there's a match, or until no match is found. So in that regard, it acts somewhat like the Chain of Responsibility pattern, but it has more structure and a more specific interface because it's really a Composite (at least, that's how I conceptualized it here).

So the main idea is to isolate the classification logic into these separate classifiers, and to separate the responsibilities of performing the actual classification (classifiers), managing the overall flow (composite classifier), and deciding which classification rules to use (the code that calls getCompositeClassifier).

I'll use a delegate-based example for simplicity, but if you want to use full blown objects, and feel that it's not an overkill, or that pros outweigh the cons, go for it.

public string GetBusinessUnitName(string orderNumber)
{
    // NOTE: "classifier" here is used in the sense of "classification rule".
    // Create the composite classifier from the available individual
    // classifiers, in a way that meets your requirements. 
    // Note that you can easily add or remove individual classifiers 
    // without worrying (too much) how they are implemented
    var compositeClassifier = getCompositeClassifier(new List<Func<OrderData, string>> {
        classifyRemakeOrder, 
        classifyDesignOrder, 
        classifySamplesOrder
        });

    var orderData = getOrderData(getOrder(orderNumber));
    return compositeClassifier(orderData);
}

// This "constructs" the composite classifier;
// this code probably won't have to change as much or as often (or at all).
// Also, you don't want to call this redundantly; create it once use that instance.
// This and the individual classifier methods could be in the same file, 
// or someplace else in your codebase, depending on how your code is organized 
// (this will also determine if these methods should be public, internal, or private).
private Func<OrderData, string> getCompositeClassifier(
    IEnumerable<Func<OrderData, string>>  children)
{
    Func<OrderData, string> compositeClassifier = (orderData) => {
        foreach(var classifier in children) 
        {
            var buName = classifier(orderData);
            if (buName != null)
                return buName;
        }

        // I'm using null to indicate that classification was not possible within the context of this classifier
        // You can use some other convention
        return null;
    };

    // Note that this method returns a delegate
    return compositeClassifier;
}

// Then for each new classifier, just add a new method with the appropriate signature
private string classifyRemakeOrder(OrderData orderData)
{
     if (orderData.RemakeReason.Equals("SHIPDMG") || orderData.RemakeReason.Equals("WRNGITM"))
        {
            return "Shipping";
        }
        else
        {
            return "Customer Service";
        }

        // I'm using null to indicate that classification was not possible within the context of this classifier
        // You can use some other convention
        return null;  
}

private string classifyDesignOrder(OrderData orderData) 
{ /* ... */ }

private string classifySamplesOrder(OrderData orderData) 
{ /* ... */ }

Depending on how exactly you implement the classifiers, and on the complexity of the logic involved, you may have some redundancy in your checks, but that may be perfectly acceptable - e.g. it may not affect performance or maintainability in any meaningful way.

Another thing you can do, once you have this kind of design in place, is to leverage LINQ (if you have to process a bunch of orders at once). Get all the XML representations of all the orders you need in one go, and use orderCollection.Select(getOrderData).Select(compositeClassifier) to turn the whole thing into a collection of business unit names.
If you need both the order data and the associated business unit name, you can use an anonymous class to keep them both throughout the LINQ query, or leverage the OrderData class itself - add the BusinessUnitName property, and modify the classifiers so that they set that property.

As for the fact that sometimes you need to retrieve additional data, like the product definitions, experiment with getting all that data in one go, in advance - it may work well. You may go about it in different ways - you could retrieve all the additional data regardless of if its actually needed, or have some more complex logic that will do it in a smarter way (but don't go for the latter unless there's a good reason for it; as long as it works well, simpler is usually better).
There also may be things you can do on the database side; you may be able to leverage the database to do the hard work of joining data together, so look into that as well.