Home > Enterprise >  C# - Refactoring handling of business logic to be more generic, less tightly-coupled
C# - Refactoring handling of business logic to be more generic, less tightly-coupled

Time:01-26

I'm working on a little proof of concept project for handling and polling the status of transactions, and have had it pointed out that my current web of if statements works ok in the PoC stage, but won't hold up or be maintainable as the business logic becomes more complex or more statuses/permutations need to be added.

For a simple example of what I'm working with, here's a function that takes 4 bools as inputs, and different permutations of these will lead to different enum values being returned:

    public static TransactionStatus GetStatusForValues(bool isOverdue, bool isUrgent, bool isMember, bool isExisting)
    {
        if (isExisting)
        {
            if (isUrgent)
            {
                if (isMember)
                {
                    return TransactionStatus.Expidited;
                }

                return TransactionStatus.Declined;
            }

            if (!isUrgent)
            {
                if (isOverdue)
                {
                    return TransactionStatus.Active;
                }
            }
        }

        if (!isOverdue)
        {
            return TransactionStatus.Active;
        }

        return TransactionStatus.Confirmed;
    }

(Don't try and read anything into the domain logic here, this is purely made up rules for the benefit of the example)

Anyway, as you can see, this example with a relatively small number of input values is kind of convoluted already. Ideally, I would like to be able to implement something that allowed combinations of values to map to a result status, but without having to use a pile of if statements or a switch that could quickly become unmanageably huge.

An idea that's half-formed in my head is to be able to do some kind of rules engine approach, where individual rules could be passed in to represent the different permutations. I also wondered about being able to use something like C#'s Predicate class to be able to check multiple conditions.

Very keen to hear anyone and everyone's take on how the above could be refactored into something a bit more sustainable, appreciate any advice!

Thanks

CodePudding user response:

Sounds like you need a pipeline.
Each part of the pipeline can be separate and testable in isolation, but you can add whatever pipeline features are required.

interface IPipeline
{
    TransactionStatus Next(SomeData somedata);
}

class IsExistingAndUrgent : IPipeline
{
    public TransactionStatus Next(SomeData data)
    {
        if(data.IsExisting && data.IsUrgent)
        {
            if (isMember)
            {
                return TransactionStatus.Expidited;
            }

            return TransactionStatus.Declined;
        }
        else
        {
            Next(somedata);
        }
    }
}

class IsOverDue : IPipeline
{
    public TransactionStatus Next(SomeData somData)
    {
       if(!someData.!isOverdue)
       {
            return TransactionStatus.Active;
       }
       return TransactionStatus.Confirmed;
    }
}

CodePudding user response:

Just for funsies - this particular example can be rewritten to something like the following with switch expressions, value tuples and pattern matching:

static TransactionStatus GetStatusForValues(bool isOverdue, bool isUrgent, bool isMember, bool isExisting) =>
    (isExisting, isUrgent, isMember, isOverdue) switch
    {
        (isExisting: true, isUrgent: true, isMember: true, isOverdue: _) => TransactionStatus.Expidited,
        (isExisting: true, isUrgent: true, isMember: false, isOverdue: _) => TransactionStatus.Declined,
        (isExisting: true, isUrgent: false, isMember: _, isOverdue: true) => TransactionStatus.Active,
        (isExisting: _, isUrgent: _, isMember: _, isOverdue: false) => TransactionStatus.Active,
        _ => TransactionStatus.Confirmed
    };

But if number of switches is much bigger or switches will have more states I would consider using some kind of rule engine or state machine - for example based on stateless project.

CodePudding user response:

I think you could use replace your true/false properties with a [Flagged] enumerator.

[Flags]
public enum TransactionState
{
    Overdue = 1, 
    Urgent = 2, 
    Member = 4,  
    Existing = 8
};

which would allow you to write this instead

if (state == TransactionState.Existing & TransactionState.Urgent & TransactionState.Member)
    return TransactionStatus.Expidited;

CodePudding user response:

I tend to tackle these things by writing code that cade be written to be able to be modified at run-time. Then you can inject in new configurations from alternative classes, for from a database, JSON, or XML.

The basic starting point looks like this:

private static List<(bool? isOverdue, bool? isUrgent, bool? isMember, bool? isExisting, TransactionStatus transactionStatus)> transactionStatusMap = new()
{
    (null, true, true, true, TransactionStatus.Expidited),
    (null, true, false, true, TransactionStatus.Declined),
    (true, false, null, true, TransactionStatus.Active),
    (false, null, null, null, TransactionStatus.Active),
    (null, null, null, null, TransactionStatus.Confirmed),
};

By using bool? I have a third state of "don't care" for matching the bool.

So now the GetStatusForValues method can be written once and never need to be modified:

public static TransactionStatus GetStatusForValues(bool isOverdue, bool isUrgent, bool isMember, bool isExisting) =>
    transactionStatusMap
        .Where(x =>
            (x.isOverdue.HasValue ? isOverdue == x.isOverdue.Value : true)
            && (x.isUrgent.HasValue ? isUrgent == x.isUrgent.Value : true)
            && (x.isMember.HasValue ? isMember == x.isMember.Value : true)
            && (x.isExisting.HasValue ? isExisting == x.isExisting.Value : true))
        .Select(x => x.transactionStatus)
        .First();

Calling it doesn't change, so TransactionStatus ts = GetStatusForValues(true, true, false, true); works as expected and returns TransactionStatus.Declined.

I ran a test to ensure my mapping works:

isOverdue | isUrgent | isMember | isExisting | original  | mapped  
--------- | -------- | -------- | ---------- | --------- | ---------
True      | True     | True     | True       | Expidited | Expidited
True      | True     | True     | False      | Confirmed | Confirmed
True      | True     | False    | True       | Declined  | Declined
True      | True     | False    | False      | Confirmed | Confirmed
True      | False    | True     | True       | Active    | Active  
True      | False    | True     | False      | Confirmed | Confirmed
True      | False    | False    | True       | Active    | Active  
True      | False    | False    | False      | Confirmed | Confirmed
False     | True     | True     | True       | Expidited | Expidited
False     | True     | True     | False      | Active    | Active  
False     | True     | False    | True       | Declined  | Declined
False     | True     | False    | False      | Active    | Active  
False     | False    | True     | True       | Active    | Active  
False     | False    | True     | False      | Active    | Active  
False     | False    | False    | True       | Active    | Active  
False     | False    | False    | False      | Active    | Active  

If you wanted to go to an XML-based approach you could consider this XML:

<map>
  <if isExisting="true">
    <if isUrgent="true">
      <if isMember="true">
        <then transactionStatus="Expidited" />
      </if>
      <then transactionStatus="Declined" />
    </if>
    <if isUrgent="false">
      <if isOverdue="true">
        <then transactionStatus="Active" />
      </if>
    </if>
  </if>
  <if isOverdue="false">
    <then transactionStatus="Active" />
  </if>
  <then transactionStatus="Confirmed" />
</map>

Then populate transactionStatusMap like this:

transactionStatusMap =
    xdocument
        .Descendants("then")
        .Select(x => new
        {
            transactionStatus = Enum.Parse<TransactionStatus>(x.Attribute("transactionStatus").Value),
            booleans =
                x
                    .Ancestors()
                    .Where(a => a.Name == "if")
                    .SelectMany(
                        x => x.Attributes(),
                        (x, a) => new { name = a.Name.LocalName, value = (bool)a })
                            .Aggregate(new Dictionary<string, bool?>()
                            {
                                { "isOverdue", null },
                                { "isUrgent", null },
                                { "isMember", null },
                                { "isExisting", null },
                            }, (q, z) => { q[z.name] = z.value; return q; })
        })
        .Select(x => (isOverdue: x.booleans["isOverdue"], isUrgent: x.booleans["isUrgent"], isMember: x.booleans["isMember"], isExisting: x.booleans["isExisting"], transactionStatus: x.transactionStatus))
        .ToList();
  • Related