Improve JavaScript Function

Question

I have the next function:

var checkNameLenght = function(name,nameLenght,allowedLenght,defaultName) {
    if((nameLenght <= allowedLenght) && !(/\s[^a-z]/i.test(name))) {
        return name; 
    }
        else {
            if(opts.debug == true) {
                console.log(name+' is to long or contains special characters / numbers | Please choose a name shorter than '+allowedLenght+' characters or remove any character / number');
            }
            return defaultName;
        }
}

What it does, is checking if a certain string is more or less than an allowed length and it also checks if the string contains any special characters or numbers ( except white space ) and if any of the above is confirmed it returns a default string, if not the string.

But this function checks at the same time for the length and regex, if((nameLenght <= allowedLenght) && !(/\s[^a-z]/i.test(name))) { ... }, but I don't want that, I would like it to check first for length and then for regex.

I tried like this:

var checkNameLenght = function(name,nameLenght,allowedLenght,defaultName) {
    if(nameLenght <= allowedLenght) {
        return name; 
    }
        else if( !(/\s[^a-z]/i.test(name)) ){
            return name;

        }
            else {
            if(opts.debug == true) {
                console.log(name+' is to long or contains special characters / numbers | Please choose a name shorter than '+allowedLenght+' characters or remove any character / number');
            }   
            return defaultName;
            }
}

But it doesn't work as the first version. What am I doing wrong with the second function ?

Answer 1

Your first method is correct. It won't check simultaneously

in Javascript A && B works as given below

1. first it will check condition A
2. if A is false it will go to else condition
3. if A is true it will check condition B
4. if B is false it will go to else condition
5. if B is also true it will execute the statements inside if

So your first method is correct. Like @nnnnnn said problem is with the regular expression and condition. As both of the conditions has to be true, it should be

if(name.length <= allowedLength && /^[a-z\s]*$/i.test(name)) {
    return name;
}else{
    ....
}

if you give

if(nameLenght <= allowedLenght) {
    return name; 
}else if( !(/\s[^a-z]/i.test(name)) ){
    return name;
}
else {
    ....
}

when either of the first two conditions becomes true, it returns name, which is not your requirement.

.

Answer 2

Using some ideas from nnnnnn, I think you mean something like this

    var checkName = function(name, allowedLength, defaultName) {
        if (name.length > allowedLength) {
            if (opts.debug == true) {
                console.log(name + ' is to long | Please choose a name up to ' + allowedLength + ' characters');
            }   
            return defaultName;
        }
        else if (/[^a-z\s]/i.test(name)) {
            if (opts.debug == true) {
                console.log(name + ' contains special characters or numbers | Please remove any character or number');
            }   
            return defaultName;
        }
        else {
            return name;
        }
    }

Answer 3

But it doesn't work as the first version

Well no, you've changed the logic. You didn't say what exactly is wrong with how the second version is working (or if it is giving you an error?) so it's hard to say exactly what's wrong. But I'll take a guess.

In the second version you first check length, and if that test passes you immediately return name without doing any further tests. If you need to test the regex too then either use the && syntax from the first version, or nest the second test inside the first if block.

it also checks if the string contains any special characters or numbers ( except white space ) and if any of the above is confirmed it returns a default string, if not the string.

Actually that's not what the regex you have is doing. This is yours:

!(/\s[^a-z]/i.test(name))

That regex is testing whether name contains a white space character followed immediately by any non alphabet character, and then the result of that has the not ! applied to it. So if your string contains non alphabetic characters that don't immediately follow white space they will not be found. If your goal is to test for any character that is not either alphabet A-Z or white space, anywhere in the string then try this:

!(/[^a-z\s]/i.test(name))

Or instead of looking for invalid characters just test that the whole string is made up of valid characters:

/^[a-z\s]*$/i.test(name)          // note: you don't need the ! for this

//  ^        - match beginning of string
//  [a-z\s]* - match zero or more alphabet or white space characters
//  $        - match end of string

Also, you don't need the nameLenght parameter, because strings have a length property built in: you can just same name.length. (This doesn't stop the function working, obviously, but it is kind of untidy.)

(Also you've misspelled "length", but you seem to have done so consistently in every variable name so it isn't causing a problem. Yet.)

EDIT: new version to work as per your comment:

var checkName = function(name, allowedLength, defaultName) {
        if(name.length > allowedLength) {
           console.log("Name is more than allowed length");
           return defaultName;
        }
        if( /[^a-z\s]/i.test(name) ){
           console.log("Name contains special characters or numbers");
           return defaultName;
        }

        // tests passed
        return name;
};

Note: you don't need any else statements because each if returns out of the function. Obviously you can remove the console.log() statements I put in, or you can add if(opt.debug) statements as in your original.

Answer 4

First, you don't really need the else there, if your first condition is true, the function will return name and stop, so a first amelioration would be:

var checkNameLenght = function(name,nameLenght,allowedLenght,defaultName) {
    if((nameLenght <= allowedLenght) && !(/\s[^a-z]/i.test(name))) {
        return name; 
    }

    if(opts.debug == true) {
       console.log(name+' is to long or contains special characters / numbers | Please choose a name shorter than '+allowedLenght+' characters or remove any character / number');
    }
    return defaultName;    
}

Now, You want to check the length first, you need to do something like this:

var checkNameLenght = function(name,nameLenght,allowedLenght,defaultName) {
    if(nameLenght <= allowedLenght) {
        if( !(/\s[^a-z]/i.test(name)) ) {
          return name;
        } 
    }

    if(opts.debug == true) {
       console.log(name+' is to long or contains special characters / numbers | Please choose a name shorter than '+allowedLenght+' characters or remove any character / number');
    }
    return defaultName;    
}

It is not clear whether you want the else to be executed only when the nameLength is bigger or when the regex matches too, so you may want to edit my code accordingly.