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 bool
s 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();