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

この種の質問については、http://refactormycode.com/が非常に役立ちます。

SOの人々は、一部のコードのリファクタリングにも真剣に取り組んでいますが。;)

于 2009-01-10T10:11:32.363 に答える
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

Here's jjnguy's suggestion, implemented:

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 に答える