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

Question

У меня есть класс, который сравнивает 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.

Answer 1

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

Еще одна вещь, о которой стоит подумать, это то, что если у вас много кода с общим шаблоном, где меняются несколько вещей (например, «какое свойство я сравниваю?», То это хороший кандидат на общий метод принимая делегата, чтобы представить эту разницу

EDIT:. Итак, теперь мы знаем, что мы можем использовать 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: Удаление документы
либо использовать существующий цикл по каждому элементу, или изменить свойства. Если ваши свойства действительно имеют тип 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 
} 

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

Answer 2

В каких рамках вы ориентируетесь? (Это будет иметь значение в ответе.)

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

Не стоит подпись выглядеть так:

DiffResults results = object.CompareTo(object2); 

Answer 3

Как вы используете по крайней мере .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.

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

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

Answer 5

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

Answer 6

Использование нескольких списков в Еогеасп легко. Сделайте это:

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

Он работает подобно тому, как это Еогеасп (TextBox т в цв & & TextBox г в дез)