120

I'm adding remote devices to a list as they announce themselves across the network. I only want to add the device to the list if it hasn't previously been added.

The announcements are coming across an async socket listener so the code to add a device can be run on multiple threads. I'm not sure what I'm doing wrong but no mater what I try I end up with duplications. Here is what I currently have.....

lock (_remoteDevicesLock)
{
    RemoteDevice rDevice = (from d in _remoteDevices
                            where d.UUID.Trim().Equals(notifyMessage.UUID.Trim(), StringComparison.OrdinalIgnoreCase)
                            select d).FirstOrDefault();
     if (rDevice != null)
     {
         //Update Device.....
     }
     else
     {
         //Create A New Remote Device
         rDevice = new RemoteDevice(notifyMessage.UUID);
         _remoteDevices.Add(rDevice);
     }
}
Oli
  • 2,996
  • 3
  • 28
  • 50

3 Answers3

193

If your requirements are to have no duplicates, you should be using a HashSet.

HashSet.Add will return false when the item already exists (if that even matters to you).

You can use the constructor that @pstrjds links to below (or here) to define the equality operator or you'll need to implement the equality methods in RemoteDevice (GetHashCode & Equals).

Austin Salonen
  • 49,173
  • 15
  • 109
  • 139
  • 3
    Was about to add this answer. You can use this overload to define the comparison - http://msdn.microsoft.com/en-us/library/bb359100(v=vs.100).aspx – pstrjds Nov 21 '12 at 16:56
  • 14
    An important note here is that HashSet is not guaranteed to respect insertion order. So if order is important (items need to appear in the list in the same order as you put them in, like what happens with `List`) then HashSet doesn't work well. – JulianR Nov 21 '12 at 17:01
  • Many thanks for this. Do I still need to keep the lock for thread safety or is there a better way? – Oli Nov 21 '12 at 17:04
  • 2
    @Oli You need to keep the lock, but the operation will be *much* quicker, so they won't be waiting on each other as much. There is no `ConcurrentSet` class, unfortunately. There is however a `ConcurrentDictionary` class, so you could use that, store your values as the keys and just store `null` in the values. – Servy Nov 21 '12 at 17:09
  • For some reason I'm still getting collisions-Most of the time it works fine but I every now and again a collision gets through. An example clash is "00000000-0000-0001-0000-001cdf885737"-Looking at my gethashcode and equals methods, they both seem to be working correctly. – Oli Nov 21 '12 at 17:49
  • 1
    @Oli: If I were you, I would post another question to address that issue (with the full source of both GetHashCode & Equals, along with cases where they match when you don't expect them to). – Austin Salonen Nov 21 '12 at 17:55
  • I've created the new question.I don't think the issue is with equality but with the items in the hashset (Or presumably list in my original question)suddenly changing their reference pointers. http://stackoverflow.com/questions/13500057/error-adding-to-hashset-from-multiple-threads – Oli Nov 21 '12 at 18:51
  • @AustinSalonen the link is outdated. Please update :) – Kcvin Jul 18 '17 at 16:23
43
//HashSet allows only the unique values to the list
HashSet<int> uniqueList = new HashSet<int>();

var a = uniqueList.Add(1);
var b = uniqueList.Add(2);
var c = uniqueList.Add(3);
var d = uniqueList.Add(2); // should not be added to the list but will not crash the app

//Dictionary allows only the unique Keys to the list, Values can be repeated
Dictionary<int, string> dict = new Dictionary<int, string>();

dict.Add(1,"Happy");
dict.Add(2, "Smile");
dict.Add(3, "Happy");
dict.Add(2, "Sad"); // should be failed // Run time error "An item with the same key has already been added." App will crash

//Dictionary allows only the unique Keys to the list, Values can be repeated
Dictionary<string, int> dictRev = new Dictionary<string, int>();

dictRev.Add("Happy", 1);
dictRev.Add("Smile", 2);
dictRev.Add("Happy", 3); // should be failed // Run time error "An item with the same key has already been added." App will crash
dictRev.Add("Sad", 2);
Julian Declercq
  • 1,536
  • 3
  • 17
  • 32
20

Just like the accepted answer says a HashSet doesn't have an order. If order is important you can continue to use a List and check if it contains the item before you add it.

if (_remoteDevices.Contains(rDevice))
    _remoteDevices.Add(rDevice);

Performing List.Contains() on a custom class/object requires implementing IEquatable<T> on the custom class or overriding the Equals. It's a good idea to also implement GetHashCode in the class as well. This is per the documentation at https://msdn.microsoft.com/en-us/library/ms224763.aspx

public class RemoteDevice: IEquatable<RemoteDevice>
{
    private readonly int id;
    public RemoteDevice(int uuid)
    {
        id = id
    }
    public int GetId
    {
        get { return id; }
    }

    // ...

    public bool Equals(RemoteDevice other)
    {
        if (this.GetId == other.GetId)
            return true;
        else
            return false;
    }
    public override int GetHashCode()
    {
        return id;
    }
}
Luke Eckley
  • 350
  • 2
  • 8
  • hi thx but what if you can't override because i'm using a reference to someone else's code - what does one do here? – BenKoshy Mar 04 '17 at 00:21