1

いくつかのコードをトレースしています。リストから特定のアイテムを削除するために、リストが以下に送信されます。

これはgotoを使用する適切な方法ですか？それも必要ですか？2番目のifをelseifに編集し、gotoを必要とせずにリストの処理を続行しませんか？（BASIC以外でgotoを見たのはこれが初めてです。）

public static IList<ClientEntity> FilterNoNeedSendBackToClients(IList<ClientEntity> src)
    {
        if (src == null)
            return null;
    checkAB01:
        for (int i = 0; i < src.Count; i++)
        {
            ClientEntity info = src[i];
            if (info.ProductNumber != null && info.ProductNumber.ToLower().Trim().Length > 0
                    &&
                    info.CancelledByReliantSyncAB01 != null && info.CancelledByReliantSyncAB01.Value == true
                )
            {
                src.Remove(info);
                goto checkAB01;
            }

            if ((info.PartnerContract == null || info.PartnerContract.Trim().Length == 0)
                &&
                (info.ProductNumber == null || info.ProductNumber.Trim().Length == 0))
            {
                src.Remove(info);
                goto checkAB01;
            }
        }
        return src;
    }
4

5 に答える 5

4

LINQはどうですか？

public static IList<ClientEntity> FilterNoNeedSendBackToClients(IList<ClientEntity> src)
{
    if (src == null)
        return null;

    return (from info in src.AsEnumerable<ClientEntity>()
            where !(!String.IsNullOrWhiteSpace(info.ProductNumber) &&
                    info.CancelledByReliantSyncAB01 == (bool?)true)
            where !(String.IsNullOrWhitespace(info.PartnerContract) &&
                    String.IsNullOrWhiteSpace(info.ProductNumber))
            select info).ToList();
}
于 2012-11-29T20:26:02.187 に答える
2

いいえ、これはgotoを使用する適切な方法ではありません。これは、リストからアイテムを適切に削除する方法を知らない人の代わりになります。（要素のスキップを防ぐために逆方向に繰り返します）

public static IList<ClientEntity> FilterNoNeedSendBackToClients(IList<ClientEntity> src)
{
    if (src == null)
        return null;

    for (int i = src.Count - 1; i >= 0; i--)
    {
        ClientEntity info = src[i];
        if (info.ProductNumber != null && info.ProductNumber.ToLower().Trim().Length > 0
                &&
                info.CancelledByReliantSyncAB01 != null && info.CancelledByReliantSyncAB01.Value == true
            )
        {
            src.RemoveAt(i);
        }

        if ((info.PartnerContract == null || info.PartnerContract.Trim().Length == 0)
            &&
            (info.ProductNumber == null || info.ProductNumber.Trim().Length == 0))
        {
            src.RemoveAt(i);
        }
    }
    return src;
}
于 2012-11-29T20:20:40.040 に答える
2

このコードを書いた人は誰でも、反復コレクションからアイテムを削除する方法を知らなかったと思います。これらのgotoの理由は、アイテムが削除されると、コレクションが小さくなり、反復エラーが発生する可能性があるためです。

これを行うためのより良い日は、逆forループを実行することです。このようにして、削除後にリスト全体を繰り返す必要はありません。以下のコードは問題なく動作します。

for (int i = src.Count - 1; i >= 0; i--) {
    src.Remove(src[i]);
}
于 2012-11-29T20:20:49.417 に答える
2

他の人が言っているように、これは（使用されることはめったにないはずです）の不適切な使用法gotoです

ただし、全体として、実装は非常に非効率的です。何かを削除するまで0..Nからループし、次に何かを削除するまで0..N（現在は1つ少ない）から再開するように見えます。さらに悪いことに、Remove呼び出しは再び0..Nから始まり、削除するアイテムを探します。

削除するものが見つからない場合は、戻ります。エントリを削除してRemoveAtから戻るforループの逆を実行することをお勧めします。

public static IList<ClientEntity> FilterNoNeedSendBackToClients(IList<ClientEntity> src)
{
    if (src == null)
        return null;

    for (int i = src.Count - 1; i >= 0; i--)
    {
        ClientEntity info = src[i];
        if (info.ProductNumber != null && info.ProductNumber.ToLower().Trim().Length > 0
            &&
            info.CancelledByReliantSyncAB01 != null && info.CancelledByReliantSyncAB01.Value == true)
        {
            src.RemoveAt(i);
        }
        else if ((info.PartnerContract == null || info.PartnerContract.Trim().Length == 0)
            &&
            (info.ProductNumber == null || info.ProductNumber.Trim().Length == 0))
        {
            src.RemoveAt(i);
        }
    }

    return src;
}

また、そこに追加しました：潜在的に真である可能性のある別のチェックを実行し、同じアイテムを再削除しようとするのelseifは危険なようです（特にインデックスを変更した後）。 if

編集：読みやすい使用可能なコードについて話している場合は、とにかくチェックを別のメソッドに移動します：

public static IList<ClientEntity> FilterNoNeedSendBackToClients(IList<ClientEntity> src)
{
    if (src == null)
        return null;

    for (int i = src.Count - 1; i >= 0; i--)
    {
        if (ShouldClientNotSendBack(src[i]))
            src.RemoveAt(i);
    }

    return src;
}

private static bool ShouldClientNotSendBack(ClientEntity info)
{
    if (!String.IsNullOrWhiteSpace(info.ProductNumber) && info.CancelledByReliantSyncAB01 == true)
    {
        return true;
    }

    if (!String.IsNullOrWhiteSpace(info.PartnerContract))
    {
        return true;
    }

    return false;
}

ShouldClientNotSendBackそのメソッドや名前を微調整することも検討するかもしれませんが（おそらく、2セットのifチェックを明確な名前を持つ個々のメソッドに移動することさえあります）、これは全体として大幅な改善だと思います。

EDITx2：実際、私はこのメソッドの使用を強く検討したいと思います。このメソッドはIList<ClientEntity>、入力コレクションを取り込んでいる間、明らかにしばらく戻ります。これは通常、実際には既存のリストを変更して同じリストインスタンスを返すときに、これが新しいリストを作成していることを開発者に伝えます。新しいリストを返すようにするか（したがって、ループコードを変更して、既存のリストから削除するのではなく、新しいリストに追加する必要があります）、または戻り型を削除して、渡されたリスト引数を変更していることがより明確になるようにします。

于 2012-11-29T20:21:24.393 に答える
2

gotoコメントで述べたように、C＃で使用する「適切な」方法はありません。キーワードは、まさにその定義によれば、応急修理です。これはC/C ++への逆戻りであり、開発者が「ジャンプ」を隠す定義されたコードブロックなしで、ASM、BASIC、またはその他の言語でプログラムを行ごとに翻訳したい場合に備えて、「万が一に備えて」含まれています。それらの間に入り込むために使用されます。これを使用するアルゴリズムは、リファクタリングする必要がないようにリファクタリングすることができ、結果のアルゴリズムはより読みやすくなります。

この場合：

public static IList<ClientEntity> FilterNoNeedSendBackToClients(IList<ClientEntity> src)
{
    if (src == null)
        return null;

    for (int i = src.Count-1; i>=0; i--)
    {
        ClientEntity info = src[i];
        if (info.ProductNumber != null && info.ProductNumber.ToLower().Trim().Length > 0
                &&
                info.CancelledByReliantSyncAB01 != null && info.CancelledByReliantSyncAB01.Value == true
            )
        {
            src.Remove(info);
            continue;
        }

        if ((info.PartnerContract == null || info.PartnerContract.Trim().Length == 0)
            &&
            (info.ProductNumber == null || info.ProductNumber.Trim().Length == 0))
        {
            src.Remove(info);
            continue;
        }
    }
    return src;
}

Chris Sinclairの答えが示すように、ループ内の条件の「どちらか/または」暗黙の構造のため、continueステートメントは必要ありませんが、そのポイントからループの終わりまで何もコーダーに示さないので、ステートメントは好きですそれを決定するために残りの部分を読む必要なしに実行されます。

必要に応じて、リストを前から後ろに移動し、アイテムが削除された場合はi、ループがリスト内の現在の位置を維持するように、続行する前にデクリメントします。カウンターの操作によりアルゴリズムの理解が難しくなるため（また、各反復でのリストのカウントの決定がわずかに遅くなるため）、このようにしないと言う人もいるかもしれませんが、パフォーマンスと可読性の両方で、よりもはるかに優れていますgoto。_

于 2012-11-29T20:22:15.890 に答える