删除 C# 中重复的硬编码循环和条件答案

【问题标题】：Remove repetitive, hard coded loops and conditions in C#删除 C# 中重复的硬编码循环和条件
【发布时间】：2025-12-11 01:55:02
【问题描述】：

我有一个类，它比较相同对象的 2 个实例，并生成它们的差异列表。这是通过循环遍历关键集合并使用已更改的列表填充一组其他集合来完成的（查看下面的代码后可能更有意义）。这可行，并生成一个对象，让我知道在“旧”对象和“新”对象之间究竟添加和删除了什么。
我的问题/担忧是……它真的很丑，有很多循环和条件。有没有更好的方法来存储/处理这个，而不必如此依赖无休止的硬编码条件组？

    public void DiffSteps()
    {
        try
        {
            //Confirm that there are 2 populated objects to compare
            if (NewStep.Id != Guid.Empty && SavedStep.Id != Guid.Empty)
            {
                //<TODO> Find a good way to compare quickly if the objects are exactly the same...hash?

                //Compare the StepDoc collections:
                OldDocs = SavedStep.StepDocs;
                NewDocs = NewStep.StepDocs;
                Collection<StepDoc> docstoDelete = new Collection<StepDoc>();

                foreach (StepDoc oldDoc in OldDocs)
                {
                    bool delete = false;
                    foreach (StepDoc newDoc in NewDocs)
                    {
                        if (newDoc.DocId == oldDoc.DocId)
                        {
                            delete = true;
                        }
                    }
                    if (delete)
                        docstoDelete.Add(oldDoc);
                }

                foreach (StepDoc doc in docstoDelete)
                {
                    OldDocs.Remove(doc);
                    NewDocs.Remove(doc);
                }


                //Same loop(s) for StepUsers...omitted for brevity

                //This is a collection of users to delete; it is the collection
                //of users that has not changed.  So, this collection also needs to be checked 
                //to see if the permisssions (or any other future properties) have changed.
                foreach (StepUser user in userstoDelete)
                {
                    //Compare the two
                    StepUser oldUser = null;
                    StepUser newUser = null;

                    foreach(StepUser oldie in OldUsers)
                    {
                        if (user.UserId == oldie.UserId)
                            oldUser = oldie;
                    }

                    foreach (StepUser newie in NewUsers)
                    {
                        if (user.UserId == newie.UserId)
                            newUser = newie;
                    }

                    if(oldUser != null && newUser != null)
                    {
                        if (oldUser.Role != newUser.Role)
                            UpdatedRoles.Add(newUser.Name, newUser.Role);
                    }

                    OldUsers.Remove(user);
                    NewUsers.Remove(user);
                }

            } 
        }
        catch(Exception ex)
        {
            string errorMessage =
                String.Format("Error generating diff between Step objects {0} and {1}", NewStep.Id, SavedStep.Id);
            log.Error(errorMessage,ex);
            throw;
        }
    }

目标框架是 3.5。

【问题讨论】：

标签： c# refactoring loops conditional-statements

【解决方案1】：

在 foreach 中使用多个列表很容易。这样做：

foreach (TextBox t in col)
{
    foreach (TextBox d in des) // here des and col are list having textboxes
    {
        // here remove first element then and break it
        RemoveAt(0);
        break;
    }
}

它的工作原理与 foreach 类似（TextBox t in col && TextBox d in des）

【讨论】：

【解决方案2】：

您使用的是 .NET 3.5 吗？我敢肯定 LINQ to Objects 会让这很多变得更简单。

另一件要考虑的事情是，如果您有很多具有通用模式的代码，其中只有少数事情发生了变化（例如“我在比较哪个属性？”那么这是一个很好的通用方法的候选者）代表这种差异的代表。

编辑：好的，现在我们知道我们可以使用 LINQ：

第 1 步：减少嵌套
首先，我会取出一层嵌套。而不是：

if (NewStep.Id != Guid.Empty && SavedStep.Id != Guid.Empty)
{
    // Body
}

我愿意：

if (NewStep.Id != Guid.Empty && SavedStep.Id != Guid.Empty)
{
    return;
}
// Body

这样的提前返回可以使代码更具可读性。

第 2 步：查找要删除的文档

如果您可以简单地为 Enumerable.Intersect 指定一个键函数，那就更好了。您可以指定一个相等比较器，但即使使用实用程序库，构建其中一个也很痛苦。嗯嗯。

var oldDocIds = OldDocs.Select(doc => doc.DocId);
var newDocIds = NewDocs.Select(doc => doc.DocId);
var deletedIds = oldDocIds.Intersect(newDocIds).ToDictionary(x => x);
var deletedDocs = oldDocIds.Where(doc => deletedIds.Contains(doc.DocId));

