# Implementation of Luhn Check Algorithm

I am beginner a 1st year cs student. I am taking CS50 and I completed the first week problem of implementing Luhn Check algorithm. Though my solution works I feel it's very sloppy and not efficient. Can someone please suggest how can I improve my solution? (It's a first-week problem so you are not intended to use array or functions.)

Here is the code:

#include<stdlib.h>
#include<cs50.h>
int main()
{
long long x,j=0;
int len,i,len_flag,flag=0,flag_val=0,sp_len=0,check_flag=0; //basic integers related  to loop etc
int res,sum,fin_sum=0,h=0,d2_sum=0,d1_sum=0,ff_num=0; // variables  related to extracting numbers
int sum_len=0,in_sum=0,in_fsum=0,len_sum=0,m=0; // extraing numbers from more than single  sum result
int sum_f=0,sum_final=0;

do{
x = get_long("enter a valid card number \n");
j=x;
len_flag = 0;
len = 0;
while(len_flag!=1)
{
x = x / 10;
if(x==0)
{
len_flag = 1;   //finding the lenght of the number
}
len++;
}
if(len==15||len==16||len==13)
{
flag = 1;
sp_len =1;
}
else
{
flag=1;
}

}while(flag!=1);

for(i=0;i<len;i++)
{
res = j % 10;
j = j / 10;
if(i%2!=0)
{

sum = res * 2;
//printf("multi_res : %d \n",sum);
len_flag = 0;
sum_len = 0;
len_sum = sum;
while(len_flag!=1)
{
len_sum = len_sum / 10;
if(len_sum==0)
{
len_flag=1;       // checking if the sum is more than single  digit
}
sum_len++;
//printf("trig\n");
}
if(sum_len>1)
{   x=0;
while(x<sum_len)
{
in_sum = sum % 10;
sum = sum/10;
in_fsum = in_fsum + in_sum;
//printf("double sum : %d \n",in_fsum);
x++;

}
}

fin_sum = fin_sum + sum;

}
if(i==len-1)
{

for(h=0;h < 1;h++)
{
fin_sum = fin_sum + in_fsum;
}

d1_sum = res;
}

else if(i%2==0)
{
sum_f = sum_f + res; // adding up the number that where not  x2
}

if(i==len-2)
{
d2_sum = res;     //5555555555554444
}

}

sum_final = sum_f + fin_sum;

//printf("sum_final : %d\n",sum_final);
//printf("sum_f_final : %d\n",sum_f);
//printf("sum_in_final : %d\n",fin_sum);

if(sum_final%10==0)
{
flag_val=1; // checking if the number is valid
}
if(ff_num == 0)
{
ff_num = (d1_sum*10) + d2_sum;     //flip
}

do
{
if(sp_len==0)
{
printf("INVALID\n");
check_flag=1;
break;
}
if((flag==1&&flag_val==1&&ff_num==34)||ff_num==37)
{
printf("AMEX\n");
check_flag=1;
break;
}
else if(flag==1&&flag_val==1&&ff_num>50&&ff_num<56)
{
printf("MASTERCARD\n");
check_flag=1;
break;
}
else if(flag==1&&flag_val==1&&d1_sum==4)
{
printf("VISA\n");
check_flag=1;
break;
}
else
{
printf("INVALID\n");
check_flag=1;
break;
}
}while(check_flag!=1);

return 0;

}



I felt like I was fixing a leak while writing the code. I would try to correct one thing and another thing would go wrong and this is the final result.

Thank you for your help. If the question was not asked properly please forgive me; it's my first time here.

• What kind of optimization are you looking for, execution time, memory size or something else? Apr 24 '20 at 17:44
• Both i feel there is a better way of doing it but i cant figure it out Apr 25 '20 at 4:49
• Hmmm, "not intended to use array" --> yet code uses "enter a valid card number \n" which is an array. Apr 25 '20 at 9:03

Code will look much better once OP can use own functions.

how can I improve my solution?

Meaningful names

Why is the credit card number called j. How about cc_num?

Many others names need like-wise improvements.

Use bool

For flags, consider bool.

#include <stdbool.h>

bool flag;


Auto format

Use a development environment that supports auto formatting. Do not format manually - just not efficient.

#include<stdlib.h>
#include<cs50.h>

int main() {
long long x, j = 0;
int len, i, len_flag, flag = 0, flag_val = 0, sp_len = 0, check_flag = 0; //basic integers related  to loop etc
int res, sum, fin_sum = 0, h = 0, d2_sum = 0, d1_sum = 0, ff_num = 0; // variables  related to extracting numbers
int sum_len = 0, in_sum = 0, in_fsum = 0, len_sum = 0, m = 0; // extraing numbers from more than single  sum result
int sum_f = 0, sum_final = 0;

do {
x = get_long("enter a valid card number \n");
j = x;
len_flag = 0;
len = 0;
while (len_flag != 1) {
x = x / 10;
if (x == 0) {
len_flag = 1;   //finding the lenght of the number
}
len++;
}
if (len == 15 || len == 16 || len == 13) {
flag = 1;
sp_len = 1;
} else {
flag = 1;
}

} while (flag != 1);
...


Define variables where needed

Rather than a sea of objects up front, defined them where needed.

Example: Refactor x. Other variables can like-wise get defined closer to their usage.

  // long long x, j = 0;
long long j = 0;
...

do {
// x = get_long("enter a valid card number \n");
long long x = get_long("enter a valid card number \n");
j = x;
...

if (sum_len > 1) {
// x = 0;
int x = 0;


Spelling

//finding the lenght of the number
finding the length of the number


Clean-up

For review purposes, no need for dead debug code.
Eliminate //printf("multi_res : %d \n",sum), etc.

Eliminate unused variables like m.

Comment code

Each chunk of code deserves to be a function, yet I understand that is prohibited for now.

I'd comment the chunks of code to help aide understanding.

  // Read input CC number
// Set j as the CC#, len_flag as the CC# length, flag to 1, ...
do {
long long x = get_long("enter a valid card number \n");


Simplify

flag only used as 1.

  do {
...
flag = 1;
} while (flag != 1);


Replaceable with

  {
...
}


Same for later do { ... } while (check_flag != 1);.

Documentation

// https://en.wikipedia.org/wiki/Luhn_algorithm


Why space before '\n' ?

// "enter a valid card number \n"
"enter a valid card number\n"


Chunks of code seem verbose

len_flag = 0;
len = 0;
while (len_flag != 1) {
x = x / 10;
if (x == 0) {
len_flag = 1;   //finding the lenght of the number
}
len++;
}


versus

// decimal digit length of x
len = 0;
do {
len++;
} while (x /= 10);

• thank you very much will definitely take care of the thing that you pointed out do you have any better way of implementing the algorithm thank you very much Apr 25 '20 at 11:07
• @rayman I'd consider processing the CC# as a string and not a number. Apr 25 '20 at 15:27