Удалите повторяющиеся жестко закодированные циклы и условия в C#

Вы используете .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 <T>, вы можете использовать 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
}

Один из вариантов еще большего упрощения - реализовать IEqualityComparer с использованием UserId (и один для документов с DocId).

На какой фреймворк вы ориентируетесь? (Это повлияет на ответ.)

Почему это недействительная функция?

Разве подпись не должна выглядеть так:

DiffResults results = object.CompareTo(object2);

Поскольку вы используете как минимум .NET 2.0, я рекомендую реализовать Equals и GetHashCode (http://msdn.microsoft.com/en-us/library/7h9bszxx.aspx) в StepDoc. В качестве подсказки о том, как он может очистить ваш код, вы можете иметь что-то вроде этого:

 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.

Если вы хотите скрыть обход древовидной структуры, вы можете создать подкласс 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
    }
}

Однако я совсем не уверен, что это что-то особо упрощает. Я не против увидеть вложенные структуры обхода. Код вложен, но не сложен и не особенно труден для понимания.

Если и StepDocs, и StepUsers реализуют IComparable <T> и хранятся в коллекциях, реализующих IList <T>, вы можете использовать следующий вспомогательный метод для упрощения этой функции. Просто вызовите его дважды: один раз в 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);
};

Использовать несколько списков в 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 в col && TextBox d в des)