Refactor: change validation state to have a static valid state #132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added static Valid property, for `ValidationState.Valid` as mentioned in reactiveui#130 (comment)
Updated document to use `ValidationState.Valid` as introduced in reactiveui#131
|
|
glennawatson
changed the title
2 tasks
worldbeater
added a commit
that referenced
this pull request
Oct 17, 2020
* Use IValidationState where possible * The new pattern plays well with our sample app * The new validation rule overloads * Use the new overload in our sample app * Additional unit tests * Approve the API * Update README.md (#131) Proposed update to indicate use of `IValidationState` or `ValidationState` with `ValidationRule`. * Refactor: change validation state to have a static valid state (#132) * Update ValidationState.cs Added static Valid property, for `ValidationState.Valid` as mentioned in #130 (comment) Updated document to use `ValidationState.Valid` as introduced in #131 * Replaced construction of valid validation states with `ValidationState.Valid`. * Added `ValidationState.Valid` to public API. * Don't break the public ObservableValidationBase API if possible * Use ValidationState.Valid in our sample app Co-authored-by: Craig Dean <thargy@yahoo.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
This is discussed in #130 (comment). Essentially, with the obsolescence of
ComponentfromValidationStatethen the most common 'valid' state is identical and so should be exposed as a static readonly property. This decreases memory utilisation.What is the current behavior?
The same object risks being created repeatedly in common usage.
What is the new behavior?
Consumers can provide the static singleton instance.
What might this PR break?
All impacts relate to the now obsolete
Componentproperty, and could result inNullReferenceExceptionsbeing thrown where consumers previously relied on the component. This is considered highly unlikely, and ultimately the property is due to be removed entirely.ObserveFor<>extension method now has anullComponentproperty, whereas it would previously reference the suppliedValidationContext.ForValidationHelperProperty<>extension methods now have anullComponentproperty, whereas they would previously reference the supplied viewmodel'sValidationContext.Please check if the PR fulfills these requirements
Other information: