10

I've been trying to get this done but no success. I am using regular expression to validate the credit card number entered by user based on the type of credit card selected through the radio button. But I keed having the alert for the all if statement. It looks like all the if else statements are tested. 

There is the HTML code fragment: 

<code>
<p><b>Payment Information:</b></p>
         <fieldset>
            <input type="radio" name="payment" value="Visa" id="visa" />Visa &nbsp;
            <input type="radio" name="payment" value="Master Card" />Master Card &nbsp;
            <input type="radio" name="payment" value="American Express" />American Express &nbsp;
            <input type="radio" name="payment" value="Discover" />Discover <br /><br />
            <label>Card Number:</label>
            <input type="text" name="cardNumber" id="cardNum" size="30" value="" onblur="ValidateCreditCardNumber()" />
</code>



And there is my javascript function :

<code>

function ValidateCreditCardNumber(){

    var ccNum = document.getElementById("cardNum").value;



    var visaRegEx = /^(?:4[0-9]{12}(?:[0-9]{3})?)$/;
    var mastercardRegEx = /^(?:5[1-5][0-9]{14})$/;
    var amexpRegEx = /^(?:3[47][0-9]{13})$/;
    var discovRegEx = /^(?:6(?:011|5[0-9][0-9])[0-9]{12})$/; 


    if (visaRegEx.test(ccNum) === false ){ // Visa validation
       alert("Please provide a valid Visa number!");   
        }  
      else  
        {  
         alert("Thank You!");  
        }  

    if (mastercardRegEx.test(ccNum) === false){ // MasterCard validation
        alert("Please provide a valid MasterCard number!");  
        }  
      else  
        {  
        alert("Thank You!");  
        } 

    if(amexpRegEx.test(ccNum) === false){ // Amex  validation
        alert("Not a valid America Express number!");  
        }  
      else  
        {   
        alert("Thank You!");  
        } 

    if (discovRegEx.test(ccNum) === false){ // Discover validation
        alert("Please provide a valid Discover number!"); 
        }  
      else  
        {  
        alert("Thank You!");  
        } 


    }
</code>


Any kind of help or advice would be really appreciated.
Cyrille
  • 115
  • 1
  • 1
  • 7
  • Aren't there mathematical rules to validate credit card numbers? See https://en.wikipedia.org/wiki/Luhn_algorithm – Phil Nov 23 '16 at 23:15
  • @Phil I believe there are but I have been told to use regular expression instead. – Cyrille Nov 23 '16 at 23:21
  • *"Told"* by whom? Tell them there's better ways – Phil Nov 23 '16 at 23:22
  • A Regular Expression will only validate the *format* of an entered value - a regex cannot compute a CC number's check-digit, for example, which should also be done. However I note that these regexs prevent the user from entering their CC number with spaces or hyphens (which improves readability and usability). I think you should just have a minimum length check and let your backend payment-processor do the actual validation+verification of the numbers. – Dai Nov 23 '16 at 23:29

3 Answers3

23

You have 4 different if blocks in your case, which hits every single block when the previous once fail.

Replace if with if else, that way if will only validate for a single use case. Also you need not let the user know to be specific about the type of card that he is trying to enter. Just a single message to enter a valid number should suffice.

function ValidateCreditCardNumber() {

  var ccNum = document.getElementById("cardNum").value;
  var visaRegEx = /^(?:4[0-9]{12}(?:[0-9]{3})?)$/;
  var mastercardRegEx = /^(?:5[1-5][0-9]{14})$/;
  var amexpRegEx = /^(?:3[47][0-9]{13})$/;
  var discovRegEx = /^(?:6(?:011|5[0-9][0-9])[0-9]{12})$/;
  var isValid = false;

  if (visaRegEx.test(ccNum)) {
    isValid = true;
  } else if(mastercardRegEx.test(ccNum)) {
    isValid = true;
  } else if(amexpRegEx.test(ccNum)) {
    isValid = true;
  } else if(discovRegEx.test(ccNum)) {
    isValid = true;
  }

  if(isValid) {
     alert("Thank You!");
  } else {
     alert("Please provide a valid Visa number!");
  }
}
Sam Logan
  • 3,343
  • 2
  • 17
  • 13
Sushanth --
  • 55,259
  • 9
  • 66
  • 105
  • 1
    Since we're optimizing here why not evaluate isValid like this: var isValid = visaRegEx.test(ccNum)) || mastercardRegEx.test(ccNum)) || amexpRegEx.test(ccNum)) || discovRegEx.test(ccNum)); – Jeppe Andreasen May 09 '18 at 10:09
  • Here's a more exact Discover card regex: https://stackoverflow.com/a/13500918/2718874 – Josh Dec 14 '18 at 00:15
2

Change your logic to this:

<script type="text/javascript">

// Store the regexes as globals so they're cached and not re-parsed on every call:
var visaPattern = /^(?:4[0-9]{12}(?:[0-9]{3})?)$/;
var mastPattern = /^(?:5[1-5][0-9]{14})$/;
var amexPattern = /^(?:3[47][0-9]{13})$/;
var discPattern = /^(?:6(?:011|5[0-9][0-9])[0-9]{12})$/; 

function validateCreditCardNumber() {

    var ccNum  = document.getElementById("cardNum").value;

    var isVisa = visaPattern.test( ccNum ) === true;
    var isMast = mastPattern.test( ccNum ) === true;
    var isAmex = amexPattern.test( ccNum ) === true;
    var isDisc = discPattern.test( ccNum ) === true;

    if( isVisa || isMast || isAmex || isDisc ) {
        // at least one regex matches, so the card number is valid.

        if( isVisa ) {
            // Visa-specific logic goes here
        }
        else if( isMast ) {
             // Mastercard-specific logic goes here
        }
        else if( isAmex ) {
            // AMEX-specific logic goes here
        }
        else if( isDisc ) {
            // Discover-specific logic goes here
        }
    }
    else {
        alert("Please enter a valid card number.");
    }
}

</script>
Dai
  • 141,631
  • 28
  • 261
  • 374
-1

This should do the job

var cc = /^(?:4[0-9]{12}(?:[0-9]{3})?|5[1-5][0-9]{14}|(222[1-9]|22[3-9][0-9]|2[3-6][0-9]{2}|27[01][0-9]|2720)[0-9]{12}|6(?:011|5[0-9][0-9])[0-9]{12}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\d{3})\d{11}|62[0-9]{14})$/

cc.test(card_number)

Source: vee-validate repo

Adam Fowler
  • 1,750
  • 1
  • 17
  • 18