-1

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);
}
Mennatalla Khougha
  • 1
  • 1
  • 4
    You might find the task easier if you realise that card numbers, phone numbers etc are not integers. They are strings. The user types in a string, you convert it to an integer, and then try to reverse the process to extract the digits which the user gave you in the first place. Work with a string. Its length is obtained from `strlen` not with logarithms. – Weather Vane Feb 28 '23 at 16:07
  • For the even-placed digits, you appear to be adding the digit itself *as well as* the sum of the digits of twice its value. [Wikipedia](https://en.wikipedia.org/wiki/Luhn_algorithm#Example_for_computing_check_digit) shows that, for example `8` you should add the digits of `16` but you already added the `8`. – Weather Vane Feb 28 '23 at 16:27
  • Refer to [Is a credit/debit card number numeric or an integer?](https://stackoverflow.com/q/7269586/2402272). – John Bollinger Feb 28 '23 at 16:31

3 Answers3

0

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.

Weather Vane
  • 33,872
  • 7
  • 36
  • 56
  • I understood and Fixed these two problems but I still don't know why It's falling – Mennatalla Khougha Feb 28 '23 at 17:07
  • You'll have to review the algorithm. One of the digits might be the required check value and not included in the calculation. Also, a side-effect of extracting the digits from an integer, is your 'even' positions are counted from the **right**, whereas with a string index they are counted from the **left**. – Weather Vane Feb 28 '23 at 17:13
  • Thank you very much .but the biggest problem was I start the loop from 0 than used i % 2 = 0 as conditional which meant I started the condition from third to last not second to last. – Mennatalla Khougha Feb 28 '23 at 17:45
  • We say thanks by voting here! – Weather Vane Feb 28 '23 at 17:48
0

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;
}
Mennatalla Khougha
  • 1
  • 1
0

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.

Fe2O3
  • 6,077
  • 2
  • 4
  • 20