5

Iam trying to implement Luhn's algorithm in the C language to check credit card validity, for those who don't know... this is it:

  • Multiply every other digit by 2, starting with the number’s second-to-last digit, and then add those products’ digits together.

  • Add the sum to the sum of the digits that weren’t multiplied by 2.

  • If the total’s last digit is 0 (or, put more formally, if the total
    modulo 10 is congruent to 0), the number is valid!


and to implement that, I looped through the whole number and if the number place I was in had a modulo 2 equal to 0 then I would multiply by two and add to a variable called totalEven.

if that wasn't the case I would add the number I was in to totalOdd without multiplication.

I would then increment the place by one and check the other numbers until I reach 16 (the max digits for a card).

I would later add both variables and check if the total modulo ten was equal to 0. If it means the credit card number is correct, else it is false.

here is the code:

#include <stdio.h>
#include <cs50.h>

//list of variables

   //is the card valid
   bool isValid = true;
   // the creditcard number
   long input;
   //mod stands for modules, and is used to single out each number as seen later
   int mod = 10;
   //the location at which number I am checking
   int place = 1;
   //num is the number I am checking that has been singled out
   int num = 0;
   //total of numbers * 2 located at locations numbered with even numbers
   int totalEven = 0;
   //total of numbers located at locations numbered with odd numbers
   int totalOdd = 0;
     //gets input and stores it in well.. input
     input = get_long("Number: ");
      
      // a formula to single out a number, starting with the ones and then as you can see, mod is muliplied by 10 to go over the second number.

      num = ((input % mod) - (input % (mod /10))) / (mod/10);
      
      //loops 16 times
      for(int i = 0; i < 16; i++)
      {
          // if the place is even execute below
          if(place % 2 == 0)
          {
              totalEven = totalEven + num * 2;
          }   
          //else do this
          else if (place % 2 != 0)
          {
             totalOdd = totalOdd + num; 
          }
          //moves to the next number
          mod = mod * 10;
          place++;
      }
      
      //fufils the last step of the algorithm

      if((totalEven + totalOdd) % 10 == 0 )
      {
          isValid = true;
      }
      else
      {
          isValid = false;
      }

problem is that this block of code gives me invalid or !isValid even though the credit card number is supposed to be correct and I checked my "formula" and it works just fine...

I have absolutely no idea what to do... I am a humble hobbyist so plz don't roast me for the monstrosity above.

here is a complete version of the code

#include <stdio.h>
#include <cs50.h>


long power();


int main(void)
{
    //AMERX 15 STRT 34 OR 37
    //MC 16 STRT 51, 52, 53, 54, 55
    //VZA 13 OR 16 STRT 4

   long input;
   bool isValid = true;
   string type;
   int mod = 10;
   int place = 1;
   int num = 0;
   int totalEven = 0;
   int totalOdd = 0;

   do
   {
      input = get_long("Number: ");
      

   }

   while(input < 0);
   
      
      for(int i = 0; i < 16; i++)
      {
          num = ((input % mod) - (input % (mod /10))) / (mod/10);
          if(place % 2 == 0)
          {
              totalEven = totalEven + num * 2;
          }
          else
          {
             totalOdd = totalOdd + num; 
          }
          
          mod = mod * 10;
          place++;
      }
      
      if((totalEven + totalOdd) % 10 == 0 )
      {
          isValid = true;
      }
      else
      {
          isValid = false;
          
          printf("%i , %i", totalEven, totalOdd);
      }
   if (isValid == true){
   if((input < (38 * power(10, 13)) && input >=(37 * power(10, 13))) || (input < (35 * power(10,13)) && input >= (34 * power(10, 13)))) 
   {
       type = "AMEX\n";
   }
   else if(input >= (51 * power(10, 14)) && input < (56 * power(10, 14)))
   {
       type = "MASTERCARD\n";
   }
   else if((input < (5 * power(10, 12)) && input >= (4 * power(10, 12))) || (input < (5 * power(10, 15)) && input >= (4 * power(10, 15))))
   {
       type = "VISA\n";
   } 
       else{
       type = "error\n";
   }
}
   else
   {
       type = "INVALID\n";
   }
   

    if((totalEven + totalOdd) % 10 == 0 )
    {
      isValid = true;
    }
    else
    {
      isValid = false;
    }
      
    printf("%s", type);

}





