HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Calculate GS1 / SSCC / UPC check digit

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
upcgs1ssccdigitcalculatecheck

Problem

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?

Solution

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

Code Snippets

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

Context

StackExchange Code Review Q#126685, answer score: 2

Revisions (0)

No revisions yet.