HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

String processing in JavaScript

Submitted by: @import:stackexchange-codereview··
0
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):


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 & 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 characters
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
    }
}

Context

StackExchange Code Review Q#27787, answer score: 2

Revisions (0)

No revisions yet.