Identify fraudulent bank transactions

Question

Task: Return true if a transaction is allowed and return false if transaction is prohibited.

Details: 1. String is represented as an "array" that can contain up to three elements.

We need to evaluate a charge against a list of rules and see if we can allow the transaction to happen.

Each rule (Allow or Deny) can contain up to two statements divided by "AND" or "OR"

The following comparison operators for rules are valid:

 ">", "<", ">=", "<=", "==", "!=" 

The rest of the rules can be made up.

Example Input (Complex):

"['CHARGE: card_country = US & currency = USD & amount = 600 &
ip_country = CA','ALLOW: amount > 500 AND currency!=EUR', 'DENY:
card_country == AUS]"

Outcome: True (because 600>500 and USD!=EUR and USD!=AUS)
Example Input 2 (Simple):

"['CHARGE: card_country = US & currency = USD & amount = 200 &
ip_country = CA','ALLOW: amount > 500"

Outcome: False (Because 200<500)

---

My Implementation and questions.

1. I struggled to figure out how to efficiently parse this string in multiple formats, so I mostly used String.Split and Trim(). Not sure if should've used regex instead?

2. I'm not sure if I structured the code correctly? I do have some classes, but I'm not sure if I even need "Rule" and "RuleSet" is there an easier way?

3. Major issue - it's hard to figure out how to interpret comparators from a string into actual code. What technique is usually used to work with dynamic comparisons?

4. Probably will fail on a lot of cases at this point. How to correctly capture exceptions and test? (I cannot wrap everything in try catch, I do not think.)

5. Charge object is a parameter name and value, not static fields. Should I use fields "currency", "amount", etc. for Charge class?

6. Overall, the code looks too complex and hard to follow. Any suggestions on how to improve?

Radar.cs

public class Radar
{
    private readonly List<string> cleanUpValues = new List<string>() { "'", "]", "["};
    private readonly List<string> operators = new List<string>() { ">", "<", ">=", "<=", "==", "!=" };
    public bool CheckTransaction(string input)
    {
        input = Utils.RemoveStrings(input, cleanUpValues);
        var result = input.Split(',');
        if (result.Count()<=1)
        {
            throw new ArgumentOutOfRangeException("input", "radar string has invalid format");
        }
        var charge = new Charge();
        RuleSet allowRules = null;
        RuleSet denyRules = null;
        foreach (var res in result)
        {
            if (res.TrimStart().StartsWith("CHARGE:"))
            {
                charge = CreateCharge(res.Replace("CHARGE:",""));
            }
            else if (res.TrimStart().StartsWith("ALLOW:"))
            {
                allowRules = CreateRuleSet(res.Replace("ALLOW:", ""), true);
            }
            else if (res.TrimStart().StartsWith("DENY:"))
            {
                denyRules = CreateRuleSet(res.Replace("DENY:",""), false);
            }
        }
        var chargeString = result[0];
        var ruleString = result[1];
        return EvaluateRulesAgainstCharge(charge, allowRules, denyRules);
    }

    public Charge CreateCharge(string chargeString)
    {
       Charge charge = new Charge();
       var splitParameters =  chargeString.Split('&');
        foreach (var parameter in splitParameters)
        {
            var param = parameter.Split('=');
            var value = param[1].Trim();
            charge.Parameters.Add(new ChargeParameter(param[0].Trim(), param[1].Trim()));
        }

        return charge;
    }
    public RuleSet CreateRuleSet(string ruleString, bool IsAllowed)
    {
        var rules = new List<string>();
        bool andOperator = false;
        if (ruleString.Contains("AND"))
        {
            rules = ruleString.Split(new string[] {"AND"}, StringSplitOptions.RemoveEmptyEntries).ToList();
            andOperator = true;
        }
        else if (ruleString.Contains("OR"))
        {
            rules = ruleString.Split(new string[] {"OR"}, StringSplitOptions.RemoveEmptyEntries).ToList();
        }
        else
        {
            rules.Add(ruleString);
        }

        var ruleSet = new RuleSet(IsAllowed, andOperator);
        foreach (var rule in rules)
        {
            foreach (var op in operators)
            {
                if (rule.Contains(op))
                {
                    var ruleParams = rule.Split(new string[] { op }, StringSplitOptions.RemoveEmptyEntries);
                    Rule r = new Rule(op, ruleParams[0].Trim(), ruleParams[1].Trim());
                    ruleSet.Rules.Add(r);
                }

            }
        }
        return ruleSet;
    }

