2

I need a method to effectively search strings for specific patterns, and changing the string based on the pattern that is found. Right now, my solution works, but I'm not happy with it at all due to a high cyclomatic complexity. What I've got is like this:

foreach (string test in string[] strings)
{
    if (test.Contains("one")
    {
        test = ExecuteCodeOne(test);
    }
    if (test.Contains("two")
    {
        test = ExecuteCodeTwo(test);
    }
    if (test.Contains("three")
    {
        test = ExecuteCodeThree(test);
    }
    if (test.Contains("four")
    {
        test = ExecuteCodeFour(test);
    }
    if (test.Contains("five")
    {
        test = ExecuteCodeFive(test);
    }
}

Each of the ExecuteCode methods executes different code, with one thing in common: they all call a version of test.Replace("some value", "some other value"). As you can see, this doesn't look ideal. I'd like to refactor this to something better. My question is: is there a design pattern I'm missing and could use to reach a lower complexity and hopefully lower execution time?

EDIT: An example of the ExecuteCode methods:

private static string ExecuteCodeOne(string test)
{
    return test.Replace("value1", "newValue1");
}

private static string ExecuteCodeTwo(string test)
{
    if (test.Contains("Value2"))
    {
        test = test.Replace(new Regex(@"Regex1").Match(test).Groups[1].Value, "");
        test = test.Replace("Value2", "");
    }
    else if (test.Contains("Value3"))
        test = test.Replace("Value3", "newValue3");

    return test;
}

So, the private methods do vastly differing things, including their own checks, but in practice always leading to a form of String.Replace().

StefRoo
  • 21
  • 3
  • Can you clarify what do the different functions (ExecuteCode{number}) do? They only do a replace or they do more things? – Th0rndike Dec 11 '18 at 14:38
  • Added an Edit! They always lead to a replace and then return, but due to their own checks the string might not always get modified. – StefRoo Dec 11 '18 at 14:59

1 Answers1

0

Since this is all made with many single "if" blocks, I suppose all cases can happen at the same time. Another thing to consider is that all functions have basically different logic in them, so it's hard to find common ground to refactor this.

The best idea I have so far is to use a dictionary that contains the word to match and the function to execute, this would make it (sort of) scalable and the code would be simpler:

 Dictionary<string, Func<string,string>> myDict = new Dictionary<string, Func<string, string>>()
            {
                {"one", ExecuteCodeOne },
                {"two", ExecuteCodeTwo },
                {"three", ExecuteCodeThree },
                {"four", ExecuteCodeFour },
                {"five", ExecuteCodeFive },
            };

Then you can loop the dictionary and apply all cases:

List<string> toReplaceList = new List<string>();
            List<string> result = new List<string>();
            foreach(string test in toReplaceList)
            {
                string temp = test;
                foreach (KeyValuePair<string, string> kv in myDict)
                {
                    if (temp.Contains(kv.Key))
                        temp = kv.Value(temp);
                }

                result.Add(temp);
            }

Not great, but still better. Hope it helps.

Th0rndike
  • 3,406
  • 3
  • 22
  • 42