CODEWITHSUNDEEP
c#performancedictionarydata-structures
Sam Marion
Sam Marion

Reputation: 690

Poor use of dictionary?

I've read on here that iterating though a dictionary is generally considered abusing the data structure and to use something else.

However, I'm having trouble coming up with a better way to accomplish what I'm trying to do.

When a tag is scanned I use its ID as the key and the value is a list of zones it was seen in. About every second I check to see if a tag in my dictionary has been seen in two or more zones and if it has, queue it up for some calculations. 

for (int i = 0; i < TagReads.Count; i++)
{
    var tag = TagReads.ElementAt(i).Value;
    if (tag.ZoneReads.Count > 1)
    {
        Report.Tags.Enqueue(tag);
        Boolean success = false;
        do
        {
            success = TagReads.TryRemove(tag.Epc, out TagInfo outTag);
        } while (!success);
    }
}

I feel like a dictionary is the correct choice here because there can be many tags to look up but something about this code nags me as being poor.

As far as efficiency goes. The speed is fine for now in our small scale test environment but I don't have a good way to find out how it will work on a massive scale until it is put to use, hence my concern.

Upvotes: 2

Views: 299

Answers (3)

Mat&#237;as Fidemraizer
Mat&#237;as Fidemraizer

Reputation: 64943

I believe that there's an alternative approach which doesn't involve iterating a big dictionary.

First of all, you need to create a HashSet<T> of tags on which you'll store those tags that have been detected in more than 2 zones. We'll call it tagsDetectedInMoreThanTwoZones.

And you may refactor your code flow as follows:

A. Whenever you detect a tag in one zone...

  1. Add the tag and the zone to the main dictionary.
  2. Create an exclusive lock against tagsDetectedInMoreThanTwoZones to avoid undesired behaviors in B..
  3. Check if the key has more than one zone. If this is true, add it to tagsDetectedInMoreThanTwoZones.
  4. Release the lock against tagsDetectedInMoreThanTwoZones.

B. Whenever you need to process a tag which has been detected in more than one zone...

  1. Create an exclusive lock against tagsDetectedInMoreThanTwoZones to avoid more than a thread trying to process them.
  2. Iterate tagsDetectedInTwoOrMoreZones.
  3. Use each tag in tagsDetectedInMoreThanTwoZones to get the zones in your current dictionary.
  4. Clear tagsDetectedInMoreThanTwoZones.
  5. Release the exclusive lock against tagsDetectedInMoreThanTwoZones.

Now you'll iterate those tags that you already know that have been detected in more than a zone!

In the long run, you can even make per-region partitions so you never get a tagsDetectedInMoreThanTwoZones set with too many items to iterate, and each set could be consumed by a dedicated thread!

Upvotes: 3

Titian Cernicova-Dragomir
Titian Cernicova-Dragomir

Reputation: 250286

If you are going to do a lot of lookup in your code and only sometimes iterate through the whole thing, then I think the dictionary use is ok. I would like to point out thought that your use of ElementAt is more alarming. ElementAt performs very poorly when used on objects that do not implement IList<T> and the dictionary does not. For IEnumerables<T> that do not implement IList the way the nth element is found is through iteration, so your for-loop will iterate the dictionary once for each element. You are better off with a standard foreach.

Upvotes: 3

bic
bic

Reputation: 907

I feel like this is a good use for a dictionary, giving you good access speed when you want to check if an ID is already in the collection.

Upvotes: 0

Related Questions