Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ca1841 #23887

Merged
merged 17 commits into from
Apr 23, 2021
Merged

Ca1841 #23887

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions docs/fundamentals/code-analysis/quality-rules/ca1841.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
---
title: "CA1841: Prefer Dictionary Contains methods (code analysis)"
description: "Learn about code analysis rule CA1841: Prefer Dictionary Contains methods"
ms.date: 04/21/2021
ms.topic: reference
f1_keywords:
- "PreferDictionaryContainsMethods"
- "CA1841"
helpviewer_keywords:
- "PreferDictionaryContainsMethods"
- "CA1841"
author: NewellClark
dev_langs:
- CSharp
- VB
---
# CA1841: Prefer Dictionary Contains methods

| | Value |
|-|-|
| **Rule ID** |CA1841|
| **Category** |[Performance](performance-warnings.md)|
| **Fix is breaking or non-breaking** |Non-breaking|

## Cause

This rule locates calls to a `Contains` method on the `Keys` or `Values` collection of an <xref:System.Collections.Generic.IDictionary%602> that could be replaced with a call to a `ContainsKey` or `ContainsValue` method on the dictionary itself.

## Rule description

Calling `Contains` on the `Keys` or `Values` collection can often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself:

- Many dictionary implementations lazily instantiate the key and value collections, which means that accessing the `Keys` or `Values` collection may result in extra allocations.
- You may end up calling an extension method on <xref:System.Collections.Generic.IEnumerable%601> if the keys or values collection uses explicit interface implementation to hide methods on <xref:System.Collections.Generic.ICollection%601>. This can lead to reduced performance, especially when accessing the key collection. Most dictionary implementations are able to provide a fast O(1) containment check for keys, while the `Contains` extension method on <xref:System.Collections.Generic.IEnumerable%601> usually does a slow O(n) containment check.

## How to fix violations

To fix violations, replace calls to `dictionary.Keys.Contains` or `dictionary.Values.Contains` with calls to `dictionary.ContainsKey` or `dictionary.ContainsValue`, respectively.

The following code snippet shows examples of violations, and how to fix them.

```csharp
using System.Collections.Generic;
// Importing this namespace brings extension methods for IEnumerable`1 into scope.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Importing this namespace brings extension methods for IEnumerable`1 into scope.
// Importing this namespace brings extension methods for IEnumerable<T> into scope.

using System.Linq;

class Example
{
void Method()
{
var dictionary = new Dictionary<string, int>();

// Violation
dictionary.Keys.Contains("hello world");

// Fixed
dictionary.ContainsKey("hello world");

// Violation
dictionary.Values.Contains(17);

// Fixed
dictionary.ContainsValue(17);
}
}
```

```vb
Imports System.Collection.Generic
' Importing this namespace brings extension methods for IEnumerable`1 into scope.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
' Importing this namespace brings extension methods for IEnumerable`1 into scope.
' Importing this namespace brings extension methods for IEnumerable(Of T) into scope.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops sorry Youssef.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, it happened that I commented at nearly the same time of you merging 😄
Will do the change in a new PR

' Note that in Visual Basic, this namespace is often imported automatically throughout the project.
Imports System.Linq

Class Example
Private Sub Method()
Dim dictionary = New Dictionary(Of String, Of Integer)

' Violation
dictionary.Keys.Contains("hello world")

' Fixed
dictionary.ContainsKey("hello world")

' Violation
dictionary.Values.Contains(17)

' Fixed
dictionary.ContainsValue(17)
End Sub
End Class
```

## When to suppress warnings

It is safe to suppress warnings from this rule if the code in question is not performance-critical.

## See also

- [Performance rules](performance-warnings.md)
1 change: 1 addition & 0 deletions docs/fundamentals/code-analysis/quality-rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ The following table lists code quality analysis rules.
> |[CA1836: Prefer `IsEmpty` over `Count` when available](ca1836.md) | Prefer `IsEmpty` property that is more efficient than `Count`, `Length`, <xref:System.Linq.Enumerable.Count%60%601%28System.Collections.Generic.IEnumerable%7B%60%600%7D%29> or <xref:System.Linq.Enumerable.LongCount%60%601%28System.Collections.Generic.IEnumerable%7B%60%600%7D%29> to determine whether the object contains or not any items. |
> | [CA1837: Use `Environment.ProcessId` instead of `Process.GetCurrentProcess().Id`](ca1837.md) | `Environment.ProcessId` is simpler and faster than `Process.GetCurrentProcess().Id`. |
> | [CA1838: Avoid `StringBuilder` parameters for P/Invokes](ca1838.md) | Marshaling of 'StringBuilder' always creates a native buffer copy, resulting in multiple allocations for one marshaling operation. |
> | [CA1841: Prefer Dictionary Contains methods](ca1841.md) | Calling `Contains` on the `Keys` or `Values` collection may often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself. |
> | [CA2000: Dispose objects before losing scope](ca2000.md) | Because an exceptional event might occur that will prevent the finalizer of an object from running, the object should be explicitly disposed before all references to it are out of scope. |
> |[CA2002: Do not lock on objects with weak identity](ca2002.md) |An object is said to have a weak identity when it can be directly accessed across application domain boundaries. A thread that tries to acquire a lock on an object that has a weak identity can be blocked by a second thread in a different application domain that has a lock on the same object. |
> | [CA2007: Do not directly await a Task](ca2007.md) | An asynchronous method [awaits](../../../csharp/language-reference/operators/await.md) a <xref:System.Threading.Tasks.Task> directly. When an asynchronous method awaits a <xref:System.Threading.Tasks.Task> directly, continuation occurs in the same thread that created the task. This behavior can be costly in terms of performance and can result in a deadlock on the UI thread. Consider calling <xref:System.Threading.Tasks.Task.ConfigureAwait(System.Boolean)?displayProperty=nameWithType> to signal your intention for continuation. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ Performance rules support high-performance libraries and applications.
| [CA1836: Prefer `IsEmpty` over `Count` when available](ca1836.md) | Prefer `IsEmpty` property that is more efficient than `Count`, `Length`, <xref:System.Linq.Enumerable.Count%60%601%28System.Collections.Generic.IEnumerable%7B%60%600%7D%29> or <xref:System.Linq.Enumerable.LongCount%60%601%28System.Collections.Generic.IEnumerable%7B%60%600%7D%29> to determine whether the object contains or not any items. |
| [CA1837: Use `Environment.ProcessId` instead of `Process.GetCurrentProcess().Id`](ca1837.md) | `Environment.ProcessId` is simpler and faster than `Process.GetCurrentProcess().Id`. |
| [CA1838: Avoid `StringBuilder` parameters for P/Invokes](ca1838.md) | Marshaling of `StringBuilder` always creates a native buffer copy, resulting in multiple allocations for one marshaling operation. |
| [CA1841: Prefer Dictionary Contains methods](ca1841.md) | Calling `Contains` on the `Keys` or `Values` collection may often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself. |
2 changes: 2 additions & 0 deletions docs/fundamentals/toc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,8 @@ items:
href: code-analysis/quality-rules/ca1837.md
- name: CA1838
href: code-analysis/quality-rules/ca1838.md
- name: CA1841
href: code-analysis/quality-rules/ca1841.md
- name: Publish rules
items:
- name: Overview
Expand Down