# Validation of credit card number

I have written the code for credit card number validation and it works correctly but I feel that the code could be reduce to fewer lines.

#include <stdio.h>

int main()

{

long long int  no;

do
{
printf("Card number: ");
scanf("%lld", &no);
}

while(no<10);

int d_1,d_2,d_3,d_4,d_5,d_6,d_7,d_8,d_9,d_10,d_11,d_12,d_13,d_14,d_15,d_16;

d_16 = no%10;
d_15 = ((no%100)/10)*2;
d_14 = (no%1000)/100;
d_13 = ((no%10000)/1000)*2;
d_12 = (no%100000)/10000;
d_11 = ((no%1000000)/100000)*2;
d_10 = (no%10000000)/1000000;
d_9 = ((no%100000000)/10000000)*2;
d_8 = (no%1000000000)/100000000;
d_7 = ((no%10000000000)/1000000000)*2;
d_6 = (no%100000000000)/10000000000;
d_5 = ((no%1000000000000)/100000000000)*2;
d_4 = (no%10000000000000)/1000000000000;
d_3 = ((no%100000000000000)/10000000000000)*2;
d_2 = (no%1000000000000000)/100000000000000;
d_1 = ((no%10000000000000000)/1000000000000000)*2;

int od[] = {d_1,d_3,d_5,d_7,d_9,d_11,d_13,d_15};
int ed[] = {d_2,d_4,d_6,d_8,d_10,d_12,d_14,d_16};

for (i=1;i<=8;i++)
{
if(od[i] >= 10)
{
int nod = (od[i]%10)+(od[i]/10);
od[i] = nod;
}
}
for (int j=0;j<8;j++)
{
}
for (i=1;i<=8;i++)
{
if(ed[i] >= 10)
{
int ood = (ed[i]%10)+(ed[i]/10);
ed[i] = ood;
}
}
for (int j=0;j<8;j++)
{
}

{
printf("The card is valid");
}
else
{
printf("The card is invalid");
}

}

• Could you provide the specification of a valid credit card number and some unit tests? – dfhwze Aug 28 at 9:18
• Did you make a typo d3 has 2 different variable names? – dfhwze Aug 28 at 9:39

I don't like this:

do
{
printf("Card number: ");
scanf("%lld", &no);
}

while(no<10);


That blank line makes it look like the while (); is a separate (possibly infinite) loop. I recommend writing the while keyword straight after the closing brace, like this:

do {
printf("Card number: ");
} while (scanf("%lld", &no) != 1);


(I've also changed the test so that we don't access an uninitialised no if the scanf() fails - it's still flawed, though, as any permanent stdin failure will lead to an infinite loop).

It's not clear why we have d_1, d_2, ..., etc. when all we do with them is to copy them into array members - why not just assign to the array members directly?

Instead of dividing no by successively bigger numbers each time, we could modify it as we go, like this:

d_16 = no % 10;
no /= 10;
d_15 = no % 10 * 2;
no /= 10;
d_14 = no % 10;


It's starting to become clear how to transform this into a loop. The missing part is whether to multiply by 2 at each step; we can tell whether the loop index is even or odd, by considering it modulo 2.

Once we are considering the digits in a loop, we can perform the addition as we go, meaning that we no longer need all those local variables.

d_16 = no%10;
d_15 = ((no%100)/10)*2;
d_14 = (no%1000)/100;
d_13 = ((no%10000)/1000)*2;
d_12 = (no%100000)/10000;
d_11 = ((no%1000000)/100000)*2;
d_10 = (no%10000000)/1000000;
d_9 = ((no%100000000)/10000000)*2;
d_8 = (no%1000000000)/100000000;
d_7 = ((no%10000000000)/1000000000)*2;
d_6 = (no%100000000000)/10000000000;
d_5 = ((no%1000000000000)/100000000000)*2;
d_4 = (no%10000000000000)/1000000000000;
d__3 = ((no%100000000000000)/10000000000000)*2;
d_2 = (no%1000000000000000)/100000000000000;
d_1 = ((no%10000000000000000)/1000000000000000)*2;


You're using too many magic numbers in your program, and they are prone to error. Instead, you could use a loop for this calculation.