Fix for bug #593

* Fixed bug #593

Bug #593 was misleadingly reported as pertaining to another project, but
testing revealed it affected this project as well.  The Code128 encoding
algorithm failed when an odd number of digits were followed by FNC1 and
other digits.  The code selection algorithm was rewritten.  A test case
showing the bug was added, and an old test case was updated to expect
the result of the new algorithm  -  another valid encoding of the same
length as the old one.

* Fixed bug #593

Bug #593 was misleadingly reported as pertaining to another project, but
testing revealed it affected this project as well.  The Code128 encoding
algorithm failed when an odd number of digits were followed by FNC1 and
other digits.  The code selection algorithm was rewritten.  A test case
showing the bug was added, and an old test case was updated to expect
the result of the new algorithm  -  another valid encoding of the same
length as the old one.

* Cleaning up

Added a private enum and changed some names to improve readability.
Also some trivial cosmetic changes.

* Further cleanup of Code128 writer test

Removed a private one-liner method used exactly once, alphabetized
imports.

core/src/main/java/com/google/zxing/oned/Code128Writer.java

@@ -49,6 +49,14 @@ public final class Code128Writer extends OneDimensionalCodeWriter {
   private static final int CODE_FNC_3 = 96;    // Code A, Code B
   private static final int CODE_FNC_4_B = 100; // Code B
 
+  // Results of minimal lookahead for code C
+  private enum CType {
+    UNCODABLE,
+    ONE_DIGIT,
+    TWO_DIGITS,
+    FNC_1
+  }
+
   @Override
   public BitMatrix encode(String contents,
                           BarcodeFormat format,
@@ -93,13 +101,7 @@ public final class Code128Writer extends OneDimensionalCodeWriter {
     
     while (position < length) {
       //Select code to use
-      int requiredDigitCount = codeSet == CODE_CODE_C ? 2 : 4;
-      int newCodeSet;
-      if (isDigits(contents, position, requiredDigitCount)) {
-        newCodeSet = CODE_CODE_C;
-      } else {
-        newCodeSet = CODE_CODE_B;
-      }
+      int newCodeSet = chooseCode(contents, position, codeSet);
       
       //Get the pattern index
       int patternIndex;
@@ -182,19 +184,72 @@ public final class Code128Writer extends OneDimensionalCodeWriter {
     return result;
   }
 
-  private static boolean isDigits(CharSequence value, int start, int length) {
-    int end = start + length;
+  private static CType findCType(CharSequence value, int start) {
     int last = value.length();
-    for (int i = start; i < end && i < last; i++) {
-      char c = value.charAt(i);
+    if (start >= last) {
+      return CType.UNCODABLE;
+    }
+    char c = value.charAt(start);
+    if (c == ESCAPE_FNC_1) {
+      return CType.FNC_1;
+    }
     if (c < '0' || c > '9') {
-        if (c != ESCAPE_FNC_1) {
-          return false;
+      return CType.UNCODABLE;
     }
-        end++; // ignore FNC_1
+    if (start + 1 >= last) {
+      return CType.ONE_DIGIT;
+    }
+    c = value.charAt(start + 1);
+    if (c < '0' || c > '9') {
+      return CType.ONE_DIGIT;
+    }
+    return CType.TWO_DIGITS;
+  }
+
+  private static int chooseCode(CharSequence value, int start, int oldCode) {
+    CType lookahead = findCType(value, start);
+    if (lookahead == CType.UNCODABLE || lookahead == CType.ONE_DIGIT) {
+      return CODE_CODE_B; // no choice
+    }
+    if (oldCode == CODE_CODE_C) { // can continue in code C
+      return oldCode;
+    }
+    if (oldCode == CODE_CODE_B) {
+      if (lookahead == CType.FNC_1) {
+        return oldCode; // can continue in code B
+      }
+      // Seen two consecutive digits, see what follows
+      lookahead = findCType(value, start + 2);
+      if (lookahead == CType.UNCODABLE || lookahead == CType.ONE_DIGIT) {
+        return oldCode; // not worth switching now
+      }
+      if (lookahead == CType.FNC_1) { // two digits, then FNC_1...
+        lookahead = findCType(value, start + 3);
+        if (lookahead == CType.TWO_DIGITS) { // then two more digits, switch
+          return CODE_CODE_C;
+        } else {
+          return CODE_CODE_B; // otherwise not worth switching
         }
       }
-    return end <= last; // end > last if we've run out of string
+      // At this point, there are at least 4 consecutive digits.
+      // Look ahead to choose whether to switch now or on the next round.
+      int index = start + 4;
+      while ((lookahead = findCType(value, index)) == CType.TWO_DIGITS) {
+        index += 2;
+      }
+      if (lookahead == CType.ONE_DIGIT) { // odd number of digits, switch later
+        return CODE_CODE_B;
+      }
+      return CODE_CODE_C; // even number of digits, switch now
+    }
+    // Here oldCode == 0, which means we are choosing the initial code
+    if (lookahead == CType.FNC_1) { // ignore FNC_1
+      lookahead = findCType(value, start + 1);
+    }
+    if (lookahead == CType.TWO_DIGITS) { // at least two digits, start in code C
+      return CODE_CODE_C;
+    }
+    return CODE_CODE_B;
   }
 
 }

core/src/test/java/com/google/zxing/oned/Code128WriterTestCase.java

@@ -21,8 +21,11 @@ import org.junit.Before;
 import org.junit.Test;
 
 import com.google.zxing.BarcodeFormat;
+import com.google.zxing.ReaderException;
+import com.google.zxing.Result;
 import com.google.zxing.Writer;
 import com.google.zxing.WriterException;
+import com.google.zxing.common.BitArray;
 import com.google.zxing.common.BitMatrix;
 
 public class Code128WriterTestCase extends Assert {
@@ -32,14 +35,18 @@ public class Code128WriterTestCase extends Assert {
   private static final String FNC3 = "10111100010";
   private static final String FNC4 = "10111101110";
   private static final String START_CODE_B = "11010010000";
+  private static final String START_CODE_C = "11010011100";
+  private static final String SWITCH_CODE_B = "10111101110";
   private static final String QUIET_SPACE = "00000";
   private static final String STOP = "1100011101011";
 
   private Writer writer;
+  private Code128Reader reader;
 
   @Before
   public void setup() {
     writer = new Code128Writer();
+    reader = new Code128Reader();
   }
 
   @Test
@@ -69,8 +76,8 @@ public class Code128WriterTestCase extends Assert {
   @Test
   public void testEncodeWithFunc1() throws WriterException {
     String toEncode = "\u00f1" + "123";
-    //                                                       "1"            "2"             "3"          check digit 61
-    String expected = QUIET_SPACE + START_CODE_B + FNC1 + "10011100110" + "11001110010" + "11001011100" + "11001000010" + STOP + QUIET_SPACE;
+    //                                                       "12"                           "3"          check digit 92
+    String expected = QUIET_SPACE + START_CODE_C + FNC1 + "10110011100" + SWITCH_CODE_B + "11001011100" + "10101111000" + STOP + QUIET_SPACE;
 
     BitMatrix result = writer.encode(toEncode, BarcodeFormat.CODE_128, 0, 0);
 
@@ -78,6 +85,18 @@ public class Code128WriterTestCase extends Assert {
     assertEquals(expected, actual);
   }
 
+  @Test
+  public void testRoundtrip() throws WriterException, ReaderException {
+    String toEncode = "\u00f1" + "10958" + "\u00f1" + "17160526";
+    String expected = "1095817160526";
+
+    BitMatrix encResult = writer.encode(toEncode, BarcodeFormat.CODE_128, 0, 0);
+    BitArray row = encResult.getRow(0, null);
+    Result rtResult = reader.decodeRow(0, row, null);
+    String actual = rtResult.getText();
+    assertEquals(expected, actual);
+  }
+
   @Test
   public void testEncodeWithFunc4() throws WriterException {
     String toEncode = "\u00f4" + "123";