How to evaluate multiple boolean and decimal comparisons in a large list of data?

Question

I have a design problem, and no-one to help me thrash it out. Except for you.

The system handles data files. Each row can be compared against a number of criteria, chosen by the user. The row is then true or false for that criteria.

Each criteria also has a price and a cost (and a margin: price - cost). However, the user is only charged if a row matches one of the criteria. And they only pay for one of the matched criteria: the one used should be the one with the highest margin.

Structurally, it looks like this in pseudo code:

SystemRow
{
    bool Criteria1 { get; set; }
    bool Criteria2 { get; set; } //etc
}

There's a separate DTO which holds a user's selected criteria as a bitwise Enum. It's passed into a row processing function along with the row.

ProcessRow(selectedCriteria, row)
{
    if(selectedCriteria.HasFlag(CriteriaEnum.Criteria1)
    {
        // this sets row.Critera1 to true if it matches conditions
        EvaluateRowForCriteria1(row);
    }
    // then evaluate other criteria one by one
}

CalculatePrice(ListOfRows)
{
    decimal runningTotal;
    foreach(row in ListOfRows)
    {
       runningTotal += [the cost of one - and only one - matching criteria]
    }
}

Each of these functions exists in a separate class, in accordance with SRP.

I can think of several ways to do this, but each is messy: hard to maintain and inefficient.

First, I can add a "Price" and "Margin" property to "Row". Then, in the class where "ProcessRow" lives I can pull all the prices and margins from the DB and store them in local variables. During "ProcessRow" I can compare the Margin of a criteria with the Marign of the row and, if better, assign the price. Then I can sum the prices in CalculatePrice.

Second, I can fetch the Price and Margin in CalculatePrice instead. Then, for each row, I can use the selected CriteriaX on the row, find the correct one to charge, and add it to the running total.

What makes this such a mess is that there's about 20 of these criteria. So either way there's going to have to be a ton of variables kicking around if I don't want to have to go to the Db to get the values on each row.

Is there a better design I can leverage here?

Your code does not make sense to me compared to your words. If a row has bool criteria then why are you doing a string comparison? "Compare the Margin of a criteria with the Marign of the row and, if better, assign the price" is not clear to me. You need to more clearly define how a margin is calculated to get a proper answer. — paparazzo
– paparazzo, Commented Mar 30, 2017 at 18:53
Confusion. Is a criteria an object with price and cost or is it a boolean? — radarbob
– radarbob, Commented Mar 30, 2017 at 19:01
@radarbob: currently, it seems the criteria are not modeled as objects, that is exactly what is missing here to clean up the mess described by the OP. See my answer. — Doc Brown
– Doc Brown, Commented Mar 30, 2017 at 19:30
@Paparazzi Sorry for the confusion - it's not a string comparison. That's just a selector to see which services have been chosen. The actual comparison that checks against a given critera is irrelevant, so I left it out of the question. Have edited, hopefully to make it clearer. — Bob Tway
– Bob Tway, Commented Mar 31, 2017 at 8:34

Christophe · Accepted Answer · 2017-03-30 18:05:20Z

Analysis of your approach

Current situation:

you don't know in advance the criteria desired by the user;
you keep the criteria to be used for a query in a container;
you iterate through all the rows;
for each row you look for all matching criteria;
your issue is to price each row by maximizing the margin;

Your current options are:

option A: price the row in the matching process, changing price if there's a better margin for another matching criteria;
option B: price the row at summation, keeping track of the best criteria.

As you have more than 20 criteria, both of your solution imply checking all of the matching criteria. In the worst case you'd try to match all 20 conditions: that's 19 too much !

Better alternative

Instead of asking yourself which is the most profitable criteria, and this for every row, I propose you to:

first order your container of selection criteria requested by the user, by decreasing order of profitability.
then for a given row, try successively each criteria IN THE RIGHT ORDER. So when a criteria matches, it's the most profitable and you no longer need to match the remaining criteria for the same line.
you can calculate the price when processing the line, or when making the summation: in any case you've only one price to use.

This approach considerably decreases the matching time for successful rows. The performance is unchanged for unselected rows.

Thanks for this. Feel foolish not to have spotted this for myself. The switch to a dynamic price was a late addition, otherwise I might have followed this approach from the first. — Bob Tway
– Bob Tway, Commented Mar 31, 2017 at 8:36

Doc Brown · Accepted Answer · 2017-03-30 18:53:40Z

1

Model your criteria as a class hierarchy, not just as a list of strings: derive each criteria class from a common interface or abstract base class:

  interface ICriteria
  { 
      bool DoesItMatch(Row row);
      double Price{get;}
      double Cost{get;}
  }

Now all you need is a factory class which creates a list of concrete ICriteria objects from the user's selection (which I assume will query your db just once). After you have this list, order it in descending order by the margin. Then the cost evaluation will be as simple as

double CalculateHighestMargin(List<ICriteria> orderedCriterias, row)
{
     foreach(var crit in orderedCriterias)
     {
        if(crit.DoesItMatch(row))
            return crit.Price - crit.Cost;
     }
     return 0;
}

Note there is no need to store the boolean values for the rows in each row object, or the price and cost of each row (at least not for the requirements described in the question).

answered Mar 30, 2017 at 18:53

Doc Brown

221k35 gold badges411 silver badges625 bronze badges

Thanks for this. It's a great solution but I may not be able to leverage it, as I've had an object model imposed on me that I'm not sure I can change.

Bob Tway
– Bob Tway

2017-03-31 08:35:12 +00:00
Commented Mar 31, 2017 at 8:35
1

@MattThrower: I don't know which idiot in your team is forbidding you to write code, but this is not about changing anything like a persistent object model, it is about adding some helper classes to organize your functions in a better way. But if you are not allowed to design such simple code structures, you should probably look for a different employer.

Doc Brown
– Doc Brown

2017-03-31 11:10:26 +00:00
Commented Mar 31, 2017 at 11:10
+1 Dr. Brown comment. Rather than change jobs, wrap the model objects you're stuck with new objects (classes) that do the right thing. You'll want to expose the existing API for compatibility, and have new methods as needed. You may want factory/builder/whatever class(es) to ensure proper construction and better keep your "working classes" decoupled. The idea is "adapter"

radarbob
– radarbob

2017-04-12 19:24:37 +00:00
Commented Apr 12, 2017 at 19:24

Add a comment |

Stack Exchange Network

How to evaluate multiple boolean and decimal comparisons in a large list of data?

2 Answers 2

Your Answer

Hot Network Questions

How to evaluate multiple boolean and decimal comparisons in a large list of data?

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions