2

In my code I got an ConcurrentDictionary now I want to iterate over each element in the Dictionary, but if a condition is true I want to remove an element from this Dictionary so I can't use a foreach loop. Also it might happen that the Dictionary will get a new element while in the loop or get one removed from a different thread. After some research I ended up using ElementAt.

Now my question is if the ConcurrentDictionary will release the indexes again like a List does. So that the first element will always have the index 0.

This is what my code looks like, CommandHandler.Timeouter is from ConcurrentDictionary:

int current = 0;

while (CommandHandler.Timeouter.Count() > current)
{
    var info = CommandHandler.Timeouter.ElementAt(current);
    var timeoutcooldown = info.Value.LastCommandTime.AddMinutes(1);

    if (timeoutcooldown < DateTime.UtcNow)
       {
           CommandHandler.Timeouter.TryRemove(info.Key, out _);
       }
    else current++;
}
Twenty
  • 5,234
  • 4
  • 32
  • 67
  • 1
    Note that dictionaries do not guarantee a certain sequential order of elements (or key-value-pairs, for that matter). In other words, the order of elements in dictionaries is non-deterministic. If you ask whether an element at index 0 (as used by the Linq extension method `ElementAt`) will always stay at index 0, then no, this is not guaranteed... –  Nov 11 '18 at 10:05
  • Well this is bad.I guess I might end up using a List and using Linq to search for what I need. – Twenty Nov 11 '18 at 10:07
  • This is a code snippet from what should end up as an spam protection. Since whenever a user writes a message the code will check how many messages he wrote in a specific amount of time and then timeout him. I went for a Dictionary since I can get a specific user from a list of datas. Also I do not care about the order. I just want to clean up the list every few minutes. @elgonzo – Twenty Nov 11 '18 at 10:12
  • 2
    You can iterate just fine through a ConcurrentDictionary (see here: https://stackoverflow.com/questions/17776422/is-this-the-proper-way-to-iterate-over-concurrentdictionary-in-c-sharp). If new elements are added to the ConcurrentDictionary by some other thread during an iteration then they might not be part of the iteration. But that isn't a problem as such new elements would then be subject of the next clean-up iteration a few minutes later. (1/2) –  Nov 11 '18 at 10:19
  • 1
    (2/2) ConcurrentDictionary also allow removal of elements within an iteration loop using ConcurrentDictionary.TryRemove (see here: https://stackoverflow.com/questions/2318005/can-i-remove-items-from-a-concurrentdictionary-from-within-an-enumeration-loop-o). See, StackOverflow has all the answers ;-) –  Nov 11 '18 at 10:19
  • Oh, so since it is a ConcurrentDictionary I can remove and add elements to it while iterating over it. Thats sweet. Thanks! – Twenty Nov 11 '18 at 10:20
  • 3
    Yes, but keep in mind to use ONLY methods provided by ConcurrentDictionary itself. No Linq or other extension methods! (because ConcurrentDictionary can ensure thread safety only with regard to its own methods) –  Nov 11 '18 at 10:21
  • I'd suggest reading up on `MemoryCache` as a possible replacement of your logic here. https://learn.microsoft.com/en-us/dotnet/api/system.runtime.caching.memorycache?view=netframework-4.7.2 – mjwills Nov 11 '18 at 12:01
  • @mjwills Is there any advantage using a MemoryCache instead of a ConcurrentDictionary? – Twenty Nov 11 '18 at 15:05
  • @Twenty The main benefit is you won't need to manually remove expired entries - it does that for you. – mjwills Nov 11 '18 at 20:40
  • @mjwills So if it is performance whise the same thanks a lot man! – Twenty Nov 11 '18 at 23:37
  • Performance characteristics may not be the same, and functionality may not be 100% the same - so be sure to test whether it is appropriate for your context. – mjwills Nov 11 '18 at 23:38
  • Thanks a lot then! @mjwills – Twenty Nov 11 '18 at 23:42

1 Answers1

3

ElementAt just treats the dictionary as an IEnumerable<KeyValuePair<TKey, TValue>>. Dictionaries are not ordered. The index is therefore meaningless. Think of the elements coming back in random order each time. Also, ElementAt has no way to make this thread safe.

It seems you want to implement cache expiration. Consider just using lock to access a normal dictionary. If there is not much contention this will be the simplest solution and very fast.

An alternative code pattern to this loop would be this:

var itemsToExpire = myDict.Where(/* compute expiration */).ToList();
foreach (var item in itemsToExpire)
   myDict.Remove(item);

No need for any complicated looping.

usr
  • 168,620
  • 35
  • 240
  • 369
  • Thanks a lot for the answer, since elgonzo already guided me, I will not mark this as answer but an upvote for sure. Thanks! – Twenty Nov 11 '18 at 10:24