This is a pretty good start. Your code is readable and understandable. Your variable names are descriptive, and you've used the right types for the work you're doing. Here are a few suggestions for improvements.
#Dealing With Strings vs. Numbers
This is a case where some of your code would be easier to write if you were dealing with strings instead of numbers. You're fortunate in that a 13-16 digit number fits inside of a long long
, and there should be no valid credit card numbers with leading 0s. That gets rid of 2 problematic issues with validating numerical inputs.
If you obtained the user's input as a string instead of a long long
, you'd need to convert the characters to values so you could check the checksum. At first that seems like more work. But here's a function that converts a char
to an int
:
int charToInt(const char c)
{
if (isdigit(c))
{
return (int)(c - `0`);
}
return -1;
}
Once you have that, getting the digits out of the card is about as easy as doing it numerically, but you don't have to keep dividing the number and making copies of it. Your checksum loop ends up looking like this:
char* cardNumber; // Assume this was gotten above and is filled out correctly
// start from second to last digit (cc_number / 10) % 10 and multiply every other digit by 2.
int nextDigit;
unsigned long numberLen = strlen(cardNumber);
char* nextDigitChar = cardNumber + numberLen - 1;
while (nextDigitChar > cardNumber)
{
// sum digits that weren't multiplied.
nextDigit = charToInt(*nextDigitChar);
// add the sum to the sum of the digits that weren't multipied by 2.
checkSum += nextDigit;
nextDigitChar--;
// Get the security digit
secDig = charToInt(*nextDigitChar);
nextDigitChar--;
// multiply every other digit
multDig = secDig * 2;
// add those products (separate /10, return modulus of 10) digits together.
if (multDig > 10)
{
checkSum += (multDig % 10) + 1;
}
else if (multDig == 10)
{
checkSum += 1;
}
else
{
checkSum += multDig;
}
}
Now this probably doesn't look like a whole lot less work than what you were doing before. However, notice that we haven't modified the input at all. So we don't need to copy it again to do other work. Speaking of which...
I would check the length of the input before doing any other work. If it isn't 13-16 characters, then there's no point in doing any checksum because we already know it's invalid. Note also, that this makes the last part of your program much easier, too. You can get the first 2 digits by simply looking at the string.
if (valid)
{
int vendorCode = charToInt(cardNumber[0]) * 10 + charToInt(cardNumber[1]);
if ((numberLen == 15) && (vendorCode >= 34 && vendorCode <= 37))
{
printf("AMEX\n");
}
else if ((numberLen == 16) && (vendorCode >= 51 && vendorCode <= 55))
{
printf("MASTERCARD\n");
}
else if (((numberLen == 13) || (numberLen == 16)) && (vendorCode / 10 == 4))
{
printf("VISA\n");
}
else
{
valid = false;
}
}
if (!valid)
{
printf("INVALID\n");
}