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

Warning when using a collection initializer with more elements than specified in the capacity #89461

Closed
CollinAlpert opened this issue Jul 25, 2023 · 10 comments

Comments

@CollinAlpert
Copy link
Contributor

Specifying the initial capacity of a data structure with a collection initializer is common to improve performance, since the backing array will not have to be resized as often. Consider the following code:

private static readonly List<int> List = new(3)
{
	1,
	2,
	3,
	4
};

In this case, the fourth element was probably added at a later point, rendering the initial capacity of 3 obsolete. A diagnostic should be emitted, warning the developer that the capacity should also be increased when adding elements to the collection initializer. The diagnostic I am proposing should analyze the following types (for now):

  • List<T>
  • HashSet<T>
  • Dictionary<TKey, TValue>
  • ConcurrentDictionary<TKey, TValue>

A code-fix could update the capacity argument to reflect the actual number of elements passed in the collection initializer.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 25, 2023
@ghost
Copy link

ghost commented Jul 25, 2023

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

Specifying the initial capacity of a data structure with a collection initializer is common to improve performance, since the backing array will not have to be resized as often. Consider the following code:

private static readonly List<int> List = new(3)
{
	1,
	2,
	3,
	4
};

In this case, the fourth element was probably added at a later point, rendering the initial capacity of 3 obsolete. A diagnostic should be emitted, warning the developer that the capacity should also be increased when adding elements to the collection initializer. The diagnostic I am proposing should analyze the following types (for now):

  • List<T>
  • HashSet<T>
  • Dictionary<TKey, TValue>
  • ConcurrentDictionary<TKey, TValue>

A code-fix could update the capacity argument to reflect the actual number of elements passed in the collection initializer.

Author: CollinAlpert
Assignees: -
Labels:

area-System.Collections

Milestone: -

@eiriktsarpalis
Copy link
Member

I suspect such a concern will eventually become obsolete once collection literals are available. @stephentoub @CyrusNajmabadi thoughts?

@CollinAlpert
Copy link
Contributor Author

This would still benefit developers targeting older TFMs who reference the analyzer as a NuGet package.

@stephentoub
Copy link
Member

This would still benefit developers targeting older TFMs who reference the analyzer as a NuGet package.

In general, our investment in static analysis is focused on moving forward rather than looking backward.

@CollinAlpert
Copy link
Contributor Author

That makes sense. But still, I doubt everyone will refactor their code to use collection literals as soon as they upgrade to .NET 8.

@eiriktsarpalis
Copy link
Member

Perhaps then this should be an analyzer pointing users towards using collection literals?

@CollinAlpert
Copy link
Contributor Author

That's also a good idea. Would the analyzer consider all collection initializers or only those where a capacity is specified? From what I've seen so far there is no upside to using a collection initializer over a collection literal, correct?

@CyrusNajmabadi
Copy link
Member

Perhaps then this should be an analyzer pointing users towards using collection literals?

Working on the right now :-)

@CyrusNajmabadi
Copy link
Member

From what I've seen so far there is no upside to using a collection initializer over a collection literal, correct

In the vast majority of cases, this is true. There are still some things a collection expression cannot do (like supply a custom dictionary comparer). So there are still times you'd want the initializer form. But my expectation is that you'd use the expression form 99% of the time.

@eiriktsarpalis
Copy link
Member

Thanks. In that case, I'd be inclined to close this.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants