phoneNumberCheck() is using 2 helper function to check if a users input of a phone number is valid.
this is a client side check, if this code is ok should I check the same at the nodeJS back end?
is splitting the number validation function to validate length and another function to validate prefix consider as over engineering?
Critical, Optional and Positive feedback are more then welcome.
Thanks,
// phone number input sanitizing
const sanitizeNumber = (phoneNumber) => {
return phoneNumber
.split('')
.map((e) => e.replace(/\D/g, ''))
.join('')
}
// phone number validation: length and prefix
const validateNumber = (sanitizedInput) => {
return sanitizedInput.length != 10 ? 'bad number length' :
sanitizedInput.startsWith('05') ? sanitizedInput : 'bad number prefix'
}
/* high level function to check phone number validity,
returns valid number or error. */
const phoneNumberCheck = (userInput) => {
// exit early if length is not correct (long strings)
if (userInput.length === 10) {
// sanitize
const sanitizedNumber = sanitizeNumber(userInput)
// validate
return validateNumber(sanitizedNumber)
}
// implict else to return false
return false
}
/* my test cases */
// returns false early
console.log(phoneNumberCheck(`.05'@0"12-34'5"67`))
// returns length error
console.log(phoneNumberCheck(`0501234-67`))
// returns prefix error
console.log(phoneNumberCheck(`0601234567`))
```
-
\$\begingroup\$ This doesn't seem to make sense to me. What exactly are valid inputs? \$\endgroup\$RoToRa– RoToRa2021年06月19日 16:23:24 +00:00Commented Jun 19, 2021 at 16:23
-
\$\begingroup\$ valid user input is specific, a string of 10 digits e.g: 05xxxxxxxx when the x's are digits \$\endgroup\$hibih– hibih2021年06月19日 17:58:41 +00:00Commented Jun 19, 2021 at 17:58
1 Answer 1
this is a client side check, if this code is ok should I check the same at the nodeJS back end?
Yes! The server has no idea what's really running on the other side of the connection - all it knows is what data it gets. Maybe it's talking to an old version of the client that had a bug in its validation code. Maybe it's talking to a third-party client that thinks you're using another format, so it sends something it thinks is acceptable but really isn't. Maybe it's talking to a fake client that wants to put bad data in your database for some reason. Network input is input and must be treated like any other input - make no assumptions you don't check.
is splitting the number validation function to validate length and another function to validate prefix consider as over engineering?
Two simple functions is usually better than one complicated function - but one simple function is often better than two simple functions. So, in general no, that could be a valid way to organize this if you had more complicated rules for validation. But in this case, I'd say yes.
On that note, I have opinions on your sanitation function in general:
- First off, the
join
ing andsplit
ting is not needed here. You can accomplish the same thing withphoneNumber.replaceAll(/\D/g, '')
- But more importantly - what useful thing does this function do? When I call
sanitizeNumber
I don't end up with a sanitized phone number - I end up with a sanitized something and have to check whether it's a phone number. That doesn't seem very useful. Why do I have to check that myself? I already have a function that claims to turn a string into a phone number, why can it fail to do that and not tell me? Shouldn't it throw an exception or returnnull
or something when given input it can't turn into a valid phone number? - Finally, the way your code is written, any time the sanitation actually ends up happening it either changes an invalid number into a different invalid number (10 characters including x non-digits -> 10-x digits, which is too short to be valid), or it leaves the input unchanged (not 10 characters -> no sanitation; 10 digits -> unchanged output). At that point, what are you even sanitizing? Either the input was already a valid phone number or it went from being an invalid phone number to still being an invalid phone number - so why not just check whether the input was a valid phone number in the first place? In this case, that'd be as simple as
userInput.match(/^05\d{8}$/)
Though I do want to point out that depending on your target audience, you might be making assumptions about phone numbers that you shouldn't be. Are you sure your user's phone number will be from the region you're in? The prefix check might be overzealous if not. Are you sure your users will all have phone numbers from your country (even if they live there)? If not, you shouldn't be assuming 10 digits. Phone numbers are complicated, and you may be better off finding a library that already handles that complexity than trying to roll your own implementation.
-
\$\begingroup\$ Thank you Sara, this code is for specific people in a specific country and it comes with instructions this is why I didn't went with a library that handle phone numbers in general. as you might guess this isn't production code yet :D, I'll implement the validation at server side - one of the advantages of using JS as frontend and backend is the code practicly the same :D. about the functions split UncleBob says extract (method) and extract and extract until you can't do that anymore. In this case I think one simple function will do. \$\endgroup\$hibih– hibih2021年06月19日 19:02:36 +00:00Commented Jun 19, 2021 at 19:02
-
\$\begingroup\$ sanitizeNumber purpuse was to strip all sql injection chars: `'"| etc'. it's a great backend one liner to: return userInput.match(/^05\d{8}$/) == null ? false : userInput. but doing that on the frontend I don't have the cause I can output to the user what the error is(missing digits? / check prefix...). \$\endgroup\$hibih– hibih2021年06月19日 19:02:47 +00:00Commented Jun 19, 2021 at 19:02
-
\$\begingroup\$ @hibih: If you're going to strip out the non-digits, you should at least do that before you check the length, not after. Otherwise, the user can add garbage characters to a too-short number to get a "valid" number that isn't actually valid, but passes your check. \$\endgroup\$Kevin– Kevin2021年06月19日 23:52:50 +00:00Commented Jun 19, 2021 at 23:52
-
\$\begingroup\$ Final points are important. Where I work (finance) we had to actually define, as a business, what a domestic phone number was due to internationally formatted mobile numbers validly connecting to domestic networks and vice-versa. End result was any number (with or without leading-zeroes where the length was above a minimum, ie 7) was valid. If the person was connected to a domestic network, then not a problem, otherwise they needed to organise an alternative. \$\endgroup\$Aaron– Aaron2021年06月20日 02:12:59 +00:00Commented Jun 20, 2021 at 2:12
-
\$\begingroup\$ @Kevin all the checks runs together on button click. \$\endgroup\$hibih– hibih2021年06月20日 10:41:52 +00:00Commented Jun 20, 2021 at 10:41