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

JavaScript onKeypress validation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptonkeypressvalidation

Problem

This is a function for JavaScript validation:

var alpha = "[ A-Za-z]";
var numeric = "[0-9]"; 
var alphanumeric = "[ A-Za-z0-9]"; 

function onKeyValidate(e,charVal){
    var keynum;
    var keyChars = /[\x00\x08]/;
    var validChars = new RegExp(charVal);
    if(window.event)
    {
        keynum = e.keyCode;
    }
    else if(e.which)
    {
        keynum = e.which;
    }
    var keychar = String.fromCharCode(keynum);
    if (!validChars.test(keychar) && !keyChars.test(keychar))   {
        return false
    } else{
        return keychar;
    }
}


and HTML code:



I want to know about code quality, creating issues or any alternative for this. I need some suggestion from experts.

Solution

-
onKeyValidate is an okay name, but a better name could be validateKeypress.

-
It seems very silly to store a RegExp as a string, and then construct it every time. Why not just declare var alpha = /[ A-Za-z]/?

-
keyChars appears to check against \x00, the null character, and \x08, the backspace character. Neither of these can ever be passed to onKeypress, so you can just take it out.

-
The standard way to get the character code is event.which || event.keyCode.

-
event is a global; I don't think you need to pass it in.

Here's a proposed rewrite:

var alpha = /[ A-Za-z]/;
var numeric = /[0-9]/; 
var alphanumeric = /[ A-Za-z0-9]/;

function validateKeypress(validChars) {
    var keyChar = String.fromCharCode(event.which || event.keyCode);
    return validChars.test(keyChar) ? keyChar : false;
}


The HTML will have to change to onkeypress="validateKeypress(alpha);".

Code Snippets

var alpha = /[ A-Za-z]/;
var numeric = /[0-9]/; 
var alphanumeric = /[ A-Za-z0-9]/;

function validateKeypress(validChars) {
    var keyChar = String.fromCharCode(event.which || event.keyCode);
    return validChars.test(keyChar) ? keyChar : false;
}

Context

StackExchange Code Review Q#59690, answer score: 6

Revisions (0)

No revisions yet.