2011-03-01 99 views
0

我想重構我的代碼,使它更優雅,更清晰,如果可以減少嵌套「if」語句。重構如果嵌套語句在C#

我的代碼「解析器」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; 
     } 
+1

沒有什麼錯嵌套IFS幾級,只要你能理解發生的事情。現在你遇到的問題並不是你的ifs嵌套得太深,這是因爲非描述性變量名使得它很難理解ifs的不同嵌套中發生了什麼。 – Bazzz 2011-03-01 12:38:45

回答

5

它看起來像你可以只使用一個單一的功能更強大的正則表達式,而不是s1s1mods12,等等。然而,因爲你已經刪節的正則表達式的降低下來到代碼一行或兩行我不能確切地告訴你看起來會是什麼樣子。

我還要罵這個代碼

  • Undescriptive變量名。
  • 不必要的重新使用的變量
  • 複製
  • 可憐的方法名(檢查,究竟是什麼?)
  • 表現可能不好,如果這種方法被調用的時候,由於缺乏再利用編譯正則表達式的。
+0

同意,在決定返回false之前,OP應用2或3個不同的正則表達式。 1個「更好」的正則表達式的結果理論上可以決定返回什麼。 – Bazzz 2011-03-01 12:39:56

+0

thx,這將是更好的代碼(我的)不批評?? – Kiquenet 2011-03-01 14:55:03

1

我想你可以在這裏減少很多重複的代碼。

編輯(去掉了一些更多的重複代碼,併發布了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; 
} 
0

似乎要測試,如果輸入包括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,如果輸入〜應變g是s1,s11,s12之一,s2, s21之一以及最後一個s3, s31的串聯。

1

這裏有兩個想法:

  • 好像你正在嘗試使用嵌套if編碼decision tree。一個更好的選擇可能是定義一個數據結構來編碼樹,然後遞歸地遍歷樹來測試匹配。

  • 我不認爲有任何真正簡單的方法來重構代碼以避免嵌套。如果你足夠勇敢,你可以使用LINQ語法。 (我的functional programming book中有一個free chapter,它解釋了monads如何工作以及如何使用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語句中。

+0

提醒我說「如果你有一把錘子,一切看起來都像釘子一樣」:) ... – MartinStettner 2011-03-01 12:41:41

+0

@MartinStettner:你說的對,但編程語言不是這種情況嗎?如果你有一個裝滿錘子的工具箱,你必須選擇一把或另一把錘子...... – 2011-03-01 12:45:10

1

第一個重構變量名稱。然後使用布爾值,像這樣的樣本中儘量壓扁代碼:

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;