Рефакторинг Ifs вложенных операторов в C #

Question

Я хотел бы сделать рефакторинг своего кода, чтобы сделать его более элегантным и понятным, если возможно уменьшить вложенность операторов if.

Мой код "анализирует" любую строку (результаты) процесса MsBuil для проверкипроцесс правильной сборки.

public static bool CheckMsBuildResult(string resultadoEjecucionScriptMsBuild)
        {
        // Build started 01/09/2010 8:54:07.
        string s1 = @"Build Started \d{1,2}/\d\d/\d{4} \d{1,2}:\d\d:\d\d";

        //Build started 9/1/2010 10:53:35 AM.
        //Build started 9/1/2010 8:42:16 AM.
        string s1Mod = @"Build Started \d{1,2}/\d{1,2}/\d{4} \d{1,2}:\d\d:\d\d";

        s1 = s1Mod;

        string s11 = @"n Generar iniciada a las \d{1,2}/\d\d/\d{4} \d{1,2}:\d\d:\d\d";

        // Compilaci�n iniciada a las 28/02/2011 14:56:55.
        string s12 = @"Compilaci.n iniciada a las \d{1,2}/\d\d/\d{4} \d{1,2}:\d\d:\d\d";


        string s2 = "Build succeeded.";
        string s21 = @"Generaci.n satisfactoria\.";
        string s3 = @"0 Error\(s\)";
        string s31 = "0 Errores";

        Regex rg = new Regex(s1, RegexOptions.Multiline | RegexOptions.IgnoreCase);
        Match mt = rg.Match(resultadoEjecucionScriptMsBuild);
        if (!mt.Success)
        {
            rg = new Regex(s11, RegexOptions.Multiline | RegexOptions.IgnoreCase);
            mt = rg.Match(resultadoEjecucionScriptMsBuild);
            if (!mt.Success)
            {
                rg = new Regex(s12, RegexOptions.Multiline | RegexOptions.IgnoreCase);
                mt = rg.Match(resultadoEjecucionScriptMsBuild);

                if (!mt.Success) return false;
            }
        }

        int i = mt.Index + mt.Length;

        rg = new Regex(s2, RegexOptions.Multiline | RegexOptions.IgnoreCase);
        mt = rg.Match(resultadoEjecucionScriptMsBuild, i);
        if (!mt.Success)
        {
            rg = new Regex(s21, RegexOptions.Multiline | RegexOptions.IgnoreCase);
            mt = rg.Match(resultadoEjecucionScriptMsBuild);
            if (!mt.Success)
            {
                return false;
            }
        }

        i = mt.Index + mt.Length;

        rg = new Regex(s3, RegexOptions.Multiline | RegexOptions.IgnoreCase);
        mt = rg.Match(resultadoEjecucionScriptMsBuild, i);
        if (!mt.Success)
        {
            rg = new Regex(s31, RegexOptions.Multiline | RegexOptions.IgnoreCase);
            mt = rg.Match(resultadoEjecucionScriptMsBuild);
            if (!mt.Success)
            {
                return false;
            }
        }

        return true;
        }

Answer 1

Похоже, что вы могли бы просто сократить это до одной или двух строк кода, используя один более мощный регулярное выражение вместо s1, s1mod, s12 и т. Д. Однако, поскольку вы отредактировали Ваши регулярные выражения Я не могу точно посоветовать, как это будет выглядеть.

Я бы тоже критиковал этот код за

• Неописуемые имена переменных.
• Ненужное повторное использование переменных
• Тиражирование
• Плохое имя метода (проверьте, что именно?)
• Потенциально низкая производительность, если этот метод вызывается часто из-за отсутствия повторного использования скомпилированных регулярных выражений.

Answer 2

Первые имена переменных рефакторинга.Затем попробуйте сгладить код, используя логические значения, как в следующем примере:

    Regex rg = new Regex(s1, RegexOptions.Multiline | RegexOptions.IgnoreCase);
    bool match_s1 = rg.Match(resultadoEjecucionScriptMsBuild).Success;        

    rg = new Regex(s11, RegexOptions.Multiline | RegexOptions.IgnoreCase);
    bool match_s11 = rg.Match(resultadoEjecucionScriptMsBuild).Success;         

    rg = new Regex(s12, RegexOptions.Multiline | RegexOptions.IgnoreCase);        
    bool match_s12 = rg.Match(resultadoEjecucionScriptMsBuild).Success;                 

    if (!match_s1 && !match_s11) 
        return match_s12; 

Answer 3

Вот две идеи:

• Похоже, вы пытаетесь закодировать дерево решений с использованием вложенного if. Лучшей альтернативой может быть определение структуры данных, которая кодирует дерево, а затем рекурсивно обходит дерево для проверки соответствия.

• Я не думаю, что существует какой-то действительно простой способ реорганизации кода, чтобы избежать вложения. Если вы достаточно смелы, вы можете использовать синтаксис LINQ. (В моей книге о функциональном программировании есть бесплатная глава , в которой объясняется, как работают монады и как писать подобные вещи с помощью LINQ).

Используя синтаксис LINQ, вы можете переписать это:

