I have a function as follows. Inside ConditionFunc_A, B, C, D and etc (Up to 10 conditions) are fetching data from DB to check validations. Can anyone help me to identify a design pattern or a refactoring process.

public void InsertTransaction(){

if(ConditionFunc_A())
    InsertToPayment();
if(ConditionFunc_B())
    InsertToSuspendedAccount();
if(ConditionFunc_C())
    InsertToPayment();
if(ConditionFunc_D())
    InsertToSuspendedAccount();

// There are around 10 ConditionFunctions   }

CodePudding user response：

You can put these groups

if(ConditionFunc_A())
    InsertToPayment();

into separate functions. Like

function TryToInsertToPayment() {
    if(ConditionFunc_A()) {
        InsertToPayment();
    }
}

Then your main function will look like

TryToInsertToPayment();
TryToInsertToSuspendedAccount();
// All other inserts

BTW it seems to me, that conditions are exclusive - so if Condition_A is true, all others should be false. Else you would not have two InsertToPayment for different conditions. Is it correct?

In this case it would work even better - just unite several conditions in one if.

function TryToInsertToPayment() {
    if(ConditionFunc_A() || ConditionFunc_C() || ConditionFunc_E()) {
        InsertToPayment();
    }
}

And main function will contain just few calls - one per each object.

CodePudding user response：

Design is a matter of taste (at least to some extent).
If you have this pattern only in one place as you described, I think maybe a sequence of ifs like you did might be the way to go.

However - since as I understand it you asked for alternatives, you can see one below.

As I mentioned in my comment above, you can put the pairs of condition activity in an array or list. This can be useful when you have different combinations of these conditions and activities (especially if they can change dynamically).

namespace ConsoleApp1
{
    public delegate bool VoidToBool();
    public delegate void VoidToVoid();
    struct MyAction
    {
        public MyAction(VoidToBool cond, VoidToVoid activity) { Cnd = cond; Act = activity; }
        public VoidToBool Cnd { get; set; }
        public VoidToVoid Act { get; set; }
    };

    class Program
    {
        static bool ConditionFunc_A() { /*...*/ return true; }
        static bool ConditionFunc_B() { /*...*/ return true; }
        static bool ConditionFunc_C() { /*...*/ return true; }
        static bool ConditionFunc_D() { /*...*/ return true; }

        static void InsertToPayment() { /*...*/ }
        static void InsertToSuspendedAccount() { /*...*/ }

        static void Main(string[] args)
        {
            MyAction[] actions = new MyAction[] { new MyAction(ConditionFunc_A, InsertToPayment),
                                                  new MyAction(ConditionFunc_B, InsertToSuspendedAccount),
                                                  new MyAction(ConditionFunc_C, InsertToPayment),
                                                  new MyAction(ConditionFunc_D, InsertToSuspendedAccount) };

            foreach (var action in actions)
            {
                if (action.Cnd()) {
                    action.Act();
                    // break;  // NOTE: enable this line if you want to stop once the first condition was met.
                }
            }
        }
    }
}

CodePudding user response：

public class Rule
{
    private readonly Func<bool> _canApply;
    private readonly Action _applyAction;

    public Rule(Func<bool> canApply, Action applyAction) => (_canApply, _applyAction) = (canApply, applyAction);
    public bool CanApply() => _canApply();
    public void Apply() => _applyAction();
}


public class TransactionManager
{
    private readonly List<Rule> _insertionRules;
    public TransactionManager() => _insertionRules = new()
    {
        new (ConditionFunc_A, InsertToPayment),
        new (ConditionFunc_B, InsertToSuspendedAccount),
        //...
    };

    public void AddInsertionRule(Rule rule) => _insertionRules.Add(rule);
    public void InsertTransaction()
    {
        foreach (var rule in _insertionRules)
            if (rule.CanApply())
                rule.Apply();
    }

    private bool ConditionFunc_A() => true;
    private bool ConditionFunc_B() => true;

    private void InsertToPayment() { }
    private void InsertToSuspendedAccount() { }
}

Note the public method AddInsertionRule to not violate the open-closed principle (i.e. you can add rules (from outside) without modifying the TransactionManager)