帮助重构这个混乱的嵌套 if 方法

Question

好吧，我想要一些意见，如何解决这个混乱的方法！

它有很多嵌套的“if”语句。

但意识到我必须确切地知道该方法在哪里失败，目前在每个相应的“else”子句中我正在记录错误（失败的“if”条件”）。

注意：忽略事物背后的任何逻辑，请关注样式和结构，因为我已经编写了所有函数名称等。

这是骨架结构：

   public void MyMethod()
{

   try
   {
    bool tryAgain = false;

    string filename = DownloadFile();

    if( IsFileFormatOk(filename) )
    {

        blah = GetBlah(filename);

        if(blah.ID > 0)
        {

            if(ImportFile(filename)
            {

                string username = GetUserFromFile(filename);

                if(isValidUser(username))
                {

                    // few more levels to go
                    //
                    //
                    //

                }
                else
                {
                    LogError(filename, ...); // specific to this if statement
                    tryAgain = true;
                }


            }
            else
            {

                LogError(filename, ...); // specific to this if statement
                tryAgain = true;
            }

        }
        else
        {
            LogError(filename, ...); // specific to this if statement
            tryAgain = true;
        }

    }
    else
    {
        LogError(filename, ...); // specific to this if statement
        tryAgain = true;
    }

   }
   catch
   {
   }
   finally
   {
    if(tryAgain)
    {
        // blah
    }
   }


}

Answer 1

我将努力更改您的逻辑，以便您可以尽快从该方法返回，而不是嵌套更多逻辑。 例如：

//  GOOD
if (!file.exists())
    return;
// Rest of the code

// BAD
if (file.exists()){
    // Code goes here
}
return;

这可能有助于删除一些嵌套并使事情变得不那么复杂。

Answer 2

我猜想有相当多的逻辑等待在其他地方提取，但无论如何，这里有另一种扁平化嵌套的方法：

try
{
  if( !IsFileFormatOk(filename) )
    throw new MySpecificException(...); // pass specific log params

  blah = GetBlah(filename);

  if(blah.ID <= 0)
    throw new MySpecificException(...); // pass specific log params

  if(!ImportFile(filename)
    throw new MySpecificException(...); // pass specific log params

  string username = GetUserFromFile(filename);

  // ...
}
catch (MySecificException e)
{
  LogError(filename, e.LogParams)
  // blah
}

Answer 3

您可以首先将顶级 if (true 分支) 放入单独的方法中。 并继续直到满意为止。

因此，通常您将： 更改

if (condition) {
  // Block1
} else {
  // Block2
}

为

if (condition) {
  Block(...);
} else {
  Block2(...);
}

请注意需要传递给新方法的局部变量。

Answer 4

您应该尝试让您调用的所有方法在发生错误时抛出异常。 抛出的异常应该包含您希望记录的任何错误消息。 创建一个 LogError() 方法，该方法接受任何异常并记录嵌入的消息。 像这样（伪代码）：

MyMethod()
{
  try
  {
    string filename = DownloadFile()
    blah = GetBlah(filename)
    ImportFile(filename)
    ...
  }
  catch DownloadFileException, GetBlahException, ImportFileException e
  {
    LogError(e)
  }
}

当然，您必须创建所有这些异常，但这非常简单。 在大多数语言中，您只需对顶级 Exception 对象进行子类化即可。

Answer 5

编辑：既然我知道有这么多级别，我们需要采取另一种方法。 我不知道“重试”位中有什么，但您需要更改逻辑，以便它测试失败案例，而不是成功案例。 如果失败，记录并返回（或以某种方式执行重试所需的任何操作）。 否则，你可以继续跳出“如果”。

if(EndOfWorld)
{
    WriteLastLogEntryEver();
    return; //run away
}

//we're safe (for now)
ChargeOnAhead();

---

在我知道嵌套 if 的级别之前，我提出了以下建议。

public void MyMethod()
{

   try
   {
    bool tryAgain = false;

    string filename = DownloadFile();

    if( IsFileFormatOk(filename) )
    {

        tryAgain = DoBlah(filename, ...);
    }
    else
    {
        LogError(filename, ...); // specific to this if statement
        tryAgain = true;
    }

   }
   catch
   {
   }
   finally
   {
    if(tryAgain)
    {
        // blah
    }
   }


}

private bool DoImport(string filename, blah)
{

    if(ImportFile(filename))
    {

            // and so forth!
            return false;
    }

    LogError(filename, ...); // specific to this if statement
    return true;
}

private bool DoBlah(string filename)
{
        blah = GetBlah(filename);

        if(blah.ID > 0)
        {

            return DoImport(filename, ...);

        }

        LogError(filename, ...); // specific to this if statement
        return true;

}

我确信您可以对此进行更多重构，并将 // 等 拉入新方法中。 这取决于您需要来回传递多少东西。 如果您传递的参数太多，请考虑作为参数或类字段是否更有意义。

---

Answer 6

这是 jjnguy 的建议，已实施：

public void MyMethod() {
    try
    {
        bool tryAgain = false;
        string filename = DownloadFile();

        if( !IsFileFormatOk(filename) )
        {
            LogError(filename, ...); // specific to this if statement
            tryAgain = true;
            return;
        }

        blah = GetBlah(filename);
        if(blah.ID <= 0)
        {
            LogError(filename, ...); // specific to this if statement
            tryAgain = true;
            return;
        }

        if(!ImportFile(filename))
        {
            LogError(filename, ...); // specific to this if statement
            tryAgain = true;
            return;
        }

        string username = GetUserFromFile(filename);

        if(!isValidUser(username))
        {
            LogError(filename, ...); // specific to this if statement
            tryAgain = true;
            return
        }

        // few more levels to go
        //

    finally
    {
        if(tryAgain)
        {
            // blah
        }
   }
}

Answer 7

对于这类问题，http://refactormycode.com/ 非常有用。

尽管 SO 人员也确实致力于重构一些代码。 ;)