Convert Hashtable usage to Dictionary<TKey, TValue

[elachlan]

Is your feature request related to a problem? Please describe

Hashtable has been used all over the winforms code base.

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

Refactoring to Dictionary<TKey, TValue is explicit in its typing and avoids boxing.

The changes should not affect Public APIs.

System.Windows.Forms: 31 references
System.Windows.Forms.Design: 126 references

Related: #8140, #2644

[elachlan]

HelpProvider has a few Hashtables. _boundControls a bit weird since the key and value are the same and of type Control.

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

Line 26 in ba551ea

private readonly Hashtable _boundControls = new Hashtable();

I thought this might be a good use of Hashset<T>, but this suggests the performance isn't as good as List<T> for objects until after 20 items. https://stackoverflow.com/a/10762995/908889

Does the team have a suggestion here?

[elachlan]

@Jericho, you might want to jump in on this issue as well.

[Jericho]

@elachlan sure. As soon as I'm done working on #8140, I'll inventory the remaining HashTable and create a list to track progress (like I did for the ArrayList).

[elachlan]

I attempted WeakHashtable. It is a complicated one. I am unsure how to proceed.

Maybe add a type parameter and implement IDictionary<object, TValue>? That would allow using a private Dictionary<EqualityWeakReference, TValue>, thus hiding the implementation and avoiding boxing.

[Jericho]

I'm done with #8140 for the time being so I'll start contributing here. First priority: create a list of the classes that need to be refactored (similar to what I did here). This will enable us to track our progress.

[Jericho]

Here's the list of classes that contains at least one private property of type HashTable:

• AssemblyNamesTypeResolutionService; Addressed by @elachlan in Refactor AssemblyNamesTypeResolutionService to replace HashTable #8213
• AxHost.PropertyBagStream
• BehaviorService
• BindingContext
• CodeDomLocalizationProvider
• ComNativeDescriptor
• ComponentDesigner.ShadowPropertyCollection; Addressed in Refactor ComponentDesigner.ShadowPropertyCollection to use Dictionary<TKey, TValue> instead of Hashtable #8222
• ComponentDesigner;Addressed in Refactor ComponentDesigner to use Dictionary<TKey, TValue> instead of Hashtable #8221
• Control.ActiveXImpl.PropertyBagStream
• DataGridView; Addressed by @elachlan in Refactor DataGridView to replace HashTable #8236
• DesignerActionService
• DesignerActionUI
• DesignerHost; Addressed in Refactor DesignerHost to use Dictionary<IComponent, IDesigner> instead of Hashtable #8272
• DesignerSerializationManager
• DesignSurfaceServiceContainer
• DocumentDesigner
• ExpressionTable
• HelpProvider; Addressed by @elachlan in Refactor HelpProvider to use Dictionary instead of Hashtable #8164
• InheritanceService
• LocalizationCodeDomSerializer
• MultilineStringEditor.MultilineStringEditorUI
• ResourceCodeDomSerializer
• ResXResourceWriter; Addressed by @elachlan in Refactor ResXResourceWriter to use Dictionary<string, string> instead of Hashtable #8160
• SelectionManager
• SelectionUIService
• TabOrder
• ToolStripControlHost.StubSite
• ToolTip; Addressed by @elachlan in Refactor ToolTip to replace HashTable #8227
• UndoEngine
• WebBrowserContainer; Addressed by @elachlan in Refactor WebBrowserContainer to replace HashTable and Enable Nullability #8226

[elachlan]

ImmutablePropertyDescriptorGridEntry.PropertyValue is something I am not 100% sure on. It creates an HashTable into an IDictionary variable, then passes that to TypeConverter.CreateInstance. I imagine the underlying converter probably relies on it being a hashtable? Anyone have any ideas on where the TypeConverter code is located?

winforms/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGridInternal/ImmutablePropertyDescriptorGridEntry.cs

Lines 47 to 65 in 4f4829d

	IDictionary values = new Hashtable(properties.Count);
	object newObject;
	for (int i = 0; i < properties.Count; i++)
	{
	if (PropertyDescriptor.Name is not null && PropertyDescriptor.Name.Equals(properties[i].Name))
	{
	values[properties[i].Name] = value;
	}
	else
	{
	values[properties[i].Name] = properties[i].GetValue(owner);
	}
	}
	try
	{
	newObject = parentConverter.CreateInstance(parentEntry, values);
	}

[halgab]

I see that ContextMenuStripGroupCollection, which is internal, overrides DictionaryBase, which is basically just a wrapper around a HashTable with extra events we're not using. As part of this issue, do you agree it would be a good idea for this class to use a Dictionary<string, ContextMenuStripGroup> directly instead?

[elachlan]

@lonitra I believe the remaining usages of HashTable to either be:

• a part of the Public API
• used for Binary Serializer purposes
• passed to public APIs defined as IDictionary. (maybe we can use a dictionary adapter here?)

Could the team verify that the remaining items can't be refactored, maybe we can close this issue off?