2017-07-31 22 views
0

我只是想知道什么是最好的方法来重构这个声明使用较少的condidtions?林reallt strucggling清理这一说法,而不会具有相同的功能,如果有人为可以点我在正确的方向,我会很gratefull重构代码使用较少的语句

try 
{ 
    var errorProviders = new List<ErrorProvider>() { epEmail, epAlternative, epMobile, epTown, epLandline, epHouseName, epForeName, epSurname, epPostcode, epCountry, epHouseName, epLocality, epCounty }; 

    foreach (Control c in panel1.Controls) 
    { 
     if (c is SpellBox || c is TextBox) 
     { 
      if (!string.IsNullOrWhiteSpace(txt_ForeName.Text) | !string.IsNullOrWhiteSpace(txt_SurName.Text)) 
      { 
       if (cmb_Title.SelectedIndex != -1) 
       { 
        if (cmb_PrefConTime.SelectedIndex != -1) 
        { 
         if (isPhoneNumber()) 
         { 
          if (errorProviders.Any(e => e.GetError(c).Length > 0)) 
          { 
           return false; 
          } 
         } 
         else 
         { 
          epPrefConNumber.SetError(cmb_PrefConNumber, "Error"); 
          return false; 
         } 
        } 
        else 
        { 
         epPrefConTime.SetError(cmb_PrefConTime, "Error in: prefered contact time feild"); 
         return false; 
        } 
       } 
       else 
       { 
        epTitle.SetError(cmb_Title, "Title"); 
        return false; 
       } 
      } 
      else 
      { 
       epBothNames.SetError(txt_SurName, "Error:"); 
       epBothNames.SetError(txt_ForeName, "Error:"); 
       return false; 
      } 
     } 
    } 
} 
catch (Exception ex) 
{ 
    MessageBox.Show(ex.ToString())+ "Error has occurred, Please cancel and try again!"); 
} 
return true; 

如果使用裸minumum条件,以减少代码的方法吗?

+0

我主要关注的是,你只在实际情况中在一个地方使用'c'。为什么每次都要评估所有这些东西,而不是一次? (我也强烈建议不要在“在一行之后”格式的代码,但这是不同的事情......) –

+0

@JonSkeet它实际上被使用了三次。第一次使用它的foreach ... –

+0

@MaxPlay:对不起,是的 - 我实际上意味着在该顶级如果,而不是foreach。我的错。 (但是在测试类型和调用'Any(e => e.GetError(c))'之间的所有内容都不涉及'c'。) –

回答

1

如果你必须检查所有这些条件,那么你必须检查所有这些条件。在我看来,你最令人震惊的问题是与你的循环内部的控制完全无关的深层嵌套和验证字段。您可以通过逆转如果条件并尽早返回来修复嵌套。对于其余部分,只需将不相关的验证移到循环外。

try 
{ 
    var errorProviders = new List<ErrorProvider>() { epEmail, epAlternative, epMobile, epTown, epLandline, epHouseName, epForeName, epSurname, epPostcode, epCountry, epHouseName, epLocality, epCounty }; 

    if (string.IsNullOrWhiteSpace(txt_ForeName.Text) && string.IsNullOrWhiteSpace(txt_SurName.Text)) 
    { 
     epBothNames.SetError(txt_SurName, "Error:"); 
     epBothNames.SetError(txt_ForeName, "Error:"); 
     return false; 
    } 

    if (cmb_Title.SelectedIndex == -1) 
    { 
     epTitle.SetError(cmb_Title, "Title"); 
     return false; 
    } 

    if (cmb_PrefConTime.SelectedIndex == -1) 
    { 
     epPrefConTime.SetError(cmb_PrefConTime, "Error in: prefered contact time feild"); 
     return false; 
    } 

    if (!isPhoneNumber()) 
    { 
     epPrefConNumber.SetError(cmb_PrefConNumber, "Error"); 
     return false; 
    } 

    foreach (Control c in panel1.Controls.Where(x => x is SpellBox || x is TextBox)) 
    { 
     if (!errorProviders.Any(e => e.GetError(c).Length > 0)) 
     { 
      return false; 
     } 
    } 
} 
catch (Exception ex) 
{ 
    MessageBox.Show(ex.ToString())+ "Error has occurred, Please cancel and try again!"); 
} 
return true; 
+0

上有错误感谢您的回答,您的权利,我不需要在循环中进行所有验证,认为我必须首先检查类型。也想我可能甚至不能使用foreach并检查面板本身 –