3

好的,我想要一些意见,我该如何解决这个混乱的方法!

它有很多嵌套的“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
    }
   }


}
4

7 回答 7

5

我会努力改变你的逻辑,这样你就可以尽快从方法中返回,而不是嵌套更多的逻辑。例如:

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

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

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

于 2009-01-09T14:49:41.413 回答
3

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

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
}
于 2009-01-09T15:09:08.240 回答
1

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

所以通常你会改变:

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

进入

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

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

于 2009-01-09T14:45:30.877 回答
0

编辑:既然我知道有这么多级别,我们需要采取另一种方法。我不知道“重试”位中有什么,但是您需要更改逻辑,以便测试失败案例,而不是成功案例。如果失败,记录并返回(或以某种方式再次尝试所需的任何操作)。否则,您可以继续在 if 之外。

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;

}

我敢肯定,您可以对它进行更多重构,并将其// and so forth引入一种新方法。这取决于你要来回传递多少东西。如果您传递的太多,请考虑作为参数或类字段是否更有意义。


于 2009-01-09T14:54:59.487 回答
0

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

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

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

于 2009-01-09T15:04:31.433 回答
0

这是 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
        }
   }
}
于 2009-01-09T15:21:05.127 回答
0

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

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

于 2009-01-10T10:11:32.363 回答