AddOrUpdate not updating

Question

I have a concurrent dictionary _dict of type <byte[], int>

I loop through the items of another list of strings (list1), and if the item exists as a key in _dict, I increment its value; if it doesn't, I add it to _dict and set its value to 1

foreach(byte[] item in list1)
{
  _dict.AddOrUpdate(item, 1, (key, oldValue) => oldValue + 1);
}

The problem is, even if the item already exists in _dict, it adds it again to _dict instead of updating it.

What's wrong with my code?

Updated code block

    internal class someClass
{
    public List<byte[]> list1 { get; set; }
    public ConcurrentDictionary<byte[], int> dict1 { get; set; } 
}

class Program
{
    static void Main(string[] args)
    {
        someClass obj = new someClass();

        if (obj.list1 != null && obj.list1.Count > 0)
        {
            foreach (byte[] item in obj.list1)
            {
                // upsert operation (add if string not in dictionary; update by incrementing count if it does exist)
                obj.dict1.AddOrUpdate(item, 1, (key, oldValue) => oldValue + 1);
            }
        }
    }
}

_"it adds it again to `_dict`"_ - that's not how dictionaries work: one key can be present only once. Please create a [mcve] and show how you determine the claimed behavior. — CodeCaster, Mar 21 '16 at 22:03
Instead of describing the code in English, consider pasting that code in the question. — Charlie, Mar 21 '16 at 22:05
I don't see anything wrong from just those 3 lines of code, but if you posted the rest of the code with it, you'd probably get better feedback. — levininja, Mar 21 '16 at 22:12
Isn't dict1 null? I don't see you creating an instance for that reference. — Mihai Caracostea, Mar 21 '16 at 22:17
You are just making this up, aren't you? Now it's string... now it's byte[]... — Mihai Caracostea, Mar 21 '16 at 22:19
I was trying to simplify it as much as possible at first, but now I am painting a more accurate picture — IsaacBok, Mar 21 '16 at 22:21
Are you expecting two different `byte[]` instances to compare equal? If so, this has nothing at all to do with `ConcurrentDictionary`. — , Mar 21 '16 at 22:21

score 2 · Answer 1 · answered Mar 21 '16 at 22:27

It is a bad idea to use byte[] as a dictionary key, because two different byte arrays will be compared by reference not the data inside. So, if you strongly need in byte[] as a dictionary key, you should write your own class for IEqualityComparer interface. For example:

public class ByteArrayComparer : IEqualityComparer<byte[]> {
  public bool Equals(byte[] x, byte[] y) {
    if ( x == null || x == null ) {
      return x.length == y.length;
    }
    return x.SequenceEqual(y);
  }
  public int GetHashCode(byte[] key) {
    return key.Sum(b => b);
  }
}

Mihai Caracostea · Accepted Answer · 2016-03-21T22:49:05.073

Since you've changed the key type from string to byte[], now the behavior you are seeing is the correct one (initialy, the question contained string keys).

Because:

The dictionary compares the keys by using the default IEqualityComparer, which in the case of byte[] is simply comparing the reference.

Since two byte[] arrays are two different objects even if they contain the same elements, they will be treated as different keys.

Consider creating an IEqualityComparer implementation yourself and use it in this constructor:

public ConcurrentDictionary(
    IEqualityComparer<TKey> comparer
)

EDIT: Here's a working example:

public class ByteArrayComparer : IEqualityComparer<byte[]>
{
    public bool Equals(byte[] x, byte[] y)
    {
        if (x == null || y == null)     
            return false;       

        if(x.Length != y.Length)
            return false;

        return x.SequenceEqual(y);
    }

    public int GetHashCode(byte[] array)
    {
        unchecked
        {
            return array.Aggregate(17, (v, b) => v * 23 + b.GetHashCode(), v => v);
        }
    }
}

internal class someClass
{
    public List<byte[]> list1 = new List<byte[]>();
    public ConcurrentDictionary<byte[], int> dict1  = new ConcurrentDictionary<byte[], int>(new ByteArrayComparer());
}

void Main()
{
    someClass obj = new someClass();

    obj.list1.Add(new byte[] { 1, 2 });
    obj.list1.Add(new byte[] { 1, 2 });
    obj.list1.Add(new byte[] { 1, 2 });
    obj.list1.Add(new byte[] { 2, 3 });
    obj.list1.Add(new byte[] { 2, 3 });
    obj.list1.Add(new byte[] { 3, 4 });

    if (obj.list1 != null && obj.list1.Count > 0)
    {
        foreach (byte[] item in obj.list1)
        {
            // upsert operation (add if string not in dictionary; update by incrementing count if it does exist)
            obj.dict1.AddOrUpdate(item, 1, (key, oldValue) => oldValue + 1);
        }
    }
}

The final content of obj.dict1 will be:

Key 01 02 
Value 3 

Key 02 03 
Value 2 

Key 03 04 
Value 1

does it know to use the IEqualityComparer you created based on the constructor ConcurrentDictionary? — IsaacBok, Mar 21 '16 at 23:08
@IsaacBok Check the constructor I've used for the dictionary. That's how I tell it which comparer to use, which in turn tells it how to compare byte arrays. — Mihai Caracostea, Mar 21 '16 at 23:10

AddOrUpdate not updating

2 Answers2