Credit : the checksum is correct for <10 digits, but for14,15,16 digits getting negative checksum

Question

The checksum is correct for less than 10 digits but for 14-16 digits it is coming back negative. What am I doing wrong?

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

int digit(long number, int place)
{
    int k = number / pow(10, place - 1) ;
    int final = k % 10;
    return final;
}


int numberofdigits(long nmber)
{
    long m = log10(nmber);
    long u = floor(m);
    int e = u + 1;
    return e;
}

int checksum(long ccn)
{
    int k = 0;
    for (int i = 2; i<numberofdigits(ccn); i = i + 2)
    {
        if (digit(ccn, i) < 5)
        {
            k = k + digit(ccn, i) * 2;
        }
        else
        {
            k = k + (digit(ccn, i) * 2) % 10 + digit(digit(ccn, i) * 2, 2);
        }
    }
    for (int i = 1; i < numberofdigits(ccn); i = i + 2)
    {
        k = k + digit(ccn, i);
    }
    return k;
}


int main(void)
{
    long input = get_long("Number: ");
    if ((numberofdigits(input) == 16 || numberofdigits(input) == 13) && (digit(input, numberofdigits(input)) == 4) && (checksum(input)%10 == 0))
    {
        printf("VISA\n");
    }
    else if (numberofdigits(input) == 15 && digit(input, numberofdigits(input)) == 3 && (digit(input, (numberofdigits(input)-1)) == 4 || digit(input, (numberofdigits(input)-1)) == 7) && checksum(input) % 10 == 0)
    {
        printf("AMEX\n");
    }
    else if (numberofdigits(input) == 16 && digit(input, numberofdigits(input)) == 5 && (digit(input, numberofdigits(input)-1) == 1 || digit(input , numberofdigits(input) - 1) == 2 || digit(input , numberofdigits(input)-1) == 3 ||digit(input, numberofdigits(input)-1) == 4 || digit(input, numberofdigits(input) - 1) == 5 ) && checksum(input) % 10 == 0)
    {
        printf("MASTERCARD\n");
    }
    else
    {
        printf("INVALID\n");
    }
}

Answer 1

this is in need of major cleanup. organize your code and avoid repeating yourself. For example figure out how many digits are in the number once. save that value. Figure out the value of the i-th digit once in your hash function loop and save that value instead of calling digit over and over.

Then you can either figure out whether the hash is valid or figure out which type of card it could be. Personally I would do the second first because if its not a valid card type, why bother trying to compute the hash?

Finally your hashing algorithm is wrong, you need to carefully reread the explanation of how to compute Luhn's algorithm, and if you have a specific question ask it. Have you tried testing it? What are you getting as a result? walk through with the debugger and verify it's doing what you think it should. If the debugger is not available yet insert print statements with useful information along the way so you can see what the values are and where you are going wrong. Something is very wrong if you are able to get a negative number. I see one problem is that you are taking in a long for your hash function but you are only returning an int, suggest you be consistent and return a long but that isn't going to solve your bigger problem.

suggest you also examine if your checksum always returned true if the rest of the code would work and return the correct credit card ( obviously ignore the false positives that dont say invalid )

computer science is science so follow the scientific method: minimize your variables and break the problem into subproblems and solve and verify that the subproblems are solved correctly to solve the whole problem. control a variable to be sure that the changes you make are accomplishing the changes you expect.