8

同じオブジェクトの 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 です。

4

6 に答える 6

7

.NET 3.5 を使用していますか? LINQ to Objects を使用すると、この多くの作業がはるかに簡単になると確信しています。

考慮すべきもう 1 つのことは、共通のパターンを持つ多くのコードがあり、いくつかの変更がある場合 (たとえば、「どのプロパティを比較するか?」など) は、デリゲートを取得するジェネリック メソッドの適切な候補であるということです。その違いを表します。

編集:さて、LINQ を使用できることがわかりました。

ステップ 1: ネスティングを減らす
 まず、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 にキー関数を単純に指定できれば、これははるかに優れています。等値比較子を指定できますが、それらの 1 つを構築するのは、ユーティリティ ライブラリを使用しても面倒です。まぁ。

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
}

さらに単純化するための 1 つのオプションは、UserId を使用して IEqualityComparer を実装することです (DocId を使用したドキュメント用のオプションも 1 つ)。

于 2008-10-16T21:20:30.207 に答える
2

少なくとも .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 のリストであることを前提としています。

于 2008-10-16T21:42:29.880 に答える
1

StepDocs と StepUsers の両方が IComparable<T> を実装し、それらが IList<T> を実装するコレクションに格納されている場合、次のヘルパー メソッドを使用してこの関数を簡略化できます。StepDocs で 1 回、StepUsers で 1 回、2 回呼び出すだけです。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);
};
于 2008-10-16T22:24:01.217 に答える
0

どのフレームワークをターゲットにしていますか? （これで答えが変わってきます。）

なぜこれは void 関数なのですか?

署名は次のようにすべきではありません:

DiffResults results = object.CompareTo(object2);
于 2008-10-16T21:25:26.763 に答える
0

ツリーのような構造のトラバーサルを隠したい場合は、「醜い」ループ構造を隠してから CompareTo インターフェイスを使用する IEnumerator サブクラスを作成できます。

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

ただし、これが特に何かを単純化するかどうかはまったくわかりません。ネストされたトラバーサル構造を見てもかまいません。コードはネストされていますが、複雑ではなく、特に理解しにくいものでもありません。

于 2008-10-16T21:50:45.060 に答える
0

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)

于 2009-11-10T11:43:03.217 に答える