第 3 步：删除文档
要么使用现有的 foreach 循环，要么更改属性。如果您的属性实际上是 List 类型，那么您可以使用 RemoveAll。

第 4 步：更新和删除用户

foreach (StepUser deleted in usersToDelete)
{
    // Should use SingleOfDefault here if there should only be one
    // matching entry in each of NewUsers/OldUsers. The
    // code below matches your existing loop.
    StepUser oldUser = OldUsers.LastOrDefault(u => u.UserId == deleted.UserId);
    StepUser newUser = NewUsers.LastOrDefault(u => u.UserId == deleted.UserId);

    // Existing code here using oldUser and newUser
}

进一步简化事情的一个选择是使用 UserId 实现一个 IEqualityComparer（一个用于带有 DocId 的文档）。

【讨论】：

【解决方案3】：

如果 StepDocs 和 StepUsers 都实现了 IComparable，并且它们存储在实现 IList 的集合中，那么您可以使用以下辅助方法来简化此功能。只需调用两次，一次使用 StepDocs，一次使用 StepUsers。使用 beforeRemoveCallback 来实现用于进行角色更新的特殊逻辑。我假设这些集合不包含重复项。我省略了参数检查。

public delegate void BeforeRemoveMatchCallback<T>(T item1, T item2);

public static void RemoveMatches<T>(
                IList<T> list1, IList<T> list2, 
                BeforeRemoveMatchCallback<T> beforeRemoveCallback) 
  where T : IComparable<T>
{
  // looping backwards lets us safely modify the collection "in flight" 
  // without requiring a temporary collection (as required by a foreach
  // solution)
  for(int i = list1.Count - 1; i >= 0; i--)
  {
    for(int j = list2.Count - 1; j >= 0; j--)
    {
      if(list1[i].CompareTo(list2[j]) == 0)
      {
         // do any cleanup stuff in this function, like your role assignments
         if(beforeRemoveCallback != null)
           beforeRemoveCallback(list[i], list[j]);

         list1.RemoveAt(i);
         list2.RemoveAt(j);
         break;
      }
    }
  }
}

以下是更新代码的 beforeRemoveCallback 示例：

BeforeRemoveMatchCallback<StepUsers> callback = 
delegate(StepUsers oldUser, StepUsers newUser)
{
  if(oldUser.Role != newUser.Role)
    UpdatedRoles.Add(newUser.Name, newUser.Role);
};

【讨论】：

【解决方案4】：

如果你想隐藏树状结构的遍历，你可以创建一个隐藏“丑陋”循环结构的 IEnumerator 子类，然后使用 CompareTo 接口：

MyTraverser t =new Traverser(oldDocs, newDocs);

foreach (object oldOne in t)
{
    if (oldOne.CompareTo(t.CurrentNewOne) != 0)
    {
        // use RTTI to figure out what to do with the object
    }
}

但是，我完全不确定这是否会特别简化任何事情。我不介意看到嵌套的遍历结构。代码是嵌套的，但并不复杂或特别难以理解。

【讨论】：

【解决方案5】：

由于您至少使用 .NET 2.0，我建议在 StepDoc 上实现 Equals 和 GetHashCode (http://msdn.microsoft.com/en-us/library/7h9bszxx.aspx)。作为它如何清理您的代码的提示，您可以使用以下内容：

 Collection<StepDoc> docstoDelete = new Collection<StepDoc>();
foreach (StepDoc oldDoc in OldDocs)
                    {
                        bool delete = false;
                        foreach (StepDoc newDoc in NewDocs)
                        {
                            if (newDoc.DocId == oldDoc.DocId)
                            {
                                delete = true;
                            }
                        }
                        if (delete) docstoDelete.Add(oldDoc);
                    }
                    foreach (StepDoc doc in docstoDelete)
                    {
                        OldDocs.Remove(doc);
                        NewDocs.Remove(doc);
                    }

用这个：

oldDocs.FindAll(newDocs.Contains).ForEach(delegate(StepDoc doc) {
                        oldDocs.Remove(doc);
                        newDocs.Remove(doc);
                    });

这假定 oldDocs 是 StepDoc 的列表。

【讨论】：

【解决方案6】：

您的目标是什么框架？（这将使答案有所不同。）

为什么这是一个 void 函数？

签名不应该是这样的吗：

DiffResults results = object.CompareTo(object2);

【讨论】：

目标框架是 3.5。它是一个 void 函数，因为它正在填充 DiffStep 的属性，如 OldDocs 等。