Regex rg = new Regex(s1, RegexOptions.Multiline | RegexOptions.IgnoreCase);
Match mt = rg.Match(resultadoEjecucionScriptMsBuild);
if (!mt.Success)
{
    rg = new Regex(s11, RegexOptions.Multiline | RegexOptions.IgnoreCase);
    mt = rg.Match(resultadoEjecucionScriptMsBuild);
    if (!mt.Success)
    {
        rg = new Regex(s12, RegexOptions.Multiline | RegexOptions.IgnoreCase);
        mt = rg.Match(resultadoEjecucionScriptMsBuild);
        if (!mt.Success) return false;
    }
}

... примерно так (обратите внимание, что вам нужно определить довольно много помощников, и я не показываю полный код - поэтому вы не можете сравнить длину - но вы можете видеть, что он избегает вложения):

var matches = 
    from rg1 in MatchRegex(s1, RegexOptions.Multiline | RegexOptions.IgnoreCase,
                          resultadoEjecucionScriptMsBuild)
    from rg2 in MatchRegex(s11, RegexOptions.Multiline | RegexOptions.IgnoreCase,
                          resultadoEjecucionScriptMsBuild)
    from rg3 in MatchRegex(s11, RegexOptions.Multiline | RegexOptions.IgnoreCase,
                          resultadoEjecucionScriptMsBuild)
    select true;

if (matches) return false;

Это может быть полезно, особенно если вам нужен доступ к некоторым из более ранних Match объектов, чтобы определить конечный результат (например, используйте rg1 в конце). В противном случае вы могли бы просто написать метод, который запускает заданное регулярное выражение и объединить их в один большой оператор if, используя &&.

Answer 4

Я думаю, что вы можете уменьшить количество повторяющегося кода здесь.

РЕДАКТИРОВАТЬ (Удалено еще несколько повторяющихся кодов и опубликован метод linq):

public static Match MatchOptions(string resultadoEjecucionScriptMsBuild, int index, params string[] strings)
{
    return strings.Select(s => new Regex(s, RegexOptions.Multiline | RegexOptions.IgnoreCase))
        .Select(rg => rg.Match(resultadoEjecucionScriptMsBuild, index)).FirstOrDefault(mt => mt != null);
}

или если вам не нравится linq:

public static Match MatchOptions(string resultadoEjecucionScriptMsBuild, int index, params string[] strings)
{
    foreach (string s in strings)
    {
        Regex rg = new Regex(s, RegexOptions.Multiline | RegexOptions.IgnoreCase);
        Match mt = rg.Match(resultadoEjecucionScriptMsBuild, index);

        if (mt != null)
            return mt;
    }

    return null;
}

public static bool CheckMsBuildResult(string resultadoEjecucionScriptMsBuild)
{
    string s1 = @" ..... ";
    string s1Mod = @" ..... ";
    string s11 = @" ..... ";

    // Compilaci�n iniciada a las 28/02/2011 14:56:55.
    string s12 = @" ..... ";

    string s2 = @" ..... ";
    string s21 = @" ..... ";
    string s3 = @" ..... ";
    string s31 = @" ..... ";

    var stringsArray = new[]
                        {
                            new[] {s1, s11, s12},
                            new[] {s2, s21},
                            new[] {s3, s31}
                        };

    Match mt = null;
    int i = 0;
    foreach (string[] strings in stringsArray)
    {
        mt = MatchOptions(resultadoEjecucionScriptMsBuild, i, strings);
        if (mt == null)
            return false;

        i = mt.Index + mt.Length;
    }

    return mt != null;
}

Answer 5

Кажется, что вы хотите проверить, если вход состоит из одного из s1|s11|s12, за которым следует один из s2|s21 и, наконец, один из s3|s31.

. Вы всегда можете сделать более сложную регулярнуювыражение, которое отражает это, используя:

var complex = "((" + s1 + ")|(" + s11 + ")|(" + s12 + "))|((" + s2 + ")|(" + s21 + ...

и используя это в одном совпадении.

Если вы просто хотите избавиться от вложенных ifs (т.е. если вы хотите рефакторинг, который поддерживаеттекущий поток управления) Я бы сгруппировал регулярные выражения в списки, такие как

var reList = new List<List<string>>() {
  new List<string>(){s1, s11, s12},
  new List<string>(){s2, s21},
  new List<string>(){s3, s31}
};

(используя инициализаторы коллекции C # 3.0)

Затем вы можете перебирать все выражения в двух вложенных циклах, таких как

var i = 0;
foreach (var outer in reList) {
  Match mt;
  foreach (var inner in outer) {
    rg = new Regex(inner, RegexOptions.Multiline | RegexOptions.IgnoreCase);
    mt = rg.Match(resultadoEjecucionScriptMsBuild, i);
    if (mt.Success)
      break;
  }
  if (!mt.Success)
    return false;
  i = mt.Index + mt.Length;
}
return true;

Эта функция возвращает true, если входная строка представляет собой конкатенацию одного из s1,s11,s12, одного из s2, s21 и, наконец, одного из s3, s31.