# Validating Card Number in Delphi

As part of processing card payments, before even attempting to process, I need to check and validate the card number to make sure it's valid. For this, I've encapsulated this validation in a record called TCardNumber. It does the trick for what it needs to do, tested with a Visa card and an Amex card. Since this is a sensitive process, I need to make sure I'm going about this right. A card number string can be implicitly passed to/from this record.

NOTE: The processing library supports more card types than standard, but I'm only worried about the 4 main ones (American Express, Visa, MasterCard, and Discover). So the TCardType does indeed include things which I don't use. The index of each corresponds with a unique identifier within the processing library I'm using.

NOTE: The function IsValid is the main function I need to be reviewed, but any input on the rest is more than welcome.

Definition:

type
TCardType = (ctUnknown = 0, ctAmex = 1, ctDiscover = 2, ctMastercard = 3, ctVisa = 4,
ctDebit = 5, ctEbt = 6, ctEgc = 7, ctWex = 8, ctVoyager = 9, ctJcb = 10, ctCup = 11);

TCardNumber = record
Num: String;
class operator implicit(Value: TCardNumber): String;
class operator implicit(Value: String): TCardNumber;
function GetStr(const Delim: String = ''): String;
function IsValid: Boolean;
function CardType: TCardType;
end;


And the implementation:

{ TCardNumber }

class operator TCardNumber.implicit(Value: TCardNumber): String;
begin
Result:= Value.Num;
end;

class operator TCardNumber.implicit(Value: String): TCardNumber;
var
S: String;
X: Integer;
C: Char;
begin
S:= Value;
//Strip away any non-numeric characters
for X := Length(S) downto 1 do begin
C:= S[X];
if not (C in ['0'..'9']) then
Delete(S, X, 1);
end;
Result.Num:= S;
end;

function TCardNumber.IsValid: Boolean;
var
S: String;
C: Char;
CheckSum: string;
i,j: Integer;
function ReverseStr(const Str: string): string;
var
i, Len: Integer;
begin
//Reverses string for checksum validation
Len := Length(Str);
SetLength(Result, Len);
for i := 1 to Len do
Result[i] := Str[Succ(Len-i)];
end;
begin
Result:= True;
S:= Num;

//Strip extra characters and validate numeric characters
for i := Length(S) downto 1 do begin //From end to beginning
C:= S[i];
if C in ['-',' '] then begin
Delete(S, i, 1);
end else
if (not CharInSet(C, ['0'..'9'])) then begin
Result:= False;
Break;
end;
end;

//Validate Length
if Result then
Result:= Length(S) in [15,16];

//Check first digit for card type
if Result then begin
C:= S[1];
//3 = American Express
//4 = Visa
//5 = MasterCard
//6 = Discover
Result:= CharInSet(C, ['3'..'6']);
end;

//Validate Checksum
//http://www.delphicode.co.uk/is-credit-card-number-valid/
if Result then begin
S := ReverseStr(S);
CheckSum := '';
for i := 1 to Length(S) do
if Odd(i) then
CheckSum := CheckSum + S[i]
else
CheckSum := CheckSum + IntToStr(StrToInt(S[i]) * 2);
j := 0;
for i := 1 to Length(CheckSum) do
j := j + StrToInt(CheckSum[i]);
Result := (j mod 10) = 0;
end;
end;

function TCardNumber.CardType: TCardType;
var
I: Integer;
begin
Result:= TCardType.ctUnknown;
if IsValid then begin
I:= StrToIntDef(Num[1], 0);
case I of
3: Result:= ctAmex;
4: Result:= ctVisa;
5: Result:= ctMasterCard;
6: Result:= ctDiscover;
end;
end;
end;

function TCardNumber.GetStr(const Delim: String): String;
var
X: Integer;
S: String;
begin
S:= Num;
if (Delim <> '') and (IsValid) then begin
if CardType = ctAmex then begin
//4 - 6 - 5
//xxxx-xxxxxx-xxxxx (15 --> 17)
Insert(Delim, S, 5);
Insert(Delim, S, 12);
end else begin
//4 - 4 - 4 - 4
//xxxx-xxxx-xxxx-xxxx (16 --> 19)
Insert(Delim, S, 5);
Insert(Delim, S, 10);
Insert(Delim, S, 15);
end;
end;
Result:= S;
end;

begin
//Return last 4 digits, prefixed by asterisk characters
Result:= Trim(Num);
Result:= Copy(Result, Length(Result)-3, 4);
Result:= '************'+Result;
end;

• I removed the eCommerce tag, this is actually for a point-of-sale application (although irrelevant). It can be used for eCommerce, but that's not the goal. – Jerry Dodge Sep 7 '15 at 21:19

It's overkill to use string functions for just computing the digit sum.

const Digit: Integer = Ord(S[i]) - Ord('0');
if Odd(i) then
CheckSum := CheckSum + Digit
else
CheckSum := CheckSum + 2 * Digit div 10 + 2 * Digit mod 10;


Also, you can get rid of the ReverseStr function by letting i be the count from the end.

Finally, you should test your code using the official test suite for credit card numbers. I hope there is such a thing; if not, take the test suite from others who have programmed the same. But then, you can also just take their code as well, since it is probably better tested than your newly invented code. Don't reinvent the wheel.

If you remember from your OOP classes at Uni, you have probably heard about the Single Responsibility Principle. Now, your record is responsible for a number of different validations, which is generally a no-no.

My suggestion:

Type

ICard = interface
[GUID HERE]
function GetCardNumber: String;
procedure SetCardNumber( Value: String );

function ValidateNumber : Boolean;
property CardNumber : String read GetCardNumber write CardNumber;
end;

TBaseCardImplementation = class( TInterfacedObject, ICard )
strict private
FNumber : String;
function GetCardNumber: String;
procedure SetCardNumber( Value: String );
strict protected
function ValidateNumber : Boolean;virtual;abstract;
property CardNumber: : String read GetCardNumber write CardNumber;
end;


Now you have an interface and a basic object encapsulating what does not change (the number) and abstracting away what changes. You should not try to have other helper methods until you know for sure that all of your validation code uses similar principles (and I have a vague recollection that they do not, I did this very thing in ASP ages ago).

Now, all you need to do is to create descendants that override the validation. By doing this, you have eliminated the need for the enum and have decoupled the implementation of each single validation. This means you can just use one card type if you so wish (by having each in its own unit, you will need to include 2 units or 3 if your interface is in another unit).

At the same time, with the visibility assigned to the members, it is very difficult (but not impossible!) to misuse the classes.