feature: Make ValidationText immutable, and add Empty property.
#141
Labels
Comments
thargy
added a commit
to thargy/ReactiveUI.Validation
that referenced
this issue
Oct 19, 2020
…ion to immutability.
thargy
added a commit
to thargy/ReactiveUI.Validation
that referenced
this issue
Oct 19, 2020
… to provide a `ValidationText.Empty` property).
thargy
added a commit
to thargy/ReactiveUI.Validation
that referenced
this issue
Oct 19, 2020
…ate, so that `Empty` contains single empty string. Updated use of obsolete constructors.
2 tasks
glennawatson
added a commit
that referenced
this issue
Oct 20, 2020
…rformance and thread-safety of ValidationText (#144) * Suggested initial implementation of #141, to prepare migration to immutability. * Fixes #142, (note depends on implementation of #141 to provide a `ValidationText.Empty` property). * #141 Enhanced backwards compatibility of ValidationText update, so that `Empty` contains single empty string. Updated use of obsolete constructors. * Updated API verification. Passed tests. * Added ExcludeFromCodeCoverage attributes. * Updated API definitions to include code coverage attributes. Restored UAP target. * ValidationText.None added, and used to indicate a ValidationText with no items - this is used when initialising a ValidationContext. ValidationText.Empty always contains a single item which is string.Empty. Previous usages of the constructors have been changed to make use of the Create methods, and the Create methods correctly re-use the singletons where possible. * Added safety checks to prevent modification of the new Empty/None properties. * Reverted removal of UAP target. * Update src/ReactiveUI.Validation.Tests/ValidationTextTests.cs Co-authored-by: Artyom V. Gorchakov <[email protected]> * Update src/ReactiveUI.Validation/Collections/ValidationText.cs Co-authored-by: Artyom V. Gorchakov <[email protected]> * Update src/ReactiveUI.Validation/Collections/ValidationText.cs Co-authored-by: Glenn <[email protected]> Co-authored-by: Artyom V. Gorchakov <[email protected]>
|
Should be resolved in #144 |
|
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Describe the solution you'd like
Currently
ValidationTextis mutable, allowing additionaltextsto be added, or for the collection to be cleared. This functionality is not currently used in the library and has performance and thread-safety implications.Describe alternatives you've considered
Having an alternative immutable implementation that can be used interchangeably with the existing mutable version, does not remove the thread-safety issues of accessing the collection whilst it may be being mutated.
Describe suggestions on how to achieve the feature
The
Add()andClear()methods should be removed, and the underlying list changed to a nullable array of _texts (nullfor empty). Although this is a breaking change, it is worth it as it allows a static readonlyValidationText.Emptysingleton property to be added and reduces memory utilisation.Ideally, the current constructors should be deprecated for a factory method that returns
Emptywhen supplied with an empty enumerable.Additional context
The text was updated successfully, but these errors were encountered: