patternjavascriptMinor
String processing in JavaScript
Viewed 0 times
javascriptprocessingstring
Problem
We have a lot of phone numbers that contain dashes and start with a zero which are from another country.
I am trying to strip the dashes and the one leading zero and select the country code from a drop-down box.
I'm using "raw" JS with DOM here (for the sake of not having to pull in a framework):
Any room for improvement? I know that
That's ok, it's for an intranet app that is usually used with IE 10.
I am trying to strip the dashes and the one leading zero and select the country code from a drop-down box.
I'm using "raw" JS with DOM here (for the sake of not having to pull in a framework):
Dash Remover
+43
+44
+49
Other
Call!
function getCCC() { return document.getElementById('countryCallingCode').value; }
function prepareTelNo() {
var wd = document.getElementById('numbertodial');
wd.value = wd.value.replace(/-/g, ''); // strip - from the tel. no.
// Cut off leading zeros
if(wd.value.charAt(0) === '0' & getCCC() != "")
wd.value = wd.value.substring(1);
}
function callTelNo() {
var telNo = document.getElementById('numbertodial').value;
window.alert('Calling '+getCCC()+telNo);
}
Any room for improvement? I know that
oninput is only supported in some browsers. That's ok, it's for an intranet app that is usually used with IE 10.
Solution
Just a few points
-
You're missing a
-
I'd say you should always use curly braces, even for one-liners. JS accepts one-liners just fine, but as a code-hygiene thing it's better to have braces there, I think.
-
You could rewrite the reg exp to remove everything that isn't a numerical digit (instead of only removing dashes). I don't know if that's what you need, but if it is
-
You also could use a little more reg exp to make sure you remove all leading zeros (even if you say there's only going to be 1). Incidentally, this allows you to skip the "starts with zero" check, and only have the
If you really, truly only want to remove 1 leading zero rather than all of them, use
-
You're missing a
& in your if conditional. Right now you're doing a bitwise AND, rather than a logical AND (which would be &&).-
I'd say you should always use curly braces, even for one-liners. JS accepts one-liners just fine, but as a code-hygiene thing it's better to have braces there, I think.
-
You could rewrite the reg exp to remove everything that isn't a numerical digit (instead of only removing dashes). I don't know if that's what you need, but if it is
wd.value = wd.value.replace(/[^\d]/g, ""); // remove all non-digit characters-
You also could use a little more reg exp to make sure you remove all leading zeros (even if you say there's only going to be 1). Incidentally, this allows you to skip the "starts with zero" check, and only have the
getCCC() != "" check. Point is, you can do a replace for leading zeros specifically, rather than "blindly" chopping something off of the string.function prepareTelNo() {
var wd = document.getElementById('numbertodial');
wd.value = wd.value.replace(/-/g, ''); // strip - from the tel. no.
// Cut off leading zeros if a country code is set
if(getCCC() != "") {
wd.value = wd.value.replace(/^0+/, ""); // cuts off all leading zeros
}
}If you really, truly only want to remove 1 leading zero rather than all of them, use
/^0/ (without the plus sign) as the reg exp pattern.Code Snippets
wd.value = wd.value.replace(/[^\d]/g, ""); // remove all non-digit charactersfunction prepareTelNo() {
var wd = document.getElementById('numbertodial');
wd.value = wd.value.replace(/-/g, ''); // strip - from the tel. no.
// Cut off leading zeros if a country code is set
if(getCCC() != "") {
wd.value = wd.value.replace(/^0+/, ""); // cuts off all leading zeros
}
}Context
StackExchange Code Review Q#27787, answer score: 2
Revisions (0)
No revisions yet.