long power(int n, int p)
{
    long result = 1;
    for(int i = 0; i<p; i++)
    {
        result = result * n;
    }
    return result;

someGuy5864
  • 63
  • 1
  • 7
  • 3
    Treat credit card numbers as a string of digits instead of as a number. It's easier to process them. And you can check that there are 16 digits more easily. There are many related questions in the [tag:cs50] tag that could help you — not all of them will have the [tag:luhn] tag. Search for '`[cs50] luhn`'. – Jonathan Leffler Aug 07 '20 at 14:06
  • 1
    `mod/10` is just 10/10 which is just 1, so I suspect there's a mistake in ` num = ((input % mod) - (input % (mod /10))) / (mod/10);`. While substantially slower, I second the point that string manipulation would be easier to wrap your head around. – erik258 Aug 07 '20 at 14:07
  • You never get each digit inside the loop. – Some programmer dude Aug 07 '20 at 14:07
  • 4
    Is `long` on your computer large enough for 16 digits? Try `#include ` and `printf("%ld\n", LONG_MAX);` – pmg Aug 07 '20 at 14:07
  • debugging suggestion: if you're set on using modulus and math to do the extraction of digits, get that algorithm working first and write a program that tests it, taking a set of inputs and verifying each output. Then expand from there to get every other digit for each number. Once that's working finish the luhn algorithm with that code you know works. – erik258 Aug 07 '20 at 14:09
  • It's crazy though, since the card number was originally entered as digits. "Number" does not automatically signify "integer", as can be testified by house numbers too, such as `"221b Baker Street"` and phone numbers like `"0123456789"` where the essential leading `0` would be lost. – Weather Vane Aug 07 '20 at 14:29
  • @DanielFarrel l am aware of 10/10 is equal to 1, but that is supposed to happen... for example if `input` was 1069 and `mod` was 10 then `num` would be.... ` num = ((1069 % 10) - (1069 % (10 /10))) / (10/10); num = (9 - (1069 % 1) / 1); num = (9 - 0 / 1); num = 9; ` then as you can see if you look into the code more deeply `mod` gets multiplied by 10 to go to the next digit – someGuy5864 Aug 07 '20 at 14:31
  • @Someprogrammerdude is it possible to clarify? – someGuy5864 Aug 07 '20 at 14:34
  • ah, then the problem is that ` num = ((input % mod) - (input % (mod /10))) / (mod/10); ` is supposed to be in the `for` loop? – erik258 Aug 07 '20 at 14:41
  • @DanielFarrell yes it is supposed to be in there, and loop 16 times moving 16 digits – someGuy5864 Aug 07 '20 at 14:46
  • 1
    Your code is way too complex. Use and array of digits instead of a number where you have to calculate the remainder inside the loop. – Support Ukraine Aug 07 '20 at 14:46

3 Answers3

1

As I was looking at your code, there some mistakes I want to point out.

  1. You forgot: #include <string.h> as you did declare string type in the code.
  2. input = get_long("Number: "); should have its own do-while loop in case user inputs letters or incorrect numbers.
  3. if(place % 2 == 0){ totalEven = totalEven + num * 2; } else if (place % 2 != 0){ totalEven = totalEven + num; } should totalOdd = totalOdd + num for the second part
  4. totalEven = totalEven + num * 2 is right and wrong at the same time. It only works if the number multiplied by 2 is less than 10. If the num * 2 >= 10, lets say num = 6, then 6 * 2 is 12 which would then be 1 + 2 + totalEven.
  5. num = ((input % mod) - (input % (mod /10))) / (mod/10); This should be in the first for loop.
  6. In #include <math.h>, there is a power function called pow which does exactly as your power() function.
Manav Dubey
  • 780
  • 11
  • 26
  • 1
    Asides: the `string` type isn't in `string.h` but is a non-standard (and ill-advised) type in `cs50.h` which was included. And `pow()` is quite different - it works with floating point values. There isn't even any need for it, there are better ways to decompose a number. – Weather Vane Aug 07 '20 at 16:39
  • I see, I wasn't aware of `string` wasn't part `string.h`. Thanks for the info about `pow()`. – Manav Dubey Aug 07 '20 at 16:45
  • @WeatherVane yes, looking back at it now, I could've turned the whole process into a more succinct program... maybe turning the whole int into a string and then using it like an array to validate the inputted number would be much better... – someGuy5864 Nov 16 '20 at 22:31
  • @someGuy5864 it's better never to have a card number, phone number, PIN, etc as an integer in the first place. "Number" does not mean "integer". Best treated as a string of digits. – Weather Vane Nov 16 '20 at 23:45
1

I'm not an expert in Luhn algorithm but when I read https://en.wikipedia.org/wiki/Luhn_algorithm it seems to me that you are doing it wrong.

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

From the rightmost digit (excluding the check digit) and moving left, double the value of every second digit. The check digit is neither doubled nor included in this calculation; the first digit doubled is the digit located immediately left of the check digit. If the result of this doubling operation is greater than 9 (e.g., 8 × 2 = 16), then add the digits of the result (e.g., 16: 1 + 6 = 7, 18: 1 + 8 = 9) or, alternatively, the same final result can be found by subtracting 9 from that result (e.g., 16: 16 − 9 = 7, 18: 18 − 9 = 9).

I don't see anywhere in your code where you handle that bolded part.

Instead of

totalEven = totalEven + num * 2;

I think you need

int tmp = num * 2;
if (tmp > 9) tmp = tmp - 9;
totalEven = totalEven + tmp;

That said - I think you are making the implementation much more complex than needed by storing the input as a number. Instead of a number you could use an array of digits.

That is - instead of

long input = 1122334455667788

use

int digits[] = {8, 8, 7, 7, 6, 6, 5, 5, 4, 4, 3, 3, 2, 2, 1, 1};
// Notice that index zero is the rightmost digit

In this way the algorithm is much more simple:

// Double every second element and check for overflow
for (idx = 1; idx < 16; idx += 2)
{
    digits[idx] = 2 * digits[idx];
    if (digits[idx] > 9) digits[idx] = digits[idx] - 9;
}

// Calculate the sum
sum = 0;
for (idx = 0; idx < 16; ++idx)
{
    sum = sum + digits[idx];
}

If you must receive the input as a number, start by calling a function that converts the number to an array of digits. You can find many, many examples of how that conversion is done here on SO. Here Converting integer into array of digits is just one of many examples.

Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
0

Caution: I have made use of CS50X Library as the question seems to be the one from the same.

#include <stdio.h>
#include <cs50.h>

// Luhn's Algorithm

int main(void) 
{
    long cardNumber = get_long("Please, enter your card number: ");
    int sum1 = 0, num = 0, remainder = 0, sum2 = 0;
    long temp = cardNumber;
    
    while (temp > 0) 
    {
        num = ((temp / 10) % 10) * 2; // Multiplying every other digit by 2, starting with the number’s second-to-last digit
        while (num > 0) 
        {
            remainder = num % 10;
            sum1 += remainder; // Adding those products’ digits together
            num /= 10;
        }
        temp /= 100;
    }
    
    // So as to restore the initial values of remainder and temp for the use in next loop
    remainder = 0;
    temp = cardNumber;
    
    while (temp > 0) 
    {
        remainder = temp % 10;
        sum2 += remainder; // Sum of the digits that weren’t multiplied by 2
        temp /= 100;
    }
    
    ((sum1 + sum2) % 10) == 0 ? printf("Valid\n") : printf("Invalid\n");
    return 0;
}
Aarush Aggarwal
  • 91
  • 1
  • 3
  • 5