Bank card validation module

Question

I hope that someone could review a module I wrote for bank card validation. I included a class that tests some of the methods, which is purely to show the methods work, rather than unit testing.

Validator

import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
 * This module provides static methods for credit card validation based on
 * Luhn's algorithm <br>
 * {@link http://en.wikipedia.org/wiki/Luhn_algorithm}
 * 
 * 
 * @author NRKirby
 *
 */ 
public final class CardValidator {
 private CardValidator() {}
 private static int[] array;
 private static int doubleNumber;
 private static int length;
 private static String number;
 private static int sum;
 private static final String MAESTRO = "(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{8}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{9}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{10}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{11}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{12}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{13}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{14}|(5018|5020|5038|5612|5893|6304|6759|6761|6762|6763|0604|6390)\\d{15}$";
 private static final String MASTERCARD = "^(?!.*(?:(?:5018|5020|5038\\d{12})))[5][0-5].{14}$";
 private static final String SOLO = "(6334|6767)\\d{12}|(6334|6767)\\d{14}|(6334|6767)\\d{15}";
 private static final String SWITCH = "(4903|4905|4911|4936|6333|6759)\\d{12}|(4903|4905|4911|4936|6333|6759)\\d{14}|(4903|4905|4911|4936|6333|6759)\\d{15}|(564182|633110)\\d{10}|(564182|633110)\\d{12}|(564182|633110)\\d{13}$";
 private static final String VISA = "^(?!.*(?:(?:4026|4405|4508|4844|4913|4917)\\d{12}|417500\\d{10}))4\\d{15}$";
 private static final String VISA_ELECTRON = "(4026|4405|4508|4844|4913|4917)\\d{12}|417500\\d{10}$";
 /**
 * Returns the concatenation of {@code number} with a valid check digit.
 * 
 * @param number the number to which a check digit will be added.
 * @return the concatenation of {@code number} with it's valid
 * check digit.
 */
 public static String getCardNumber(String number) {
 return number + Integer.toString(getCheckDigit(number));
 }
 /**
 * Returns the name of the card vendor for {@code cardNumber}.
 * 
 * @param cardNumber 
 * @return the card vendor (if known) <br>
 * {@code else} returns an empty string
 */
 public static String getCardVendor(String cardNumber) {
 if (isVisa(cardNumber)) { return "Visa"; }
 else if (isVisaElectron(cardNumber)) { return "Visa Electron"; }
 else if (isMasterCard(cardNumber)) { return "MasterCard"; }
 else if (isMaestro(cardNumber)) { return "Maestro"; }
 else if (isSolo(cardNumber)) { return "Solo"; }
 else if (isSwitch(cardNumber)) { return "Switch"; }
 else return "";
 }
 /**
 * Returns the valid check digit for {@code number}.
 * 
 * @param number the number for the check digit to be generated.
 * @return the valid check digit for {@code number}. 
 */
 public static int getCheckDigit(String number) {
 getStringLength(number);
 toIntArray(number);
 sum = 0;
 for (int i=length-1;i>=0;i-=2) {
 doubleNumber = array[i]*2;
 if (doubleNumber == 10) { sum += 1; }
 else if (doubleNumber > 10) { sum += 1 + (doubleNumber % 10); }
 else { sum += doubleNumber; }
 }
 for (int i=length-2;i>=1;i-=2) {
 sum += array[i];
 }
 if (sum % 10 != 0) {
 return 10 - (sum % 10);
 }
 else { return 0; }
 }
 private static void getStringLength(String number) {
 length = number.length();
 }
 /**
 * Returns a string with any non-digit characters removed from
 * {@code str}.
 * 
 * @param str the string to have non-digits removed from.
 * @return a string containing only numerical digits.
 */
 public static String removeNonDigits(String str) {
 return str.replaceAll("\\D", "");
 }
 /**
 * Validates {@code cardNumber}.
 * 
 * @param cardNumber the card number to be validated.
 * @return {@code true} if the card number is valid <br>
 * {@code false} if the card number is invalid
 */
 public static boolean validate(String cardNumber) {
 number = removeNonDigits(cardNumber);
 if (number == null || number == "") { 
 return false; 
 } 
 getStringLength(number);
 array = toIntArray(cardNumber);
 sum = 0;
 for (int i=length-2;i>=0;i-=2) {
 doubleNumber = array[i]*2;
 if (doubleNumber == 10) { sum += 1; }
 else if (doubleNumber > 10) { sum += 1 + (doubleNumber % 10); }
 else { sum += doubleNumber; }
 }
 for (int i=length-1;i>=1;i-=2) {
 sum += array[i];
 }
 return sum % 10 == 0;
 }
 private static int[] toIntArray(String number) {
 int length = number.length();
 int[] intArray = new int[length];
 for (int i = 0; i < length; i++) {
 intArray[i] = Integer.parseInt(number.substring(i, i+1));
 }
 return intArray;
 }
 private static boolean isMaestro(String number) {
 Pattern p = Pattern.compile(MAESTRO);
 Matcher m = p.matcher(number);
 return m.matches();
 }
 private static boolean isMasterCard(String number) {
 Pattern p = Pattern.compile(MASTERCARD);
 Matcher m = p.matcher(number);
 return m.matches();
 }
 private static boolean isSolo(String number) {
 Pattern p = Pattern.compile(SOLO);
 Matcher m = p.matcher(number);
 return m.matches();
 }
 private static boolean isSwitch(String number) {
 Pattern p = Pattern.compile(SWITCH);
 Matcher m = p.matcher(number);
 return m.matches();
 }
 private static boolean isVisa(String number) {
 Pattern p = Pattern.compile(VISA);
 Matcher m = p.matcher(number);
 return m.matches();
 }
 private static boolean isVisaElectron(String number) {
 Pattern p = Pattern.compile(VISA_ELECTRON);
 Matcher m = p.matcher(number);
 return m.matches();
 }
}

Test

public class CardValidatorTest {
 public static void main(String[] args) {
 System.out.println(CardValidator.validate("5522139001463839"));
 System.out.println(CardValidator.validate("4751290028474914"));
 System.out.println(CardValidator.validate("6011000990139424"));
 System.out.println(CardValidator.validate("5555555555554444"));
 System.out.println(CardValidator.validate("515105105105100"));
 System.out.println(CardValidator.validate("4111111111111111"));
 System.out.println(CardValidator.validate("4012888888881881"));
 System.out.println(CardValidator.validate("6331101999990016"));
 System.out.println(CardValidator.getCheckDigit("475129002698491"));
 System.out.println(CardValidator.getCheckDigit("552213900896383"));
 System.out.println(CardValidator.getCardNumber("552213969846383"));
 System.out.println(CardValidator.getCardVendor("5018000000000000"));
 System.out.println(CardValidator.getCardVendor("6334000000000000"));
 System.out.println(CardValidator.getCardVendor("5522139698463839"));
 System.out.println(CardValidator.getCardVendor("5018139564463839"));
 }
}

Answer 1

There are three purposes to your code:

1. Converting the human-friendly input string to a canonical digits-only format
2. Verifying the plausibility of the number using the Luhn Algorithm
3. Classifying the type of card according to the length and prefix

Since a class should ideally have a single purpose, consider splitting your class into two, especially for (2) and (3).

Your design is rather rigid and inflexible when it comes to supporting the various card types. Each card type has a pattern (as a string constant), a method (largely copy-and-paste), and a conditional inside getCardVendor() (which includes the name of the card type). That is a code maintenance hassle: adding a new kind of card requires you to touch three places. Not to mention, you have to rebuild this class to make it happen; support for the new card type cannot be added at runtime.

The solution to all those problems is Refactoring, namely the Replace Conditional With Polymorphism transformation. There should be an abstract CardType base class, and subclasses for each vendor. Something like:

public abstract class CardType {
 /**
 * A regular expression for the valid card numbers of this CardType.
 * It is safe to assume that the number has been canonicalized.
 */
 protected abstract Pattern getNumberPattern();
 /**
 * The name of this type of card, e.g. "Visa Electron"
 */
 public abstract String toString();
 /**
 * Checks whether the given number appears to be valid
 * for this CardType, assuming that the number has already
 * passed the Luhn Algorithm verification.
 */
 public boolean isCardNumber(String number) {
 ...
 }
}

• Anirban Nag 'tintinmj': How to determine which card type is the number at runtime by polymorphism?

Answer 2

A few more things, next to the answer by 200_sucess:

• Your class maintains state and as such it should not be all static methods, but work with proper instances. However, you also use state in a rather odd way. There is no need to have the length be a field and set it before working with it. Especially since you are using it inconsistently (see your toIntArray method).
• You are heavily using "string-typing", i.e. everything is a string. Especially card types/vendors should be at least an enum, if not separate types as suggested by 200_success.
• There is no reason to compile the pattern over and over every time. The whole point of being able to separately compile it is so that this overhead doesn't have to be done every time you want to check something against the pattern.
• Don't ever compare strings with ==! Always use equals.
• Checking a string for null or empty is a very old bad-practice situation. At least trim it before checking for emptiness, i.e. foo == null || foo.trim().isEmpty(). Many libraries such as Apache Commons or Guava offer shortcuts for this.
• If you cannot determine the card vendor, is empty string really a good result? Why not throw e.g. a UnknownVendorException? Or at least return null. Returning empty string feels awkward here.

Answer 3

I would like to suggest a few more points to consider:

• How can you handle different types (AmEx, store cards, gift cards, etc). That might be out of scope - but this is so close to accepting it.

• You are currently returning a String. But - how can I use that in my code? Now I have to see if VISA is anywhere in the string...and hope that no one is using a Visa Master Care Card or some other slick marketing.

Building on @200_success, I would consider an enum for MajorCardType (Amex, Visa, MasterCard, Discover, custom, etc), and another property for isValid (number passes the Luhn test).

• Refactor your getCardVendor. If the card doesn't pass the validate - it isn't a valid card - and it really doesn't matter of the vendor. Because you don't have a valid card. And, the way getCardVendor is now, you have to carefully locate the correct placement of the check as to not return too early.

Finally - consider reviewing Wiki and then implementing a few custom (up to 10) parsers based on MII and then subparsers based on IIN. Have those return the CardType.