CS50 Luhn's Algorithm Problem Mistaks Invalids card with Visa

Question

The programm identifies this invalid cards as Visa (4111111111111113, 4222222222223). I really don't know how to solve this problem. The program pass all other checks expet these two and I don't know where it goes wrong. I think the problem in the loop but the program identifies "AMEX", "MASTERCARD", "VISA" and "INVALID" correctly excpet these two cards.

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

int main(void)
{
    string cardType = "VALID";
    long card = get_long("Number: ");
    int length = floor(log10(card)) + 1;
    bool isValid = false;
    long number = card;
    long startNumber = card;
    int productNumber;
    int sum = 0;

    while (startNumber > 100)
    {
        startNumber /= 10;
    }

    for (int i = 0; number < 0; i ++)
    {
        sum += number % 10;
        if ( i % 2 == 0)
        {
            productNumber = (number % 10) * 2 ;
            if (productNumber > 9)
            {
                productNumber = (productNumber % 10) + (productNumber / 10);
            }
            sum += productNumber;
        }
         number /= 10;
    }

    if (sum % 10 == 0)
    {
        isValid = true;
    }

    if (length == 15 && isValid && (startNumber == 34 || startNumber == 37))
    {
        cardType = "AMEX";
    }
    else if (length == 16 && isValid  && (startNumber == 51 || startNumber == 52 || startNumber == 53 || startNumber == 54 || startNumber == 55))
    {
        cardType = "MASTERCARD";
    }
    else if ((length == 13 || length == 16) && isValid && (startNumber >= 40 && startNumber < 50 ))
    {
        cardType = "VISA";
    }
    else {
        cardType = "INVALID";
    }

    printf("%s\n", cardType);
}

Answer 1

Your checksum loop has at least two faults.
a) It isn't looping at all so the checksum test always passes. Change

for (int i = 0; number < 0; i ++)

to

for (int i = 0; i < length; i ++)

b) The Luhn algorithm is incorrect because for even-placed digits you sum the digit as well as the sum of the digits of twice its value. The whole loop should be this:

for (int i = 0; i < length; i ++)
{
    if ( i % 2 == 0)
    {
        productNumber = (number % 10) * 2 ;
        if (productNumber > 9)
        {
            productNumber = (productNumber % 10) + (productNumber / 10);
        }
        sum += productNumber;
    }
    else
        sum += number % 10;
    number /= 10;
}

c) However the checksum test is still failing for reasons I haven't yet found.

Answer 2

I needed to start my loop from 1 not 0 that's why it wasn't passing the check thank you.

    for (int i = 1; i <= length ; i ++)
{
    if ( i % 2 == 0)
    {
        productNumber = (number % 10) * 2 ;

        if (productNumber > 9)
        {
            productNumber = (productNumber % 10) + (productNumber / 10);
        }

        sum += productNumber;
    }
    else
    {
        sum += number % 10;
    }
     number /= 10;
}

Answer 3

Calculating the checksum can be done with less code.

int len = 1, sum = number%10; // Start with value of "check digit"
for( long long cpy = number/10; cpy; cpy /= 10, len++ )
    if( len%2 )
        sum += "0246813579"[cpy % 10] - '0';
    else
        sum += cpy % 10;
bool valid = sum % 10 == 0;

The string represents the values arrived at for even positions of the digit when doubled. The appropriate character is turned into a value by subtracting '0'.

Bonus: a happy consequence: len becomes the length of the credit card "number".

Note: I agree with others that this CS50 problem should be dealing with a string of digits, not an integer value.

---

EDIT
For an exercise, here's code that extracts the important information from an integer credit card "number".

    int len = 1, sum = cn%10; // Start with value of "check digit"
    int prfx0 = 0, prfx1 = 0;
    for( cn /= 10; cn; cn /= 10, len++ )
        if( len%2 )
            prfx1 = cn, sum += "0246813579"[cn % 10] - '0';
        else
            prfx0 = cn, sum += cn % 10;

    sum %= 10; // should equal 0
    if( prfx1 >= 10 ) prfx0 = prfx1; // pick the 2 digit value
    printf( "len %d  sum %d  prfx %d\n", len, sum, prfx0 );

The machine works a bit harder, but this determines the checksum value, the length of the "number" and the (hoped for) 2 digit prefix. All that's left is to test which, if any, type of credit card this is.