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;
    }

Answer 1

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.
}