    public bool EvaluateRulesAgainstCharge(Charge charge, RuleSet allowRules, RuleSet denyRules)
    {
        bool AllowTransaction = true;
        if (allowRules != null)
        {
            foreach (var rule in allowRules.Rules)
            {
                var chargeParam = charge.Parameters.First(p => p.ParameterName == rule.ParameterName);
                if (rule.ParameterName == "amount")
                {
                    AllowTransaction = AllowTransaction && Utils.Compare<double>(rule.Operator, Double.Parse(chargeParam.ParameterValue), Double.Parse(rule.Value));
                }
                else
                {
                    AllowTransaction = AllowTransaction && Utils.Compare<string>(rule.Operator, chargeParam.ParameterValue, rule.Value);
                }

                if (!AllowTransaction)
                    break;
            }
        }
        if (denyRules != null)
        {
            foreach (var rule in denyRules.Rules)
            {
                var chargeParam = charge.Parameters.First(p => p.ParameterName == rule.ParameterName);
                if (rule.ParameterName == "amount")
                {
                    AllowTransaction = AllowTransaction && !Utils.Compare<double>(rule.Operator, Double.Parse(chargeParam.ParameterValue), Double.Parse(rule.Value));
                }
                else
                {
                    AllowTransaction = AllowTransaction && !Utils.Compare<string>(rule.Operator, chargeParam.ParameterValue, rule.Value);
                }

                if (!AllowTransaction)
                    break;
            }
        }
        return AllowTransaction;
    }

}   

Charge.cs

public class Charge
{
    private List<ChargeParameter> parameters = new List<ChargeParameter>();
    public List<ChargeParameter> Parameters { get => parameters; set => parameters = value; }
}

ChargeParameter.cs

public class ChargeParameter
{
    public string ParameterName { get; private set; }
    public string ParameterValue { get; private set; }
    public ChargeParameter(string parameterName, string parameterValue)
    {
        ParameterName = parameterName;
        ParameterValue = parameterValue;
    }
}   

Rule.cs

public class Rule
{
    public string Operator { get; private set; }
    public string ParameterName { get; private set; }
    public string Value { get; private set; }

    public Rule(string op, string parameterName, string value)
    {
        Operator = op;
        ParameterName = parameterName;
        Value = value;
    }
}

RuleSet.cs

public class RuleSet
{
    public bool IsAllow { get; private set; }
    public List<Rule> Rules { get; private set; }
    public bool AndOperator { get; private set; }

    public RuleSet(bool isAllow, bool andOperator)
    {
        IsAllow = isAllow;
        AndOperator = andOperator;
        Rules = new List<Rule>();
    }
    public void AddRule(Rule rule)
    {
        Rules.Add(rule);
    }
}

Utils.cs

public static class Utils
{
    public static string RemoveStrings(string input, List<string> charsToRemove)
    {

        foreach (var c in charsToRemove)
        {
            input = input.Replace(c, string.Empty);
        }
        return input;
    }
    public static bool Compare<T>(string op, T left, T right) where T : IComparable<T>
    {
        switch (op)
        {
            case "<": return left.CompareTo(right) < 0;
            case ">": return left.CompareTo(right) > 0;
            case "<=": return left.CompareTo(right) <= 0;
            case ">=": return left.CompareTo(right) >= 0;
            case "==": return left.Equals(right);
            case "!=": return !left.Equals(right);
            default: throw new ArgumentException("Invalid comparison operator: {0}", op);
        }
    }
}

Answer 1

I'm not sure how to answer all your questions but at least I'll provide my two cents :)

1. Don't use regex unless you really need to. Regex is notorious for its slow performance on large strings.
2. I was thinking about some rule engine before I got to your code. So I think your class structure is perfectly fine.

---

Regarding the code readability: - You could rename Utils.RemoveStrings to Utils.SanitizeInput or something in order to better express the intention behind your method. Also, you can move cleanUpValues inside in order to reduce the number of parameters. - Charge parameters can be converted to auto-property

public List<ChargeParameter> Parameters { get; } = new List<ChargeParameter>();

• Please pay attention there is some dead code.
• Many methods can be made private
  • No need to use && operator for your evaluation. It can be rewritten like this.

    private bool EvaluateRulesAgainstCharge(Charge charge, RuleSet allowRules, RuleSet denyRules)
    {
        var allowTransaction = CheckRules(charge, allowRules, true);

        if (!allowTransaction)
            return false;

        allowTransaction = CheckRules(charge, denyRules, false);
        return allowTransaction;
    }

    private static bool CheckRules(Charge charge, RuleSet rules, bool expectedResult)
    {
        var allowTransaction = true;
        if (rules != null)
        {
            foreach (var rule in rules.Rules)
            {
                var chargeParam = charge.Parameters.First(p => p.ParameterName == rule.ParameterName);
                if (rule.ParameterName == "amount")
                {
                    allowTransaction = Utils.Compare(rule.Operator, double.Parse(chargeParam.ParameterValue),
                        double.Parse(rule.Value)) == expectedResult;
                }
                else
                {
                    allowTransaction = Utils.Compare(rule.Operator, chargeParam.ParameterValue, rule.Value) == expectedResult;
                }

                if (!allowTransaction)
                    break;
            }
        }

        return allowTransaction;
    }
```