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

Вы используете.NET 3.5? Я уверен, что LINQ к Объектам сделал бы многое из этого намного более простым.

Другая вещь думать о состоит в том что, если у Вас есть много кода с общим шаблоном, где всего несколько вещей изменяются (например, "какое свойство я сравниваю?" затем это - хороший кандидат на общий метод, берущий делегата для представления того различия.

Править: Хорошо, теперь мы знаем, что можем использовать LINQ:

Шаг 1: Уменьшите вложение
Во-первых я вынул бы один уровень вложения. Вместо:

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

Я сделал бы:

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

Ранние возвраты как этот могут сделать код намного более читаемым.

Шаг 2: Нахождение, что документы удаляют

Это было бы намного более хорошо, если Вы могли бы просто указать ключевую функцию к Счетному. Пересечься. Можно указать компаратор равенства, но создание одного из тех является болью, даже со служебной библиотекой. А-ч хорошо.

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 или измените свойства. Если Ваши свойства имеют на самом деле список типов <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).

Поскольку Вы используете, по крайней мере.NET 2.0, я рекомендую, чтобы реализация Равнялась и 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.

Если и 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);
};

Для какой платформы Вы нацелены? (Это будет иметь значение в ответе.)

Почему это - пустая функция?

Не был должен подпись быть похожей:

DiffResults results = object.CompareTo(object2);

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

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

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