CODEWITHSUNDEEP
c#.netlist
Nate M.
Nate M.

Reputation: 842

Improve efficiency of modifying object properties in a list

I have a list of custom objects that I am working with. I need to find matching objects, and save two attributes to the object, and move on. I can't help but think that my method of working with these objects is sub-optimal. Given I am working with large volumes of data (in this instance a list with ~ 10000 objects, but in other instances significantly larger), I would appreciate any information that might help me optimize the process.

List<WebListingVerification> listings = new List<WebListingVerification>(); //This list is fully populated, and is actually passed into the function.

string sku = reader["vsr_sku"].ToString();
string vendorName = reader["v_name"].ToString();
string vendorSku = reader["vsr_vendor_sku"].ToString();

WebListingVerification listing = listings.Find(x => x.SKU == sku);
if(listing != null)
{
    listings.Remove(listing);
    listing.Vendor = vendorName;
    listing.VendorSKU = vendorSku;
    listings.Add(listing);
}

As you can see above, I first remove the listing, then edit it, and then re-add it. I imagine there is a way to safely edit the object in the list without running Remove / Add which would help a great deal, but I can't seem to find how to do it. I'm not sure if you could do a compound function off of the listings.Find call (listings.Find(x => x.SKU == sku).Vendor = "vendor") but it would be unsafe, as there will be null returns in this circumstance anyways so..

Any help optimizing this would be greatly appreciated.

Edit

Thank you for the comments, I did not understand the fact that the result of the List.Find function call is in fact a pointer to the object in the list, and not a copy of the object. This clears up my issue!

In addition, thank you for the additional answers. I was looking for a simple improvement, predominantly to remove the Add / Remove routines, but the additional answers give me some good ideas on how to write these routines in the future which may net some significant performance improvements. I've been focused on reporting tasks in the past few months, so this example snippet is very similar to probably 100 different routines where I am gathering data from various source databases. Again, I very much appreciate the input.

Upvotes: 0

Views: 91

Answers (4)

Frozenthia
Frozenthia

Reputation: 759

If your items are unique, might I suggest a HashSet<T>?

HashSet<WebListingVerification> listings = new HashSet<WebListingVerification>();

string sku = reader["vsr_sku"].ToString();
string vendorName = reader["v_name"].ToString();
string vendorSku = reader["vsr_vendor_sku"].ToString();

if(listings.Contains(listing))
{
    listings.Remove(listing);
    listing.Vendor = vendorName;
    listing.VendorSKU = vendorSku;
    listings.Add(listing);
}

You'd have to roll your own IEqualityComparer<T> interface on the WebListingVerification object and match on the SKU, which I assume is unique.

public class WebListingVerification : IEqualityComparer<WeblistingVerification>
{
    public string Sku { get; set; }

    public bool Equals(WebListingVerification obj, WebListingVerification obj2)
    {
        if (obj == null && obj2 == null)
            return true;
        else if (obj == null | obj2 == null)
            return false;
        else if (obj.Sku == obj2.Sku)
            return true;
        else
            return false;
    }

    public int GetHashCode(WebListingVerification obj)
    {
        return Sku.GetHashCode();
    }
}

HashSet.Contains() performance is phenomenal on large datasets like this.

Upvotes: 1

PhillipH
PhillipH

Reputation: 6222

No need to remove and add back the object into the list. Just;

if(listing != null)
{
    listing.Vendor = vendorName;
    listing.VendorSKU = vendorSku;
}

Upvotes: 0

George Lica
George Lica

Reputation: 1816

public class WebListingVerification
    {
        public string Sku { get; set; }

        public string VendorName { get; set; }

        public string VendorSku { get; set; }
    }

    public class ListingManager : IEnumerable <WebListingVerification>
    {
        private Dictionary<string, WebListingVerification> _webListDictionary;

        public ListingManager(IEnumerable <WebListingVerification> existingListings)
        {
            if (existingListings == null)
                _webListDictionary = new Dictionary<string, WebListingVerification>();
            else
                _webListDictionary = existingListings.ToDictionary(a => a.Sku);
        }

        public void AddOrUpdate (string sku, string vendorName, string vendorSku)
        {
            WebListingVerification verification;
            if (false == _webListDictionary.TryGetValue (sku, out verification))
                _webListDictionary[sku] = verification = new WebListingVerification();

            verification.VendorName = vendorName;
            verification.VendorSku = vendorSku;
        }

        public IEnumerator<WebListingVerification> GetEnumerator()
        {
            foreach (var item in _webListDictionary)
                yield return item.Value;
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();   
        }
    }

Upvotes: 2

Janne Matikainen
Janne Matikainen

Reputation: 5121

To speed up the lookup you could first convert your list into a dictionary. Note though if your update method is a method, you should not do the conversion inside the method, but outside the update loop.

var dictionary = listings.ToDictionary(l => l.SKU);

And get the item from the dictionary with the sku value.

WebListingVerification listing;
if (dictionary.TryGetValue(sku, out listing))
{
    listing.Vendor = vendorName;
    listing.VendorSKU = vendorSku;
}

Upvotes: 0

Related Questions