Calculate GS1 / SSCC / UPC check digit

Question

I try to write a function which calculates the check digit for shipping label based on the algorithm provided by GS1 http://www.gs1.org/how-calculate-check-digit-manually

Version 1: my first try

public static int GetGTINCheckDigitV1(string code)
{
 int sum = 0;
 for (int i = 0; i < code.Length; i++)
 {
 int n = int.Parse(code.Substring(code.Length - 1 - i, 1));
 sum += i % 2 == 0 ? n * 3 : n;
 }
 return sum % 10 == 0 ? 0 : 10 - sum % 10;
}

Version 2: using Reverse()

public static int GetGTINCheckDigitV2(string code)
{
 int sum = 0;
 var list = code.Reverse().ToArray();
 for (int i = 0; i < list.Length; i++)
 {
 int n = (int)Char.GetNumericValue(list[i]);
 sum += i % 2 == 0 ? n * 3 : n;
 }
 return sum % 10 == 0 ? 0 : 10 - sum % 10;
}

Version 3: using lambda

public static int GetGTINCheckDigitV3(string code)
{
 var sum = code.Reverse().Select((c, i) => (int)char.GetNumericValue(c) * (i % 2 == 0 ? 3 : 1)).Sum();
 return (10 - sum % 10) % 10;
}

Is there anything I can improve this function or any concern?

Answer 1

It looks good! I only have some minor suggestions. You might want to not have the LINQ expression all on one line. Right now I have to scroll to see it all.

An alternative is to use Enumerable.Range() to create a range of indexes:

public static int GetGTINCheckDigitUsingRange(string code)
{
 var reversed = code.Reverse().ToArray();
 var sum = Enumerable
 .Range(0, reversed.Count())
 .Sum(i => (int)char.GetNumericValue(reversed[i]) * (i % 2 == 0 ? 3 : 1)); 
 return (10 - sum % 10) % 10;
}

Another alternative is to use query syntax:

public static int GetGTINCheckDigitUsingQuery(string code)
{
 var reversed = code.Reverse().ToArray();
 var sum = 
 (from i in Enumerable.Range(0, reversed.Count())
 let digit = (int)char.GetNumericValue(reversed[i])
 select digit * (i % 2 == 0 ? 3 : 1)).Sum();
 return (10 - sum % 10) % 10;
}

I think your version three is still better than my alternatives, but you might prefer one of them. Enumerable.Range() is nice for replacing for loops, and query syntax is nice for declaring intermediate values (like digit) and replacing nested loops. But the Select() method that takes a lambda that includes the index as a parameter is very nice for this situation. With enough practice you can get to the point where your code looks like version three right away. I rarely write loops anymore. I actually find them tougher to write because they make you focus more on "how" than "what".

Answer 2

A public method like this without parameter validation is a red sign. Never ever trust any given input for a public method. Passing null to that method would throw an ArgumentNullException which would be the correct exception but the provided details would be misleading and would expose implementation details to the user. The user of that method doesn't need to know that you use System.Linq.Enumerable.Reverse.

You don't take into account that the maximum digits which the passed in value can contain shouldn't exceed 17 digits (SSCC) either.