Feature/#4 provide default checks
Providing a base implementation for https://github.com/effad/ValidatorFX/issues/4
Thank you for the PR. Please be patient, I'll try to look into it the week after the next.
I had another look at this PR and I'm not sure if I want to integrate it the way it stands. The problems I found:
- Messages must be i18n
- If we just add messageCreator functions, the interface will have even more parameters
- i18n can be done as parameter or by using a "string repository interface" ...
- ValidatorFX until now elegantly sidestepped the problem by requiring the library client to provide message strings within the check-function
- There's always (at least) two variants of create...Check: One with a node, another one without node. Keeping to the fluent API style of ValidatorFX, this should be done with a separate decorates(...) call, espcially since one check can also decorate multiple nodes.
- Passing severity is contrary to what we do with "normal" checks, where severity is decided in the Consumer<Context> passed in withMethod
- the isAssignableTo... checks confused me; they should be named isConvertibleTo... or isMappableTo
- regular checks habe ObservableValue as the "lowest common denominator" of what can be observed. DefaultChecks uses Property and the name of the Property
- I didn't even know properties can have a name
- Most properties we get from JavaFX widgets have either nondescript names or no name at all
So overall I think I'll use this PR as a good base / inspiration of what is wanted but will try to come up with an implementation that is more in line with the (undocumented, implicit ...) design ideas behind ValidatorFX.
I attempted to address your concerns regarding my pull request:
- i18n: I tried to mimic what you did with the way decorations work, but I'm not entirely certain this is an optimal way.
- decorates: I removed the overloaded methods that had the decorates parameter
- Severity: Maybe this could be solved using a builder pattern, but I'm not really confident that this would provide a better developing experience
- isAssignableTo: I renamed those methods to your proposed method name 'isMappableTo'
- "lowes common denominator": I refactored the methods to use ObservableValue instead of properties. This also lead to introducing a new parameter "fieldName"
If you can spare the time, you could reevaluate whether the PR now fits your ideas behind ValidatorFX more closely, otherwise I think using the methods in this PR as inspiration for a future enhancement is fine as well.