-
Notifications
You must be signed in to change notification settings - Fork 112
Move FieldValidator Validate() methods to be be on a pointer value #110
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
Comments
@rs: Created a new label type for this issue. Hope that's OK. |
np, what should we use it for? |
Good idea btw. We might want to keep the check to make sure custom types are use properly even if their methodes aren’t on pointers. Does it make sense? |
Depends if we can make the tests right. Sometimes It's not thecnically necessary to use a pointer type, such as map and slice types. When there is no Compile, it's also technically no reason to make the field a pointer.
My thinking was to label issues that are not bugs, but could help with the stability of projects using rest-layer. E.g.: earlier errors, safer and more robust implementations off existing features. |
I want to close this issue in favor of #194. |
We use quite some documentation-real estate to explain that
schema.FeildValidator
s should be set using pointers so that any (potential)schema.Compiler
interface (maybe renamed after #100 ) can be detected and called. THere is also code catching this mis-usage and giving a runtime error.However, it's generally better to fail faster, and it would be pretty easy to make this a compile-time error by moving the Validate method for most FieldValidator implementations to be on a pointer value:
This also means that the run-time checks can be removed!
PS! Some types, such as maps and slices, do not require neither Compiler nor Validate to be on a pointer value due to how these types work.
The text was updated successfully, but these errors were encountered: