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

DataDictionary.Empty is not protected by potential bugs. #280

Open
frabe1579 opened this issue Oct 31, 2022 · 3 comments
Open

DataDictionary.Empty is not protected by potential bugs. #280

frabe1579 opened this issue Oct 31, 2022 · 3 comments

Comments

@frabe1579
Copy link

I've found a potential source of bugs:
the class Foundatio.Utility.DataDictionary exposes a static readonly field Empty to represent empty dictionaries, but it's instance is not readonly, so values can be added to Empty causing potential bugs.
The current declaration is:

public static readonly DataDictionary Empty = new();

A new safe declaration shoud be

public static DataDictionary Empty => new();

This at least returns always empty dictionaries, but creates a new instance at every call.
Another approach should be to re-implement DataDictionary with explicit implementation of IDictionary and IReadOnlyDictionary, or something else.

@niemyjski
Copy link
Member

Yes, we should work on making empty be read-only. @ejsmith thoughts on above, kind of would be nice to make it truly readonly instead of returning a new instance. But would take a little bit of work.

@ejsmith
Copy link
Contributor

ejsmith commented Oct 31, 2022

Yeah, this is an issue. We would definitely want to return an empty read only instance and it should be the same instance for all usages to avoid allocating a dictionary.

@frabe1579
Copy link
Author

Maybe a IReadOnlyDictionary<,>, with appropriate extension methods, should be used when exposed for example on IQueueEntry<>, but I don't know if it's used in other code.

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

No branches or pull requests

3 participants