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

Add convenience / perf methods to ListViewItem/ControlCollection #10944

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JeremyKuhne
Copy link
Member

@JeremyKuhne JeremyKuhne commented Feb 23, 2024

ListViewItemCollection doesn't have an AddRange that takes an IEnumerable<ListViewItem>, which necessitates unnecessary array allocations when adding from a collection.

It also doesn't implement a generic IList<T> which makes typed usage and LINQ usage difficult. This adds that.

Also add IEnumerable<Control> to ControlCollection to address the LINQ scenario. Control has IList, but indexed setting throws. I don't want to add IList<Control> until we've had time to evaluate the implications of implicitly doing the public steps with the collection that replicate what those setters should be doing.

Also tweak ArrangedElementCollection so the static empty collection can't be written to.

This doesn't fix the ambiguity of AddRange([..]) with ListViewItemCollection, but it does avoid the need for it in some cases.

Microsoft Reviewers: Open in CodeFlow

lonitra
lonitra previously approved these changes Feb 23, 2024
Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

🚀

Copy link

codecov bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 54.54545% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 74.21761%. Comparing base (1e92b29) to head (2bad759).
Report is 1 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #10944         +/-   ##
===================================================
- Coverage   74.22517%   74.21761%   -0.00757%     
===================================================
  Files           3020        3020                 
  Lines         626328      626350         +22     
  Branches       46693       46701          +8     
===================================================
- Hits          464893      464862         -31     
- Misses        158039      158096         +57     
+ Partials        3396        3392          -4     
Flag Coverage Δ
Debug 74.21761% <54.54545%> (-0.00757%) ⬇️
integration 18.24226% <21.53846%> (-0.02464%) ⬇️
production 46.86395% <53.84615%> (-0.01310%) ⬇️
test 97.03589% <100.00000%> (-0.00118%) ⬇️
unit 43.83024% <53.84615%> (-0.00163%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

…numerable<ListViewItem>`, which necessitates unnecessary array allocations when adding from a collection.

It also doesn't implement a generic `IList<T>` which makes typed usage and LINQ usage difficult. This adds that.

Also add `IEnumerable<Control>` to `ControlCollection` to address the LINQ scenario. Control has `IList`, but indexed setting throws. I don't want to add `IList<Control>` until we've had time to evaluate the implications of implicitly doing the public steps with the collection that replicate what those setters should be doing.

Also tweak `ArrangedElementCollection` so the static empty collection can't be written to.

This doesn't fix the ambiguity of `AddRange([..])` with `ListViewItemCollection`, but it does avoid the need for it in some cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants