C语言信用卡校验代码优化求助：循环实现遇异常

Hey there! Great job getting the long version of your credit card validator working—looping to simplify is such a smart move, so let's break down why your rewritten code isn't behaving, fix it, and even share some extra tips to make it cleaner.

What's Wrong with Your Loops? #

Let's start with the digit extraction loop that's causing the repeated 1s:

for (int d = 0; d < 16; d++) {
    for (long i = 1000000000000000; i > 9; i /= 10) {
        digit[d] = ((creditCard / i) % 10);
    }
}

The problem here is the inner loop: for every index d, you're running through every divisor from 1e15 down to 10, and overwriting digit[d] every single time. By the end of the inner loop, digit[d] will always hold the second-last digit of your card number (since i ends at 10), no matter what d is. That's why you're seeing the same value repeated!

Your oddDigits loop has the exact same issue:

for (int n = 0; n < 8; n++) {
    for (int t = 0; t < 15; t += 2) {
        oddDigits[n] = digit[t]*2;
    }
}

The inner loop overwrites oddDigits[n] 8 times per n, so every element in oddDigits ends up being digit[14] * 2 (the last even-indexed digit in your array).

Fixed Loop-Based Code #

Here's how to rewrite those loops correctly, with no redundant inner loops:

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

int main(void) {
    // User input for credit card number
    long creditCard = get_long("Credit Card: ");
    
    // Initialize arrays (set to 0 to handle shorter card numbers)
    int digit[16] = {0};
    int oddDigits[8] = {0};
    
    // Extract digits with a single loop
    long divisor = 1000000000000000; // Starts at 10^15 for the first digit
    for (int d = 0; d < 16; d++) {
        digit[d] = (creditCard / divisor) % 10;
        divisor /= 10; // Move to the next lower digit each iteration
    }
    
    // Populate oddDigits (every other digit starting from index 0)
    for (int n = 0; n < 8; n++) {
        oddDigits[n] = digit[2 * n] * 2; // Map n to indices 0,2,4,...14
    }
    
    // Rest of your validation logic (same as your working code, with a small improvement)
    for (int o = 0; o < 8; o++) {
        if (oddDigits[o] > 9) {
            oddDigits[o] -= 9;
        }
    }
    
    int sum = 0;
    for (int s = 0; s < 8; s++) {
        sum += oddDigits[s];
    }
    
    for (int s2 = 1; s2 < 16; s2 += 2) {
        sum += digit[s2];
    }
    
    int check = sum % 10;
    if (check != 0)
        printf("INVALID\n");
    else {
        // Calculate actual card length to avoid checking leading zeros manually
        int cardLength = 0;
        for (int i = 0; i < 16; i++) {
            if (digit[i] != 0) {
                cardLength = 16 - i;
                break;
            }
        }
        
        // Cleaned up card type checks
        if ((cardLength == 15) && (digit[1] == 3) && (digit[2] == 4 || digit[2] == 7)) {
            printf("AMEX\n");
        } else if ((cardLength == 16) && (digit[0] == 5) && (digit[1] >= 1 && digit[1] <= 5)) {
            printf("MASTERCARD\n");
        } else if ((cardLength == 13 || cardLength == 16) && (digit[cardLength ==13 ? 3 : 0] == 4)) {
            printf("VISA\n");
        } else {
            printf("INVALID\n");
        }
    }
}

Key Improvements #

1. Single Loop for Digit Extraction: We use a single divisor variable that shrinks by 10 each iteration, so each digit[d] gets the correct value without overwriting.
2. Simplified oddDigits Population: Directly map n to even indices (2*n) instead of looping through all even indices every time.
3. Automatic Card Length Detection: No more checking for leading zeros—we calculate how long the actual card number is, making the card type logic cleaner.
4. Zero-Initialized Arrays: Shorter card numbers (like 13-digit Visa) will have leading zeros in the array, which won't break your calculations.

Bonus: Even More Streamlined Approach #

You can skip using arrays entirely by processing digits on the fly, which saves memory and makes the code shorter. Here's a quick example:

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

int main(void) {
    long creditCard = get_long("Credit Card: ");
    long temp = creditCard;
    int sum = 0;
    int length = 0;
    int firstDigit = 0, secondDigit = 0;
    
    while (temp > 0) {
        // Add the last digit to the sum
        sum += temp % 10;
        temp /= 10;
        length++;
        
        if (temp == 0) break;
        
        // Double the next digit, add its individual digits to the sum
        int doubled = (temp % 10) * 2;
        sum += doubled / 10 + doubled % 10;
        temp /= 10;
        length++;
        
        // Track first and second digits for card type checks
        if (temp < 100 && temp >= 10) {
            firstDigit = temp / 10;
            secondDigit = temp % 10;
        } else if (temp < 10 && temp > 0) {
            firstDigit = temp;
            secondDigit = 0;
        }
    }
    
    if (sum % 10 != 0) {
        printf("INVALID\n");
        return 0;
    }
    
    // Determine card type
    if ((length == 15) && (firstDigit == 3) && (secondDigit == 4 || secondDigit == 7)) {
        printf("AMEX\n");
    } else if ((length == 16) && (firstDigit == 5) && (secondDigit >=1 && secondDigit <=5)) {
        printf("MASTERCARD\n");
    } else if ((length ==13 || length ==16) && (firstDigit ==4)) {
        printf("VISA\n");
    } else {
        printf("INVALID\n");
    }
    
    return 0;
}

This approach processes each digit as you go, no arrays needed—perfect for keeping things concise!

内容的提问来源于stack exchange，提问作者Syler