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

SGTIN96 encode for EPC on RFID

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

Problem

This is a class to encode and decode SGTIN96 product identifiers, typically used to write EPC on RFID tags. Refer to EPC(TM) Generation 1 Tag Data Standards Version 1.1 Rev.1.27.

```
import java.math.BigInteger;
import java.util.HashMap;
import java.util.Map;

// Refer to EPCTM Generation 1 Tag Data Standards Version 1.1 Rev.1.27
// http://www.gs1.org/sites/default/files/docs/epc/tds_1_1_rev_1_27-standard-20050510.pdf

public class SGTIN96 {
// Table 6. The EPC SGTIN-96 bit allocation, header, and maximum decimal values. page 27.
private final static Integer BIN = 2;
private final static Integer HEX = 16;
private final static String sgtin96_bin_header = "00110000";
private final static Integer sgtin96_filter_value_bits = 3;
private final static Integer sgtin96_partition_value_bits = 3;
private final static Integer sgtin96_serial_number_bits = 38;
private final static Integer sgtin96_length_bits = 96;
private final static Integer sgtin96_length_hex = 24;
private static HashMap sgtin96_company_prefix_len_partitions;

static {
// Table 7. SGTIN-96 Partitions. page 28.
// Column order: (L), P, M, N Nd
sgtin96_company_prefix_len_partitions = new HashMap();
sgtin96_company_prefix_len_partitions.put(12, new int[]{0, 40, 4, 1});
sgtin96_company_prefix_len_partitions.put(11, new int[]{1, 37, 7, 2});
sgtin96_company_prefix_len_partitions.put(10, new int[]{2, 34, 10, 3});
sgtin96_company_prefix_len_partitions.put(9, new int[]{3, 30, 14, 4});
sgtin96_company_prefix_len_partitions.put(8, new int[]{4, 27, 17, 5});
sgtin96_company_prefix_len_partitions.put(7, new int[]{5, 24, 20, 6});
sgtin96_company_prefix_len_partitions.put(6, new int[]{6, 20, 24, 7});
}

private static int[] getPartitionsByCompanyPrefixLengthInDigits(int company_prefix_length) {
// column 3 (L)
return sgtin96_company_prefix_len_partitions.get(company_prefix_length)

Solution

I hope there are tests. Because when I attempted to refactor some parts
I wasn't quite sure if it retained the meaning or not.

There are docstrings, but I think the exceptional situations should at
least be mentioned, i.e. "Malformed input will raise an exception
instead of returning empty output." or so.

Reuse computed values, e.g. serial_number.toString(), to reduce the
lines of code to read; it will be probably optimised away regardless.

Some variables don't have the best names, e.g. thirteen doesn't tell
anything about the intended use.

sgtin96_company_prefix_len_partitions can be final too.

The inner part of the loop could reuse the same computation. Also
substring isn't the best option here - charAt is potentially more
performant, but also requires changing parseInt to something
different; see also
this SO post:

int parsed = Character.getNumericValue(thirteen.charAt(i));
if (i%2 == 0) {
    termB += parsed;
} else {
    termA += parsed;
}


The return value should be more structured than a map, the easiest way
for that would be a value class with three members:

public static class EPCID {
    /**
     * Encoded Filter Value
     */
    private final String filterValue;
    /**
     * Item Reference
     */
    private final String itemReference;
    /**
     * Serial Number
     */
    private final String serialNumber;
    /**
     * Encoded GTIN-14
     */
    private final String gtin14;

    public EPCID(String filterValue, String itemReference, String serialNumber, String gtin14) {
        this.filter_value = filterValue;
        this.item_reference = itemReference;
        this.serial_number = serialNumber;
        this.gtin14 = gtin14;
    }

    public String getFilterValue() {
        return this.filterValue;
    }

    public String getItemReference() {
        return this.itemReference;
    }

    public String getSerialNumber() {
        return this.serialNumber;
    }

    public String getGtin14() {
        return this.gtin14;
    }
}


Which is then returned from the method:

/**
 * ...
 * @return the decoded EPCID
 */
public static EPCID decode(String sgtin96_epc) {
    ...
    return new EPCID(filter_value.toString(), item_reference_and_indicator, serial_number.toString(), gtin14);
}


Possibly change the data types if String isn't actually the best
option there. If the map is really needed I'd add a conversion/view to
the value class instead.

The helper functions (binary to hex and back) should probably be moved
into a separate class if used anywhere else since they aren't really
part of this classes responsibility.

The zero-fill function can be a bit nicer by exiting early and using
Arrays.fill
instead of the replace call, which does more than you need it to:

private static String zeroFill(String s, int n) {
    int fill = n - s.length();
    if (fill == 0) {
        return s;
    }
    char[] zeroes = new char[fill];
    Arrays.fill(zeroes, '0');
    return zeroes + s;
}


If it's a concern the whole class should be profiled for unnecessary
allocations btw. since it's using a lot of string contenations
etc. which could possibly be avoided by using
StringBuilder,
or by preallocating space.

That serial_number is sometimes a Long and sometimes a String is
hopefull not an issue? E.g. a leading zero doesn't have to be
preserved?

The serial number check can be moved out of the encode method easily:

private static void checkSerialNumber(Long serial_number) {
    String serial_number_string = serial_number.toString();
    if (serial_number_string.length() > 1 && serial_number_string.charAt(0) == '0') {
        throw new IllegalArgumentException("serial number may not begin with 0");
    }
}


This also highlights one question: In which situation, except it being
0, does this raise an exception?

Unless I'm missing something, sgtin96_company_prefix_len_partitions
could very well be a regular two-dimensional array instead of a map.

The naming scheme is all over the place; at least constants should be
uppercase though, that makes it a bit easier to understand.

Some common code can still be factored out,
e.g. binaryToInt(string.substring(...)) can be moved into a separate
method as it's used quite often:

private static Integer binarySubstringToInt(String bin, int start, int end) {
    return Integer.parseInt(bin.substring(start, end), BIN);
}


The method getPartitionsByPartitionValue returns an opaque int[]
array. It would be better if the return value actually had some
meaningful name. The method is also linearly scanning the list, which
is per se bad; since the input value is already verified, I'd rather
have a pre-constructed array somewhere, where it would be just
array[partition_value] instead.

Code Snippets

int parsed = Character.getNumericValue(thirteen.charAt(i));
if (i%2 == 0) {
    termB += parsed;
} else {
    termA += parsed;
}
public static class EPCID {
    /**
     * Encoded Filter Value
     */
    private final String filterValue;
    /**
     * Item Reference
     */
    private final String itemReference;
    /**
     * Serial Number
     */
    private final String serialNumber;
    /**
     * Encoded GTIN-14
     */
    private final String gtin14;

    public EPCID(String filterValue, String itemReference, String serialNumber, String gtin14) {
        this.filter_value = filterValue;
        this.item_reference = itemReference;
        this.serial_number = serialNumber;
        this.gtin14 = gtin14;
    }

    public String getFilterValue() {
        return this.filterValue;
    }

    public String getItemReference() {
        return this.itemReference;
    }

    public String getSerialNumber() {
        return this.serialNumber;
    }

    public String getGtin14() {
        return this.gtin14;
    }
}
/**
 * ...
 * @return the decoded EPCID
 */
public static EPCID decode(String sgtin96_epc) {
    ...
    return new EPCID(filter_value.toString(), item_reference_and_indicator, serial_number.toString(), gtin14);
}
private static String zeroFill(String s, int n) {
    int fill = n - s.length();
    if (fill == 0) {
        return s;
    }
    char[] zeroes = new char[fill];
    Arrays.fill(zeroes, '0');
    return zeroes + s;
}
private static void checkSerialNumber(Long serial_number) {
    String serial_number_string = serial_number.toString();
    if (serial_number_string.length() > 1 && serial_number_string.charAt(0) == '0') {
        throw new IllegalArgumentException("serial number may not begin with 0");
    }
}

Context

StackExchange Code Review Q#105121, answer score: 3

Revisions (0)

No revisions yet.