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):

• WeakRefCollection: use List<WeakRefObject?> instead of ArrayList
  • Addressed in Refactor WeakRefCollection to use List<WeakRefObject?> instead of ArrayList  #8189
• AutoCompleteStringCollection:
  • @elachlan already took care of this in Refactor AutoCompleteStringCollection to use List<string> instead of ArrayList #8142
• ArrangedElementCollection: use List<IArrangedElement> instead of ArrayList
  • Addressed in Refactor ArrangedElementCollection to use List<IArrangedElement> instead of ArrayList #8207
• TableLayoutStyleCollection: use List<TableLayoutStyle> instead of ArrayList
  • Addressed in Refactor TableLayoutStyleCollection to use List<TableLayoutStyle> instead of ArrayList #8212
• DesignerEventService: use List<IDesignerHost> instead of ArrayList
  • Addressed by @elachlan in Refactor DesignerEventService to use List<IDesignerHost> instead of ArrayList #8181
• ExtenderProviderService: use List<IExtenderProvider> instead of ArrayList
  • Addressed by @elachlan in Refactor ExtenderProviderService to use List<IExtenderProvider> instead of ArrayList #8182
• MenuCommandService: use List<DesignerVerb> instead of ArrayList
  • Addressed by @elachlan in Refactor MenuCommandService to use List<T> instead of ArrayList #8183
• 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 Refactor ReferenceService to use List<T> instead of ArrayList #8185
• CodeDomComponentSerializationService: there are two ArrayList. One can be replaced with List<string> and the other one can be replaced with List<CodeExpression>
  • Addressed in Refactor CodeDomComponentSerializationService to use List<T> instead of ArrayList #8214
• 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 Refactor UndoEngine to use List<T> instead of ArrayList #8215
• BehaviorService: replace ArrayList with List<Behavior>
  • Addressed by @elachlan in Refactor BehaviorService to use List<T> instead of ArrayList #8187
• 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 Refactor CommandSet and associated classes to use List<T> and Dictionary<TKey, TValue> #8200
• DocumentDesigner: replace ArrayList with List<IComponent>
  • Addressed by @elachlan in Refactor DocumentDesigner to use List<T> instead of ArrayList #8194
• PbrsForward: replace ArrayList with List<BufferedKey>
  • Addressed by @elachlan in Refactor PbrsForward to use List<T> instead of ArrayList #8193
• ToolStripKeyboardHandlingService: there are two ArrayList that can be replaced with List<MenuCommand>
  • Addressed by @elachlan in Refactor ToolStripKeyboardHandlingService to use List<T> instead of ArrayList #8195
• DataGridView: replace ArrayList with List<DataGridViewRow>
  • Addressed in Refactor several data grid view classes to use List<T> instead of ArrayList #8188
• DataGridViewCellCollection: replace ArrayList with List<DataGridViewCell>
  • Addressed in Refactor several data grid view classes to use List<T> instead of ArrayList #8188
• DataGridViewColumnCollection: replace ArrayList with List<DataGridViewColumn>
  • @elachlan already took care of this in Refactor DataGridViewColumnCollection to replace ArrayList #8153
• DataGridViewSelectedCellCollection: replace ArrayList with List<DataGridViewCell>
  • Addressed in Refactor several data grid view classes to use List<T> instead of ArrayList #8188
• DataGridViewSelectedColumnCollection: replace ArrayList with List<DataGridViewColumn>
  • Addressed in Refactor several data grid view classes to use List<T> instead of ArrayList #8188
• DataGridViewSelectedRowCollection: replace ArrayList with List<DataGridViewRow>
  • Addressed in Refactor several data grid view classes to use List<T> instead of ArrayList #8188
• PropertyGridView: replace ArrayList with List<GridEntryCollection>
  • Addressed in Refactor PropertyGridView to use List<GridEntryCollection> instead of ArrayList #8210
• BindingCollection: replace ArrayList with List<Binding>
  • Adressed in Refactor BindingCollection to replace ArrayList with List<Binding> #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>?

• DataGridViewComboBoxCell
  • Addressed in Refactor several data grid view classes to use List<T> instead of ArrayList #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?

winforms/src/System.Windows.Forms/src/System/Windows/Forms/ListViewGroupConverter.cs

Lines 113 to 133 in 5aca3ed

	/// <summary>
	/// Retrieves a collection containing a set of standard values for the data type this
	/// validator is designed for. This will return null if the data type does not support
	/// a standard set of values.
	/// </summary>
	public override StandardValuesCollection? GetStandardValues(ITypeDescriptorContext? context)
	{
	if (context is not null && context.Instance is ListViewItem item && item.ListView is not null)
	{
	var list = new ArrayList();
	foreach (ListViewGroup group in item.ListView.Groups)
	{
	list.Add(group);
	}
	list.Add(null);
	return new StandardValuesCollection(list);
	}
	return null;
	}

[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?