up vote 1 down vote favorite

How can I improve or shorten my implementation of the Luhn algorithm?

    int digits;
    cout << "enter the numer of digits" << endl;
    cin >> digits;
    int Acc_num[digits];
    int shuma = 0;

The shuma variable is the sum.

I created an array to store the digits from the string credit_card.

And asked the user how many digits his credit card has, etc. (the digits int). 

    string credit_card;
    cout << "enter the identification number" << endl;
    cin >> credit_card;

    // store the digits
    for (int i = 0 ; i < digits ; i ++ ) {
        Acc_num[i] = credit_card[i];
    }

    for (int i = 0 ; i <= (digits - 1) ; i ++ ) {
        Acc_num[i] -= 48;
    }

    // Double every other
    for (int i = 1 ; i <=digits ; i ++) {
        if (i % 2 == 0) {
            Acc_num[i-1] =  2 * Acc_num[i-1];
        } else {
            Acc_num[i-1] = Acc_num [i-1];
        }
    }

    //Sum digits
    for (int i = 1 ; i <= digits ; i ++ ) {
        if (Acc_num[i-1] > 9 && i % 2 == 0) {
            int mod = Acc_num[i-1] % 10 ;
            Acc_num[i-1]  =  1 + mod ;
        } else {
        Acc_num[i-1] = Acc_num[i-1];
        }
    }

    // the sum
    for (int i = 0 ; i <= (digits - 1) ; i ++ ) {
        shuma += Acc_num[i];
    }

    if (shuma % 10 == 0) {
        cout << "\nthis number is valid" << endl;
    } else {
        cout << "\nthis number is invalid" << endl;
    }
share|improve this question

migrated from stackoverflow.com Apr 29 '16 at 20:45

This question came from our site for professional and enthusiast programmers.
up vote 2 down vote accepted

Doubling the even digits and subtracting 9 if > 10 would be mod 9 so a really simple, compact algorithm might be:

for (int i = 0; i < nDigits; i++)
{
    int   digit = digitArray [i] - '0';   // Char to number
    if  (i & 1)  // Digit 1, 3, 5 not 0, 2, 4 - "even digits" starting at 1
        if  ((digit <<= 1) >= 10) //  Double it, check >= 10
            digit -= 9;           //  Same as summing the digits
    shuma += digit;
}
int checksum = shuma % 10;
share|improve this answer
    
Code error - should have been >= 10, not > 10 but you get the idea. Break the problem down, solve with math where possible and don't write code that "does" things, write code that "takes care of" them. – Mike Apr 24 '16 at 20:27
up vote 1 down vote

It looks like you could do all of those calculations in one loop. Also, all of the places where you do Acc_num[i-1] = Acc_num [i-1]; amount to a no-op, so you could remove them.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.