Please Give feedback On my Javascript project
I have completed the Telephone Number Validator project but, I need the feedback. Is it a fully dynamic function or is it just covering the test cases in the project?
function telephoneCheck(str) {
let reg1 = /^(1\s{0,1}){0,1}\d{3}(-|\s){0,1}\d{3}(-|\s){0,1}\d{4}$/;
let reg2 = /^(1\s{0,1}){0,1}[(]\d{3}[)](-|\s){0,1}\d{3}(-|\s){0,1}\d{4}$/;
if(reg1.test(str)){
return true;
}
else if(reg2.test(str)){
return true;
}
else{
return false;
}
}
telephoneCheck("1(555) 555-5555");
User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.159 Safari/537.36
It looks good to me. I think the tests cover most of the possible scenarios, especially the ones that could potentially get past the algorithm. One tip I have is that you can replace all the {0,1} s in your regexs with a ?, which means the same thing.
Great work on completing the project!
I’d change just some minor things, such as use let. You should only use let when you want a mutable variable, if you aren’t going to change its value using const
Then you keep your if with less code and precise on what you want, and if it doesn’t match the condition you just follow and return false. You must always pay attention to your if's, looking for good legibility of the code.
If is not preceded by a * or +, a question mark just means look for 0 or 1 of the previous character or group.
Lazy matching is finding the smallest part of the string that matches the regex.
var str = '<h1>Hello World</h1>'
var greedyRegex = /<.*>/g
var lazyRegex= /<.*?>/g
console.log(str.match(greedyRegex)) // returns [ '<h1>Hello World</h1>' ]
console.log(str.match(lazyRegex)) // returns [ '<h1>', '</h1>']
In this example, the greedy regex is looking for a ‘<’ and ‘>’ with anything in between. Since regular expressions are by default greedy, it will skip the first > since there is one later on in the string. However, the lazy regex will stop at the first > that it finds.
You’ve got a working solution, which is the most important part. Now you can challenge yourself to make it a little more concise. For example, this can be solved with one regex instead of two. And once you solve it with one regex you can literally solve this challenge with one line of code.
I think another thing that can be cleaned up a little is:
(-|\s)
This is matching either a hyphen or any white space character. You don’t actually want to match any white space character, you only want to match a space. And there is an alternative to using the or operator here which I think is more commonly used, especially if you had more than two characters you wanted to match. Instead of using parens, think square brackets.
Another one:
[(]
The square brackets here are really meaningless. As alluded to above, you use them when you need to indicate a set of characters to match. You only have one character in the square brackets and thus they are unnecessary. But you will have to escape them if you don’t use the square brackets, which is perhaps why you were using them to begin with. I guess it’s a matter of personal preference.