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

Bank card validation module

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

Problem

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
* {@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

Solution

There are three purposes to your code:

  • Converting the human-friendly input string to a canonical digits-only format



  • Verifying the plausibility of the number using the Luhn Algorithm



  • 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) {
        …
    }
}

Code Snippets

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) {
        …
    }
}

Context

StackExchange Code Review Q#54926, answer score: 11

Revisions (0)

No revisions yet.