c# - 矢印のアンチパターンを防ぐ方法

Question

コードをより読みやすいものにリファクタリングする最善の方法について、少し混乱しています。

次のコードを検討してください。

var foo = getfoo();
if(foo!=null)
{
    var bar = getbar(foo);
    if(bar!=null)
    {
        var moo = getmoo(bar);
        if(moo!=null)
        {
            var cow = getcow(moo);
            ...
        }
    }
}
return;

ご覧のとおり、ネストされifた各 if は以前の値に依存しているため、多数のネストされたブロックが必要です。

この点で、コードを少しきれいにする方法を考えていました。

私が自分で考えたいくつかのオプションは次のとおりです。

各if句の後に戻ります。つまり、メソッドを終了する場所が複数あることを意味します
s をスローArgumentNullExceptionします。その後、最後にそれらをキャッチし、return ステートメントを finally 句 (または try/catch ブロックの外側) に配置します。
ラベルを操作し、goto:

これらのオプションのほとんどは、私には少し「汚い」ように見えるので、私が作成したこの混乱をクリーンアップする良い方法があるかどうか疑問に思っていました.

score 27 · Accepted Answer

式を連鎖させることができます。割り当ては割り当てられた値を返すため、その結果を確認できます。また、割り当てられた変数を次の式で使用できます。

式が false を返すとすぐに、他の式は実行されなくなります。これは、(and操作のために) 式全体が既に false を返すためです。

したがって、次のようなものが機能するはずです。

Foo foo; Bar bar; Moo moo; Cow cow;

if( (foo = getfoo()) != null &&
    (bar = getbar(foo)) != null &&
    (moo = getmoo(bar)) != null &&
    (cow = getcow(moo)) != null )
{
  ..
}

score 16 · Accepted Answer

最初にあなたの提案（各if句の後に戻る）は非常に良い方法です：

  // Contract (first check all the input)
  var foo = getfoo();

  if (Object.ReferenceEquals(null, foo))
    return; // <- Or throw exception, put assert etc.

  var bar = getbar(foo);

  if (Object.ReferenceEquals(null, bar))
    return; // <- Or throw exception, put assert etc.

  var moo = getmoo(bar);

  if (Object.ReferenceEquals(null, moo))
    return; // <- Or throw exception, put assert etc.

  // Routine: all instances (foo, bar, moo) are correct (not null) and we can work with them
  ...

2番目の可能性（あなたの場合）は、getbar（）関数とgetmoo（）関数をわずかに変更して、null入力でnullを返すようにすることです。

  var foo = getfoo();
  var bar = getbar(foo); // return null if foo is null
  var moo = getmoo(bar); // return null if bar is null

  if ((foo == null) || (bar == null) || (moo == null))
    return; // <- Or throw exception, put assert(s) etc.    

  // Routine: all instances (foo, bar, moo) are correct (not null)
  ...

3 つ目の可能性は、複雑なケースでは Null Object Desing Pattern を使用できることです。

http://en.wikipedia.org/wiki/Null_Object_pattern

score 11 · Accepted Answer

昔ながらのやり方：

var foo;
var bar;
var moo;
var cow;
var failed = false;
failed = failed || (foo = getfoo()) == null;
failed = failed || (bar = getbar(foo)) == null;
failed = failed || (moo = getmoo(bar)) == null;
failed = failed || (cow = getcow(moo)) == null;

はるかに明確 - 矢印なし - そして永遠に拡張可能です。

に渡ってまたはDark Sideを使用しないでください。gotoreturn

score 5 · Accepted Answer

var foo =                        getFoo();
var bar = (foo == null) ? null : getBar(foo);
var moo = (bar == null) ? null : getMoo(bar);
var cow = (moo == null) ? null : getCow(moo);
if (cow != null) {
  ...
}

score 1 · Accepted Answer

try
{
  if (getcow(getmoo(getbar(getfoo()))) == null)
  {
    throw new NullPointerException();
  }
catch(NullPointerException ex)
{
  return; //or whatever you want to do when something is null
}

//... rest of the method

これにより、メソッドのメインロジックが整理され、例外的な戻り値が 1 つだけになります。その欠点は、get* メソッドが遅い場合は遅くなる可能性があることと、どのメソッドが null 値を返したかをデバッガーで判断するのが難しいことです。

score 1 · Accepted Answer

別の方法は、プログラムフローを制御するために「偽の」単一ループを使用することです。お勧めとは言えませんが、arrowhead よりも見栄えが良く、読みやすいことは間違いありません。

「ステージ」、「フェーズ」、またはそのような変数を追加すると、デバッグやエラー処理が簡素化される場合があります。

int stage = 0;
do { // for break only, possibly with no indent

var foo = getfoo();
if(foo==null) break;

stage = 1;
var bar = getbar(foo);
if(bar==null) break;

stage = 2;
var moo = getmoo(bar);
if(moo==null) break;

stage = 3;
var cow = getcow(moo);

return 0; // end of non-erroreous program flow
}  while (0); // make sure to leave an appropriate comment about the "fake" while

// free resources if necessary
// leave an error message
ERR("error during stage %d", stage);

//return a proper error (based on stage?)
return ERROR;

score 1 · Accepted Answer

Rex Kerr の答えは実に素晴らしいものです。
ただし、コードを変更できる場合は、Jens Schauder の回答の方がおそらく優れています (Null オブジェクトパターン)

例をより具体的にすることができれば、おそらくさらに多くの答えを得ることができます
。たとえば、メソッドの「場所」に応じて、次のようなものを使用できます。

namespace ConsoleApplication8
{
    using MyLibrary;
    using static MyLibrary.MyHelpers;

    class Foo { }
    class Bar { }
    class Moo { }
    class Cow { }


    internal class Program
    {
        private static void Main(string[] args)
        {
            var cow = getfoo()?.getbar()?.getmoo()?.getcow();
        }
    }
}

namespace MyLibrary
{
    using ConsoleApplication8;
    static class MyExtensions
    {
        public static Cow getcow(this Moo moo) => null;
        public static Moo getmoo(this Bar bar) => null;
        public static Bar getbar(this Foo foo) => null;
    }

    static class MyHelpers
    {
        public static Foo getfoo() => null;
    }
}

score -3 · Accepted Answer

これは、 goto を使用する 1 つのケースです。

あなたの例は私を限界に追いやるのに十分ではないかもしれません.あなたの方法が十分に単純であれば、複数のリターンがより良いです. しかし、このパターンはかなり広範囲になる可能性があり、多くの場合、最後にクリーンアップコードが必要になります。可能であればここで他の回答のほとんどを使用しますが、多くの場合、唯一の判読可能な解決策はを使用することgotoです。

(そうするときは、必ずラベルへのすべての参照を 1 つのブロック内に配置して、コードを見ている人gotoがと変数の両方がコードのその部分に限定されていることを認識できるようにしてください。)

Javascript と Java では、次のことができます。

bigIf:  {

    if (!something)      break bigIf;
    if (!somethingelse)  break bigIf;
    if (!otherthing)     break bigIf;

    // Conditionally do something...
}
// Always do something else...
return;

Javascript と Java にはgoto's' がないため、この状況ではそれらが必要であることに他の人が気づいたと思います。

呼び出し元のコードに強制する try/catch の混乱を除いて、例外は私にとっても機能します。また、C# はスロー時にスタックトレースを挿入するため、特に通常は最初のチェックでキックアウトする場合は、コードが遅くなります。

c# - 矢印のアンチパターンを防ぐ方法

13 に答える 13

Related

Reference