1
\$\begingroup\$

I wrote a script for validate the input field for following requirements

  1. Not null
  2. Multiple of hundreds
  3. Less than needed amount

Here is my script:

    $('#investAmount,#invest-button').bind('keypress', function(event){         
        if(event.which == 13){
            if(amountValidation('#investAmount','#inNeedAmt','#invalidAmt','#mult100','#needAmtValidation')){
                return false;
            }
            else{$(elementId).removeClass('errorField');}
        }
    });
    $('#invest-button').click( function(event){
        if(amountValidation('#investAmount','#inNeedAmt','#invalidAmt','#mult100','#needAmtValidation')){
            return false;
        }
        else{$(elementId).removeClass('errorField');}
    });



function amountValidation(elementId,needAmt,errorMsgId,errorMsgId2,errorMsgId3){    
    var gvAmount =$(elementId).val();

    var errAmount = gvAmount == '';
    if(errAmount == true){$(elementId).addClass('errorField');}
    $(errorMsgId).toggle(errAmount);

    var errGVAmount = !errAmount && !gvAmount.match(/^[1-9]\d*00$/);
    $(errorMsgId2).toggle(errGVAmount); 
    if(errGVAmount == true){$(elementId).addClass('errorField');}

    var errNeedAmount =parseInt($(needAmt).val()) <  parseInt(gvAmount);
    $(errorMsgId3).toggle(errNeedAmount);
    if(errNeedAmount == true){$(elementId).addClass('errorField');}

    return errAmount || errGVAmount || errNeedAmount;   
}

Actually I am going to use this function in my website multiple places please help me to make this better script. My jsfiddle is here

\$\endgroup\$
3
\$\begingroup\$

I'm not much of a jQuery freak, but I still have some points on this code. In your amountValidation function, you have some duplication and overhead:

if(errAmount == true)

can be written:

if(errAmount)

Also you always do the same thing when an error occurs, but that's in each statement:

if(errAmount == true){/* mark the field */};  
if(errGVAmount == true){ /* mark the exact same field */};
if(errNeedAmount == true { /* and yet the same thing again */ };

you can shorten this to:

if(errAmount || errGVAmount || errNeedAmount){ /*mark field*/ };

Your variable names could use some change too:

errAmount on the first look is like the geeneral error overall, not that amount is null / empty.
You could use errAmountNull instead.

errNeedAmount sounds like you have no amount given yet, which would be errAmount.
I would use errAmountTooSmall instead

Nobody knows, what the GV in errGVAmount stands for.
From the requirements I would call it: errAmountUnallowedValue

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.