current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I am decoding GS1 barcodes and need to match up application identifiers with relevant data about them.

00    SSCC (Serial Shipping Container Code)   N2+N18  FALSE   SSCC 
01    Global Trade Item Number (GTIN)         N2+N14  FALSE   GTIN 
02    GTIN of Contained Trade Items           N2+N14  FALSE   CONTENT 
10    Batch or Lot Number                     N2+X.20 TRUE    BATCH/LOT 
11    Production Date (YYMMDD)                N2+N6   FALSE   PROD DATE 
12    Due Date (YYMMDD)                       N2+N6   FALSE   DUE DATE 
13    Packaging Date (YYMMDD)                 N2+N6   FALSE   PACK DATE 
15    Best Before Date (YYMMDD)               N2+N6   FALSE   BEST BEFORE or BEST BY 
16    Sell By Date (YYMMDD)                   N2+N6   FALSE   SELL BY 
17    Expiration Date (YYMMDD)                N2+N6   FALSE   USE BY OR EXPIRY 
20    Variant Number                          N2+N2   FALSE   VARIANT 
21    Serial Number                           N2+X.20 TRUE    SERIAL 
240   Additional Item Identification          N3+X.30 TRUE    ADDITIONAL ID 
241   Customer Part Number                    N3+X.30 TRUE    CUST. PART NO. 
242   Made-to-Order Variation Number          N3+N.6  TRUE    MTO VARIANT 
243   Packaging Component Number              N3+X.20 TRUE    PCN 
250   Secondary Serial Number                 N3+X.30 TRUE    SECONDARY SERIAL 
251   Reference to Source Entity              N3+X.30 TRUE    REF. TO SOURCE 
253   Global Document Type Identifier (GDTI)  N3+N13+XTRUE    GDTI 
254   GLN Extension Component                 N3+X.20 TRUE    GLN EXTENSION COMPONENT 
255   Global Coupon Number (GCN)              N3+N13+NTRUE    GCN 
30    Count of Items (Variable Measu ...      N2+N..8 TRUE    VAR. COUNT 
3100  Net weight  kilograms (Var... )     N4+N6   FALSE   NET WEIGHT (kg) 
...

Full list here.

This is how I identify them:

public Dictionary<int, Tuple<string, string, bool, string>> gs1AiDict { get; private set; } 

var aiDataOut = new Tuple<string, string, bool, string>("","",false,"");

//Get first 4 digits (max identifier length)
//the string input pi might look like this: ~10000123
int applicationIdentifier = Convert.ToInt16(pi.Substring(1,4));
//Try to find an identifier with 2 digits.
if (gs1AiDict.TryGetValue(applicationIdentifier / 100, out aiDataOut))
{
    //assign found data to tuple
    id = aiDataOut.Item1;
    format = aiDataOut.Item2;
    func = aiDataOut.Item3;
    abrv = aiDataOut.Item4;
}
//look up the first 3 digits
else
{
    if (gs1AiDict.TryGetValue(applicationIdentifier / 10, out aiDataOut))
    {
        id = aiDataOut.Item1;
        format = aiDataOut.Item2;
        func = aiDataOut.Item3;
        abrv = aiDataOut.Item4;
    }
    //look up first 4 digits
    else
    {
        if (gs1AiDict.TryGetValue(applicationIdentifier, out aiDataOut))
        {
            id = aiDataOut.Item1;
            format = aiDataOut.Item2;
            func = aiDataOut.Item3;
            abrv = aiDataOut.Item4;
        }
        //no identifiers are 5 digits
        else
        {
         console.log("No Application Identifier found!");
        }
    }
}

My tests have worked wonderfully, but my issue is the readability and efficiency of this solution. Are 3 dictionary key look ups for about 140 keys too much?

share|improve this question
    
Why are you dividing the identifiers by 10 or 100? That seems like a bug waiting to happen. For example, 1 (Global Trade Item Number) and 10 (Batch or lot number) will resolve to the same entry, given your look-up order and conversion to short values. –  Dan Lyons Nov 17 '14 at 18:42
    
You're right. I have changed the keys to be strings that way no conversion will be required. I had initially made them integers because I thought it made sense, but all this conversion can not be good. –  user4067565 Nov 17 '14 at 19:06
    
What you may and may not do after receiving answers –  Jamal♦ Nov 17 '14 at 19:13

1 Answer 1

up vote 3 down vote accepted

Code duplication is a sin

You could use shorter and more readable code using a simple for loop:

int applicationIdentifier = Convert.ToInt16(pi.Substring(1,4));
for (int i = 100; i >= 1; i /= 10)
{
    int maybeId = applicationIdentifier / i;
    if (gs1AiDict.TryGetValue(maybeId, out aiDataOut))
    {
        id = aiDataOut.Item1;
        format = aiDataOut.Item2;
        func = aiDataOut.Item3;
        abrv = aiDataOut.Item4;
        return;
    }
}   
Console.WriteLine("No Application Identifier found!");

You should also consider making your data object something along the lines of a Dicitionary<int, ApplicationInfo>, that 4-member tuple doesn't help readibility. Then you could also return an ApplicationInfo object instead of copying the fields to local variables.

share|improve this answer
    
Indeed that function is much cleaner. Regarding the use of "Application Info" to hold the data: could you point me to a use of that? When I was researching how to get multiple parameters per key, all I found were Tuples. –  user4067565 Nov 17 '14 at 16:18
    
@Phyre ApplicationInfo is a name I made up, it's not a existing type. I was suggesting that you should create a class that holds a single entry of application information. –  Rotem Nov 17 '14 at 16:23

Your Answer

 
discard

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.