Skip to content

Conversation

@96erardo
Copy link
Contributor

@96erardo 96erardo commented Jul 14, 2020

Support for string and number values in isValidNumber function

@96erardo 96erardo requested a review from alacret July 14, 2020 12:25
Copy link
Contributor

@alacret alacret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jesusrodrz @GerardoAGL96 me parece que isValidString debe forzar a recibir un numero, y quizas un rango seria util tambien.

Me parece que el proceso de convertir un dato de un tipo a otro (normalizacion) debe ser manejado aparte. En todo caso, si queremos tener algo asi como utilitario, podemos hacerlo de una manera mas especifica:

  • isStringAValidNumber que reciba un string, y por debajo use isValidNumber o algo asi.

Ref: https://gomakethings.com/converting-strings-to-numbers-with-vanilla-javascript/

@96erardo
Copy link
Contributor Author

@alacret I added the isStringAValidNumber function to validate those type of cases, i still preserve the isValidNumber validator to not introduce breaking changes.

@96erardo 96erardo requested a review from alacret July 16, 2020 21:19
* @returns {boolean} If the string is valid number or not.
*/
export const isValidNumber = (stringToTest: string, allowZero: boolean = false, allowNegative: boolean = false): boolean => {
export const isValidNumber = (value: string, allowZero: boolean = false, allowNegative: boolean = false): boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const isValidNumber = (value: string, allowZero: boolean = false, allowNegative: boolean = false): boolean => {
export const isValidNumber = (value: number, allowZero: boolean = false, allowNegative: boolean = false): boolean => {
if(!typeof number !== "number") return false;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants