-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat(ui-utils): add overload signatures for validateNumber
function in validationUtils
#2029
Conversation
ef4d97f
to
78d7171
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on integrating currying into the validateNumber
it makes the usage more flexible and clean. However, I'm curious about the use of function composition here. Could you explain a bit more about why we're mixing it with the validation logic, is that a need you have to use it on another PR?
thanks for clarifying
webapp/src/utils/validationUtils.ts
Outdated
interface NumberValidationOptions { | ||
min?: number; | ||
max?: number; | ||
} | ||
|
||
interface ValidationOptions extends NumberValidationOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidating shared properties like min and max into a single interface eliminates redundancy but keep in mind the context, here their serve different purposes. I think we should keep them separate to simplify future additions and flexibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to commit a fix for the length message sorry.
I change this.
webapp/src/utils/validationUtils.ts
Outdated
|
||
const value = valueOrOpts; | ||
|
||
if (isFinite(value)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're returning an error message when the number is valid (finite). This should be changed to check for the opposite condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Composition here it's just to show the double avantage to have this signature in general. |
2586ed1
to
61fbed4
Compare
61fbed4
to
37e05f1
Compare
37e05f1
to
6afb77c
Compare
Modification to allow
validateNumber
function to support currying and composition:=>
=>