1

我正在跟踪一些代码，并将一个列表发送到下面，以从列表中删除某些项目。

这是使用 goto 的正确方法吗？甚至有必要吗？难道您不只是将第二个 if 编辑为 else if，然后继续处理列表而不需要 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然后返回。

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;
}

此外，我添加了一个elseifthere: 似乎很危险，进行另一项 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方法和/或名称（甚至可能将两组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 回答