0
\$\begingroup\$

I am using the following code to try and only allow letters, numbers, backspace, space, dashes, slashes, and ampersands. I also want all other keys including the function keys (F1, F2, etc) to be disallowed. I feel like this code can be optimized to make it more streamlined.

This function is part of a live search feature, and I dont want the ajax request to be sent if on of the "prohibited" keys are pressed.

searchBox.keyup(function (e) { // DYNAMIC AUTO SEARCH FUNCTION

        var functionKey = [112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123].indexOf(e.which) > -1;

        if (!String.fromCharCode(e.which).match(/[^A-Za-z0-9 ]\x08/) && !functionKey) {

        // code to execute if key is allowed

        }

});
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

There are two items I can see here which concern me. The first is that, if you have already computed the prohibetedKeyPressed, why not make the boolean check fail-fast? In other words, put the !prohibitedKeyPressed before the regex so that you don't have to do the regex when there's a prohibited key?

I would actually take that further, and turn your code in to guard-clauses:

searchBox.keyup(function (e) { // DYNAMIC AUTO SEARCH FUNCTION

        if ([112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123].indexOf(e.which) > -1) {
            return;
        }

        if (!String.fromCharCode(e.which).match(/[^A-Za-z0-9 ]\x08/)) {
            return;
        }

        // code to execute if key is allowed

});

That makes the return criteria clear.

Now, the regex should probably be anchored.... but worse than that, the expression is a double-negative - if not the char is not a letter/digit.... do something. Now that I look more closely, it does not work at all..... it will never match anything. The \x08 needs to be inside the character class.... hmmm... have you tested this? The search will always happen because the single key-press will never be a single-character and a backspace.... so, the ajax call will happen whenever a non function key is pressed.....

I would make the code:

if (String.fromCharCode(e.which).match(/^[A-Za-z0-9 \x08]$/)) {
    // do something with it - call ajax
}

Now, having done that, why do you need the initial not-a-function-key check at all? Can't the whole thing just be a positive test?

searchBox.keyup(function (e) { // DYNAMIC AUTO SEARCH FUNCTION
    if (String.fromCharCode(e.which).match(/^[A-Za-z0-9 \x08]$/)) {
        // do something with it - call ajax
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.