Refactor `PropertyStore` and review/update usage
- Dominant language
- C#
- Stars
- 4.9k
- Forks
- 1.1k
- Avg merge
- 20h 23m
- Merged PRs (30d)
- 103
Description
### Background and motivation
We have had a few bugs caused by incorrect usage of `PropertyStore`. It's clunky and we can use private backing fields instead. This would allow nullable annotations and compile time checks which would have prevented #8990
Also can look into using a dictionary instead to gain perf improvements. But
Issue raised based on comment: https://github.com/dotnet/winforms/pull/9503#pullrequestreview-1533416827
### API Proposal
```csharp
internal class PropertyStore
{
private sealed class ValueWrapper where T : struct
{
public T Value { get; }
public ValueWrapper(T value)
{
Value = value;
}
}
private readonly Dictionary properties = new();
private static int s_currentKey = 0;
public static int CreateKey() => s_currentKey++;
public void SetValue(int key, T value) where T : struct
{
properties[key] = new ValueWrapper(value);
}
public void SetObject(int key, T value)
{
properties[key] = value;
}
public T GetValue(int key) where T : struct
{
if (properties.TryGetValue(key, out var value) && value is ValueWrapper wrapper)
{
return wrapper.Value;
}
return default;
}
public T? GetObject(int key)
{
return properties.TryGetValue(key, out var value) && value is T tValue ? tValue : default;
}
public bool TryGetValue(int key, out T? value) where T : struct
{
if (properties.TryGetValue(key, out var objValue) && objValue is ValueWrapper wrapper)
{
value = wrapper.Value;
return true;
}
value = default;
return false;
}
public bool TryGetObject(int key, out T? value) where T : class
{
if (properties.TryGetValue(key, out var objValue) && objValue is T tValue)
{
value = tValue;
return true;
}
value = null;
return false;
}
public bool Remove(int key) => properties.Remove(key);
}
```
### API Usage
N/A
### Alternative Designs
_No response_
### Risks
It is a large refactor and could cause subtle bugs. The current design is meant for performance reasons (I don't understand why?).
### Will this feature affect UI controls?
Controls will need testing.
Contributor guide
Assessment
This issue has not been assessed yet.