CODEWITHSUNDEEP
c#linqdictionary
Evorlor
Evorlor

Reputation: 7553

What is the best way to remove all instances from a Dictionary?

I have a dictionary. The same instance of an object can spread across multiple keys. I am writing a method so that you can pass the instance, and all keys with that instance will be removed.

The following works, but I am weak with Linq. Is there a better approach to this? I am interested in elegant code, but I am moreso focused on speed. How can I make this code run faster (without removing whether or not the removal was successful)?

/// <summary>
/// Removes obstacle from the level, and returns true if the removal was successful
/// </summary>
public bool Remove(Obstacle obstacle)
{
    if (!obstacleMap.ContainsValue(obstacle))
    {
        return false;
    }
    foreach (var key in obstacleMap.Keys.Where(k => obstacleMap[k] == obstacle))
    {
        obstacleMap.Remove(key);
    }
    return true;
}

Upvotes: 0

Views: 529

Answers (4)

Denis.Coelho
Denis.Coelho

Reputation: 31

Try this:

Instead of:

foreach (var key in obstacleMap.Keys.Where(k => obstacleMap[k] == obstacle))
{
    obstacleMap.Remove(key);
}

Use this:

 var fObstacleMap = obstacleMap.Where(x => x.Value != obstacle).ToDictionary(y => y.Key, y => y.Value);

This create another Dictionary without the object passed by obstacle parameter and don't throw exception.

Upvotes: 0

Vishal Madhvani
Vishal Madhvani

Reputation: 442

You can also use Grouping. Not sure which is more performant.

var newDictionary = dictionary.GroupBy(_ => _.Value)
                              .Where(_ => _.Key != valueToRemove)
                              .ToDictionary(_=> _.Single(), _ => _.Key);

DotNetFiddle: https://dotnetfiddle.net/gRgCeX

Upvotes: 0

Ivan Stoev
Ivan Stoev

Reputation: 205579

Here is a more efficient way to do that, which also avoids exception when removing items from the dictionary while enumerating it:

public bool Remove(Obstacle obstacle)
{
    var removeKeys = obstacleMap.Where(e => e.Value == obstacle).Select(e => e.Key).ToList();
    foreach (var key in removeKeys) obstacleMap.Remove(key);
    return removeKeys.Count > 0;
}

Upvotes: 2

itsme86
itsme86

Reputation: 19486

The fastest way would probably be to create a second dictionary with the obstacle object as the key and the value would be a list of keys in the first dictionary which reference the object.

The way your dictionary is set up now, it doesn't lend itself well to what you're trying to do in your Remove() method. Key lookups are much faster than value lookups.

List<TKey> keys;
if (dict2.TryGetValue(obstacle, out keys))
{
    foreach (TKey key in keys)
    {
        obstacleMap.Remove(key);
    }

    dict2.Remove(obstacle);
}

Just remember to update both dictionaries when adding an obstacle.

Upvotes: 0

Related Questions