Don't guard Dictionary<K, V>.Remove(key) by ContainsKey(key)

[terrajobst]

The pair can be combined into just the Remove call.

Category: Performance

[jeffhandley]

Estimates:

• Analyzer: Medium
• Fixer: Medium

[carlossanlop]

Suggested severity: Info

Example

// Before
        public void RemoveValue(Dictionary<string, int> data, string key)
        {​​
            if (data.ContainsKey(key))
                data.Remove(key);
            // or
            if (data.ContainsKey(key))
                data.Remove(key, out var value);
        }​​

// After
        public void RemoveValue(Dictionary<string, int> data, string key)
        {​​
            data.Remove(key);
            // or
            data.Remove(key, out var value1);
        }​​

Avoid triggering the analyzer if there's additional code inside the if statement:

        public void DontTrigger(Dictionary<string, int> data, string key, Dictionary<string, int> otherData)
        {​​
            if (data.ContainsKey(key))
            {
                otherData[key] = data[key]; // Unrelated code
                data.Remove(key);
            }
        }​​

[terrajobst]

Video

• Makes sense.

We could further improve the fixer to handle this as well:

// Before
if (data.ContainsKey(key))
{
    data.Remove(key);
    SomeUnrelatedCode();
}
// After
if (data.Remove(key))
{
    SomeUnrelatedCode();
}

But it's not necessary -- we can start simple and take it from there.

[chucker]

I have the analyzer mostly ready to go for both C# and VB. No fixer yet, though.

Do you want a PR, or should I first write a fixer?

[carlossanlop]

Thanks for your help, @chucker! I'm assigning the issue to you. Feel free to submit a PR whenever you get a chance, and we can take a look.

[chucker]

(The PR linked above does now include the fixer, too.)

[paulomorgado]

is there an issue like this for HashSet<T>?

• Don't guard HashSet<T>.Remove(item) by HashSet<T>.Contains(item)?
• Don't guard HashSet<T>.Add(item) by HashSet<T>.Contains(item)?

[carlossanlop]

@paulomorgado if you can't find any issue, please create an API proposal.

[paulomorgado]

An API proposal????

[chucker]

@paulomorgado yes — make a new issue over here

See also these guidelines.