Programming a Luhn Algorithm

Question

i have a luhn algorithm, i am trying to follow the steps on Wikipedia for the algorithm, and it works for the examples they give. and i thought it was correct. But it doesn't work on any of my personal cards. nor on any of the test values i have found around looking for a solution to this.

I have seen other solutions for this using lamba and inline linq. But i don't want to copy and paste anything. i would rather actually understand what i am coding.

49927398716      pass
49927398717      fail
1234567812345678 fail 
1234567812345670 pass (mine fails this one)

my code is as follows.

    private bool CalculateLuhnAlgorithm()
    {
        string newListOfNumbers = this._number; //this._number is a string
        int sumOfAllValues = 0;
        if (_cardType != CardType.Unavailable)
        {
            //odd numbers minus the check Digit. 
            for (int i = this._number.Length - 2; i > 0; i -= 2)
            {
                int number = (int)char.GetNumericValue(this._number[i]) * 2;
                if (number >= 10)
                {
                    string concatinatedNumber = number.ToString();
                    int firstNumber = (int)char.GetNumericValue(concatinatedNumber[0]);
                    int secondNumber = (int)char.GetNumericValue(concatinatedNumber[1]);
                    number = firstNumber + secondNumber;
                }
                newListOfNumbers = newListOfNumbers.Remove(i, 1);
                newListOfNumbers = newListOfNumbers.Insert(i, number.ToString());
            }

            // add up the complete total
            foreach (char c in newListOfNumbers)
            {
                sumOfAllValues += (int)char.GetNumericValue(c);
            }

        }

        // get the luhn validity
        return (sumOfAllValues %10) == 0;
    }

@DrKoch The check digit is summed up the the other numbers which should result in a value that is divisible by 10. — juharr
– juharr, Commented Jan 8, 2015 at 16:41
Also, you should check that your input string has an even length — DrKoch
– DrKoch, Commented Jan 8, 2015 at 16:43
@DrKoch The newListOfNumbers is initialized as the string, then the odd number are doubled and replaced. Then the digits of newListOfNumbers are summed, which includes the last digit of the original string, the check digit. Also I totally agree that a check for the length of the string should be added. — juharr
– juharr, Commented Jan 8, 2015 at 16:47

juharr · Accepted Answer · 2015-01-08 17:07:14Z

The problem is that your for loop continues while i > 0. That means you never double the first digit. It should instead continue while i >= 0.

Now here are some other suggestions.

When you double the number and it is greater than 10 you can just do

if(number > 10)
    number = (number % 10) + 1;

This works because the highest single digit value (9) doubled is (18). So the first digit is always 1 and you can just mod the number by 10 to get the second digit.

You should add a check to make sure the string has an even number of digits, and maybe specific lower and upper limits depending on what the account numbers you are working with are.

Instead of creating a second string that contains the digits with the "odd" ones doubled you should just keep track of sum as you iterate through the digits, like this.

private bool CalculateLuhnAlgorithm()
{
    if(this.number.Length % 2 != 0)
        return false; // Or maybe throw an exception?
    if (_cardType != CardType.Unavailable)
    {
        int sumOfAllValues = 0;
        //odd numbers minus the check Digit. 
        for (int i = 0; i < this._number.Length; i++)
        {
            if(i%2 != 0) // because i is 0 based instead of 1 based.
                sumOfAllValues += (int)char.GetNumericValue(this._number[i])
            else
            {
                int number = (int)char.GetNumericValue(this._number[i]) * 2;
                if (number >= 10)
                {
                    number = (number % 10) + 1;
                }
                sumOfAllValues += number;
            }
        }

        // get the luhn validity
        return (sumOfAllValues %10) == 0;
    }
    else
    {
         // Not completely sure what this should do, 
         // but in your code it just results in true.
         return true;
    }
}

Finially you might want to replace the char.GetNumericValue with int.TryParse so you can validate strings that contain non-numeric values. char.GetNumericValue returns -1 for values that are not numeric.

int digit = 0;
if(!int.TryParse(this._number.SubString(i, 1), out digit)
{
    return false;  // Or throw an exception.
}

Collectives™ on Stack Overflow

Programming a Luhn Algorithm

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related