2
\$\begingroup\$

I'm looking for a better more efficient way to write some code I have where I"m synchronizing two lists. Basically, the first list is a list of devices I need to check. The second list is a list of devices that I have already checked with dates. I need to synchronize the two lists so I only check devices that are new or out of date based on the date. I also need to delete any devices that have been removed.

Here is my current code that works fine, it just looks and feels SO clunky.

public IEnumerable<AssetBlob> Synchronize(IEnumerable<AssetBlob> assets, string id) {
      //Get list of devices already checked in lineitemsmap
      List<AssetBlob> items = new List<AssetBlob>();
      IEnumerable<LineItemsMap> lineItems = auditRepo.GetLineItemsMap(id).DeviceResults;
      List<LineItemsMap> deleted = new List<LineItemsMap>();

      //put new items into list
      items.AddRange((from t0 in assets
                      join t1 in lineItems on t0.Id equals t1.BlobId into t1_join
                      from t1 in t1_join.DefaultIfEmpty()
                      where t1 == null
                      select t0).ToList());

      //list of existing items that need updated
      items.AddRange((from t0 in assets
                      join t1 in lineItems on t0.Id equals t1.BlobId
                      where t0.Imported > t1.Created
                      select t0).ToList());

      deleted.AddRange((from t0 in assets
                        join t1 in lineItems on t0.Id equals t1.BlobId
                        where t0.Imported > t1.Created
                        select t1).ToList());

      //Delete items in lineitems that don't exists in assets list
      deleted.AddRange((from t0 in lineItems
                        join t1 in assets on t0.BlobId equals t1.Id into t1_join
                        from t1 in t1_join.DefaultIfEmpty()
                        where t1 == null
                        select t0).ToList());

      if (deleted.Any()) {
        auditRepo.RemoveLineItemMaps(deleted);
      }

      return items;
    }

Any suggestion or improvements would be great. Thank you.

\$\endgroup\$
  • \$\begingroup\$ You're used to writing SQL queries with outer joins, don't you? :-) \$\endgroup\$ – svick Jul 30 '12 at 16:41
  • \$\begingroup\$ I have to use outer joins because some of the records won't be in both tables. \$\endgroup\$ – Kyle Rogers Jul 30 '12 at 17:05
  • \$\begingroup\$ You should look into .Intersect and .Except methods. \$\endgroup\$ – Kris Jul 31 '12 at 12:30
3
\$\begingroup\$

I haven't tested this due to time constraints, and lack of knowledge of the data, but from my understanding of the problem, this should work.

I have taken the linq join queries, two for the insert, two for the delete and created two IEnumerable.Where clauses, one for insert, one for delete. The two sets of criteria are combined in an OR statement within the .Where statement.

The one thing I wasn't sure on is if the original list needed to be returned. That wasn't clear in the original code. If it does need to be returned, that's an easy change to implement.

EDIT: Changed to do work per Kyles suggestion in comments.

public IEnumerable<AssetBlob> Synchronize(IEnumerable<AssetBlob> assets, string id)
{
    var assetsList = assets.ToList();

    //Get list of devices already checked in lineitemsmap
    IEnumerable<LineItemsMap> lineItems = auditRepo.GetLineItemsMap(id).DeviceResults;

    var items =
        assetsList.Where(
            existingAsset => 
                // ExisingItems that need updating
                // Any returns true if any of the items in a list meet the criteria
                lineItems.Any(lineItem => lineItem.BlobId == existingAsset.Id && existingAsset.Imported > lineItem.Created) ||
                // New items that need inserting.
                // All returns true if all of the items in a list meet the criteria
                // therefore All == !Any
                lineItems.All(lineItem => lineItem.BlobId == existingAsset.Id)).ToList();


    var deleted =
        lineItems.Where(lineItem =>
                        assetsList.Any(existingAsset => existingAsset.Imported > lineItem.Created) ||
                        assetsList.All(existingAsset => existingAsset.Id != lineItem.BlobId)).ToList();

    // If there is a foreach in RemoveLineItemMaps you can probably remove this check.
    // Just cleans up the code a little, and the foreach will take care of this check.
    if (deleted.Any())
    {
        auditRepo.RemoveLineItemMaps(deleted);
    }

    return items;
}
\$\endgroup\$
  • \$\begingroup\$ Could you explain what did change outside of comments, so that people don't have to go through all the code? \$\endgroup\$ – svick Jul 30 '12 at 17:28
  • \$\begingroup\$ @svick Is that better? \$\endgroup\$ – Jeff Vanzella Jul 30 '12 at 17:35
  • \$\begingroup\$ Interesting. All my tests passed for this but the scenario where a device previously existed got removed. So basically when 'assets' = 2 and lineitems = 3. deleted.Count() should be 1. \$\endgroup\$ – Kyle Rogers Jul 30 '12 at 18:36
  • \$\begingroup\$ All my tests passed once I switched the boolean on the All() for deleted to !=. Thanks for the suggestion. \$\endgroup\$ – Kyle Rogers Jul 30 '12 at 19:07
  • \$\begingroup\$ Glad you got it working \$\endgroup\$ – Jeff Vanzella Jul 30 '12 at 22:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.