Convert `ArrayList` usage to `List<T>` where possible

[elachlan]

Is your feature request related to a problem? Please describe

Winforms uses Arraylist extensively.

Describe the solution you'd like and alternatives you've considered

Convert Arraylist usage to List<T> to reduce boxing and improve memory utilization.

Will this feature affect UI controls?

N/A

Related: #2644

Remaining occurrences in System.Windows.Forms.Design

File	Line Count
src\System\ComponentModel\Design\ExceptionCollection.cs	33
src\System\Windows\Forms\Design\ToolStripItemDataObject.cs	34
src\System\Windows\Forms\Design\ControlDesigner.DesignerControlCollectionCodeDomSerializer.cs	38
src\System\Windows\Forms\Design\UpDownBaseDesigner.cs	63
src\System\Windows\Forms\Design\DesignBindingValueUIHandler.cs	66
src\System\Windows\Forms\Design\ImageListImageEditor.cs	108
src\System\Windows\Forms\Design\ImageCollectionEditor.cs	113
src\System\Windows\Forms\Design\GroupBoxDesigner.cs	124
src\System\Windows\Forms\Design\ComboBoxDesigner.cs	128
src\System\Windows\Forms\Design\LabelDesigner.cs	146
src\System\Windows\Forms\Design\TextBoxBaseDesigner.cs	148
src\System\Windows\Forms\Design\OleDragDropHandler.CfCodeToolboxItem.cs	162
src\System\Windows\Forms\Design\ButtonBaseDesigner.cs	218
src\System\Windows\Forms\Design\Behavior\ContainerSelectorBehavior.cs	230
src\System\Windows\Forms\Design\ToolStripAdornerWindowService.cs	256
src\System\Windows\Forms\Design\BaseContextMenuStrip.cs	290
src\System\Windows\Forms\Design\OleDragDropHandler.ComponentDataObject.cs	302
src\System\ComponentModel\Design\InheritedPropertyDescriptor.cs	308
src\System\Windows\Forms\Design\Behavior\ToolboxItemSnapLineBehavior.cs	338
src\System\ComponentModel\Design\CollectionEditor.cs	347
src\System\Windows\Forms\Design\FormDocumentDesigner.cs	432
src\System\ComponentModel\Design\SelectionService.cs	446
src\System\Windows\Forms\Design\Behavior\SelectionManager.cs	458
src\System\ComponentModel\Design\DesignSurface.cs	491
src\System\Windows\Forms\Design\DesignerFrame.cs	576
src\System\Windows\Forms\Design\ToolStripDesignerUtils.cs	584
src\System\ComponentModel\Design\Serialization\CollectionCodeDomSerializer.cs	648
src\System\Windows\Forms\Design\DesignerUtils.cs	816
src\System\Windows\Forms\Design\Behavior\ResizeBehavior.cs	819
src\System\ComponentModel\Design\Serialization\BasicDesignerLoader.cs	857
src\System\ComponentModel\Design\Serialization\DesignerSerializationManager.cs	896
src\System\Windows\Forms\Design\ToolStripItemBehavior.cs	905
src\System\Windows\Forms\Design\Behavior\DropSourceBehavior.cs	1029
src\System\Windows\Forms\Design\OleDragDropHandler.cs	1045
src\System\Windows\Forms\Design\ToolStripItemDesigner.cs	1177
src\System\ComponentModel\Design\CollectionEditor.CollectionEditorCollectionForm.cs	1223
src\System\ComponentModel\Design\Serialization\CodeDomComponentSerializationService.cs	1471
src\System\ComponentModel\Design\DesignerHost.cs	1615
src\System\Windows\Forms\Design\ToolStripKeyboardHandlingService.cs	1891
src\System\Windows\Forms\Design\ControlDesigner.cs	2205
src\System\Windows\Forms\Design\ToolStripDesigner.cs	2260
src\System\Windows\Forms\Design\ParentControlDesigner.cs	2378
src\System\Windows\Forms\Design\ToolStripMenuItemDesigner.cs	2527
src\System\ComponentModel\Design\Serialization\CodeDomSerializerBase.cs	3020

[ghost]

This issue is now marked as "help wanted", and we’re looking for a community volunteer to work on this issue. If we receive no interest in 180 days, we will close the issue. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

[merriemcgaw]

This is a great idea, and a wonderful place for the community to engage and find the right areas to make changes like this. Of course, we'd need to avoid making this happen on the public surface area 😄

[elachlan]

One issue I have run into is that there are a few collections which expose IList.IsFixedSize, List<T> doesn't have that property. I don't think FixedSize() gets called either. So it might be an idea to change IsFixedSize to just return false?

[Jericho]

I would be happy to contribute. I will start with combing through the code base to identify the various classes that would be affected.

[elachlan]

@Jericho awesome! Just respond here and I will add it to the issue description.

I think the easiest way to get this done is to fix them one by one instead of batches. I suspect it will be nicer for reviewers and isolates any regressions. It also speeds up the time is takes to merge.

[RussKie]

the easiest way to get this done is to fix them one by one instead of batches. I suspect it will be nicer for reviewers and isolates any regressions. It also speeds up the time is takes to merge.

Yes, that's the best way. The smaller and the more targeted a PR - the easier it is to review and faster to merge. That said, it's easier to look at a PR that makes the same changes in 50 files than to look at 50 PRs making the same change in one file. So it's a matter of balance and good judgement.

[Jericho]

I think the easiest way to get this done is to fix them one by one instead of batches.

@elachlan Thanks for the guidance. I'll also keep @RussKie 's suggestion in mind and combine my PRs into a single, larger PR if there are too many.

[Jericho]

Here's the list of classes that will benefit from replacing ArrayList with List<T> (I'll continuously maintain this list as I discover more classes):

• [x] WeakRefCollection: use List<WeakRefObject?> instead of ArrayList
  • Addressed in #8189
• [x] AutoCompleteStringCollection:
  • @elachlan already took care of this in #8142
• [x] ArrangedElementCollection: use List<IArrangedElement> instead of ArrayList
  • Addressed in #8207
• [x] TableLayoutStyleCollection: use List<TableLayoutStyle> instead of ArrayList
  • Addressed in #8212
• [x] DesignerEventService: use List<IDesignerHost> instead of ArrayList
  • Addressed by @elachlan in #8181
• [x] ExtenderProviderService: use List<IExtenderProvider> instead of ArrayList
  • Addressed by @elachlan in #8182
• [x] MenuCommandService: use List<DesignerVerb> instead of ArrayList
  • Addressed by @elachlan in #8183
• [x] ReferenceService: there are three ArrayList. Two of them can be replaced with List<IComponent> and the other one can be replaced with List<ReferenceHolder>
  • Addressed by @elachlan in #8185
• [x] CodeDomComponentSerializationService: there are two ArrayList. One can be replaced with List<string> and the other one can be replaced with List<CodeExpression>
  • Addressed in #8214
• [x] UndoEngine: there are five array lists in the class. Two of them can be replaced with List<IComponent>, and the remaining three can be replaced with List<UndoEvent>, List<ChangeUndoEvent> and List<AddRemoveUndoEvent> respectively.
  • Addressed in #8215
• [x] BehaviorService: replace ArrayList with List<Behavior>
  • Addressed by @elachlan in #8187
• [x] DragAssistanceManager: there are seven ArrayList in this class. Four of them can be replaced with List<SnapLine>, one can be replaced with List<SnapLineType> and two can be replaced with List<Line>
  • Addressed by @elachlan in #8200
• [x] DocumentDesigner: replace ArrayList with List<IComponent>
  • Addressed by @elachlan in #8194
• [x] PbrsForward: replace ArrayList with List<BufferedKey>
  • Addressed by @elachlan in #8193
• [x] ToolStripKeyboardHandlingService: there are two ArrayList that can be replaced with List<MenuCommand>
  • Addressed by @elachlan in #8195
• [x] DataGridView: replace ArrayList with List<DataGridViewRow>
  • Addressed in #8188
• [x] DataGridViewCellCollection: replace ArrayList with List<DataGridViewCell>
  • Addressed in #8188
• [x] DataGridViewColumnCollection: replace ArrayList with List<DataGridViewColumn>
  • @elachlan already took care of this in #8153
• [x] DataGridViewSelectedCellCollection: replace ArrayList with List<DataGridViewCell>
  • Addressed in #8188
• [x] DataGridViewSelectedColumnCollection: replace ArrayList with List<DataGridViewColumn>
  • Addressed in #8188
• [x] DataGridViewSelectedRowCollection: replace ArrayList with List<DataGridViewRow>
  • Addressed in #8188
• [x] PropertyGridView: replace ArrayList with List<GridEntryCollection>
  • Addressed in #8210
• [x] BindingCollection: replace ArrayList with List<Binding>
  • Adressed in #8364
• [ ] Com2IManagedPerPropertyBrowsingHandler: replace ArrayList with List<Attribute>

Here are classes that use an ArrayList and the type of the elements is object. Is it worth it to convert to List<object>?

• [x] DataGridViewComboBoxCell
  • Addressed in #8188
• [ ] SelectionService: Currently the ArrayList contains objects but I suspect the intent is to keep a reference of visual components rather than objects. Therefore, I think List<IComponent> would be more appropriate

[elachlan]

@dreddy-work I think this can be refactored to remove the usage or ArrayList. But I don't understand the adding of the null value to the list. Can you make sense of it at all?

https://github.com/dotnet/winforms/blob/5aca3ed6f324894e8f00c67c7c01374da5c5d6d8/src/System.Windows.Forms/src/System/Windows/Forms/ListViewGroupConverter.cs#L113-L133

[elachlan]

@dreddy-work when an ICollection or IList is used as the type for a parameter and we are passing in an ArrayList, can we convert our code to pass in List<T>? Its technically compatible, but I am unsure if the expectation is that if we have a public API that is say IList, should the returned value be an ArrayList?

[dreddy-work]

@dreddy-work when an ICollection or IList is used as the type for a parameter and we are passing in an ArrayList, can we convert our code to pass in List<T>?

internal or private we could change them. General rule on public APIs surface change is, Business justifications vs impact analysis.

Its technically compatible, but I am unsure if the expectation is that if we have a public API that is say IList, should the returned value be an ArrayList?

Can you elaborate on this? I didn't get it fully.

[dreddy-work]

@dreddy-work I think this can be refactored to remove the usage or ArrayList. But I don't understand the adding of the null value to the list. Can you make sense of it at all?

What i see is, some typeconverters include none or null in standard value collections depending on the type of the converter it was designed for and whether it could be assigned to them. In this case, i see it is intentional.

[halgab]

I also see a few examples of Stacks that can be easily replaced with Stack<T>. Are you interested in such changes? There is no boxing involved in those cases though.

[elachlan]

If the API surface isn't public and then yes. Maybe create a separate issue for it and add a link back here.

[dreddy-work]

Agree with @elachlan. Just need to be cautious on public APIs and indirect public surface area.

[halgab]

okay, done: #8728

[halgab]

Same question with the internal uses of StringCollection (I found only one to be fair). And does it also deserve its own issue?

[jlechem]

Are there still areas I can fix for this issue?