ccs50luhn

Why is this C code returning unexpected values?


I want to multiply every other digit by 2, starting with the number’s second-to-last digit, and then add those products’ digits together, but the first printed values seem completely nonsensical.

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

int len(long li);

int main(void)
{
    // Get credit card number
    long credit_card = 378282246310005;

    // Adding every second digit's double's digits, starting with the second to last
    int sum = 0;
    int digit_doubled;
    for (int i = 0; i < len(credit_card); i++)
    {
        if (i % 2 == 0)
        {
            continue;
        }
        digit_doubled = (int) (credit_card / pow(10, i)) % 10 * 2;
        printf("Digit doubled %i: %i\n", i, digit_doubled);
        for (int j = 0; j < len(digit_doubled); j++)
        {
            sum += (int) (digit_doubled / pow(10, j)) % 10;
        }
    }
}

int len(long li)
{
    if (li == 0)
    {
        return 1;
    }
    return floor(log10(li)) + 1;
}

I have tried modifying the expression to see what results I'd get. When I deleted the % 10 * 2 from the end of digit_doubled = (int) (credit_card / pow(10, i)) % 10 * 2;, I got results that indicate some kind of integer overflow, but I have absolutely no idea where it could be coming from since my program isn't really producing any high values anywhere.


Solution

  • In the first loop when i=1 you cast a value greater than the maximum value that can be stored in an integer (2147483647) which cause it to be truncated and on my system go negative:

    digit_doubled = (int) (credit_card / pow(10, i)) % 10 * 2; // =>
    digit_doubled = (int) 37828224631000 % 10 * 2; / =>
    digit_doubled = -1847312168 % 10 * 2; // =>
    digit_doubled = -16;
    

    I suggest you include stdint.h and use uint64_t instead of silently assume that a long is 64 bit. Alternatively, consider using a string as a credit number is an identifier not a number (even though it is written as one).

    Mixing functions that operate on double for integer type values will open you up to floating point rounding errors. You could use uint64_t versions of these functions instead of double. Below I implemented a len() and my_pow10() functions for you. As the value of digit_doubled is at most 18 you can just inline that calculation instead of using a loop.

    #include <stdio.h>
    #include <stdint.h>
    
    uint64_t my_pow10(uint64_t x) {
        if(x == 0) return 1;
        size_t v = 10;
        for(size_t i = 1; i < x; i++, v *= 10);
        return v;
    }
    
    size_t len(uint64_t v) {
        size_t i = 0;
        for(; v; i++, v /= 10);
        return i;
    }
    
    int main(void) {
        const uint64_t credit_card = 378282246310005;
        const uint8_t credit_card_len = len(credit_card);
        uint8_t sum = 0;
        for (uint8_t i = 1; i < credit_card_len; i += 2) {
            uint8_t digit_doubled = credit_card / my_pow10(i) % 10 * 2;
            printf("Digit doubled %hhu: %hhu\n", i, digit_doubled);
            sum += digit_doubled / 10 + digit_doubled % 10;
        }
        printf("sum = %u\n", sum);
    }