Fixed or removed a bunch of TODOs, and enforced the 100 columns limit in a bunch of places. Also allowed QR Codes to be encoded of contacts without names.

git-svn-id: https://zxing.googlecode.com/svn/trunk@907 59b500cc-1b3d-0410-9834-0bbf25fbcc57

android/build.xml

@@ -278,10 +278,7 @@ limitations under the License.
     </exec>
   </target>
 
-  <!-- Invoke the proper target depending on whether or not
+  <!-- Invoke the proper target depending on whether or not an assets directory is present. -->
-an assets directory is present. -->
-  <!-- TODO: find a nicer way to include the "-A ${asset-dir}" argument
-only when the assets dir exists. -->
   <target name="package-res">
     <available file="${asset-dir}" type="dir"
                property="res-target" value="and-assets" />

android/src/com/google/zxing/client/android/DecodeThread.java

@@ -101,8 +101,9 @@ final class DecodeThread extends Thread {
     mMultiFormatReader.setHints(hints);
   }
 
-  // TODO: This is fragile in case we add new formats. It would be better to have a new enum
+  /**
-  // value which represented all 1D formats.
+   * Select the 1D formats we want this client to decode by hand.
+   */
   private void setDecode1DMode() {
     Hashtable<DecodeHintType, Object> hints = new Hashtable<DecodeHintType, Object>(3);
     Vector<BarcodeFormat> vector = new Vector<BarcodeFormat>();

android/src/com/google/zxing/client/android/QRCodeEncoder.java

@@ -61,7 +61,8 @@ public final class QRCodeEncoder {
     return mTitle;
   }
 
-  // TODO: The string encoding should live in the core ZXing library.
+  // It would be nice if the string encoding lived in the core ZXing library, but we use platform
+  // specific code like PhoneNumberUtils, so it can't.
   private boolean encodeContents(Intent intent) {
     if (intent == null) {
       return false;
@@ -102,10 +103,13 @@ public final class QRCodeEncoder {
     } else if (type.equals(Contents.Type.CONTACT)) {
       Bundle bundle = intent.getBundleExtra(Intents.Encode.DATA);
       if (bundle != null) {
+        mContents = "MECARD:";
+        mDisplayContents = "";
         String name = bundle.getString(Contacts.Intents.Insert.NAME);
         if (name != null && name.length() > 0) {
-          mContents = "MECARD:N:" + name + ';';
+          mContents = "N:" + name + ';';
-          mDisplayContents = name;
+          mDisplayContents += name;
+        }
         String address = bundle.getString(Contacts.Intents.Insert.POSTAL);
         if (address != null && address.length() > 0) {
           mContents += "ADR:" + address + ';';
@@ -125,8 +129,12 @@ public final class QRCodeEncoder {
             mDisplayContents += '\n' + email;
           }
         }
+        // Make sure we've encoded at least one field.
+        if (mDisplayContents.length() > 0) {
           mContents += ";";
           mTitle = mActivity.getString(R.string.contents_contact);
+        } else {
+          mContents = null;
         }
       }
     } else if (type.equals(Contents.Type.LOCATION)) {

android/src/com/google/zxing/client/android/ShareActivity.java

@@ -143,11 +143,11 @@ public final class ShareActivity extends Activity {
     if (contactCursor != null && contactCursor.moveToFirst()) {
       int nameColumn = contactCursor.getColumnIndex(Contacts.PeopleColumns.NAME);
       String name = contactCursor.getString(nameColumn);
-      if (name == null || name.length() == 0) {
+
-        // TODO: Show error
+      // Don't require a name to be present, this contact might be just a phone number.
-        return;
+      if (name != null && name.length() > 0) {
-      }
         bundle.putString(Contacts.Intents.Insert.NAME, name);
+      }
       contactCursor.close();
 
       Uri phonesUri = Uri.withAppendedPath(contactUri, Contacts.People.Phones.CONTENT_DIRECTORY);

android/src/com/google/zxing/client/android/result/ResultHandler.java

@@ -149,8 +149,9 @@ public abstract class ResultHandler {
   public final void addContact(String[] names, String[] phoneNumbers, String[] emails, String note,
                          String address, String org, String title) {
 
+    // Only use the first name in the array, if present.
     Intent intent = new Intent(Contacts.Intents.Insert.ACTION, Contacts.People.CONTENT_URI);
-    putExtra(intent, Contacts.Intents.Insert.NAME, names);
+    putExtra(intent, Contacts.Intents.Insert.NAME, names != null ? names[0] : null);
 
     int phoneCount = Math.min((phoneNumbers != null) ? phoneNumbers.length : 0,
         Contents.PHONE_KEYS.length);
@@ -188,7 +189,8 @@ public abstract class ResultHandler {
   }
 
   public final void shareBySMS(String contents) {
-    sendSMSFromUri("smsto:", mActivity.getString(R.string.msg_share_subject_line) + ":\n" + contents);
+    sendSMSFromUri("smsto:", mActivity.getString(R.string.msg_share_subject_line) + ":\n" +
+        contents);
   }
 
   public final void sendSMS(String phoneNumber, String body) {
@@ -252,7 +254,8 @@ public abstract class ResultHandler {
   }
 
   public final void openProductSearch(String upc) {
-    Uri uri = Uri.parse("http://www.google." + LocaleManager.getCountryTLD() + "/products?q=" + upc);
+    Uri uri = Uri.parse("http://www.google." + LocaleManager.getCountryTLD() + "/products?q=" +
+        upc);
     launchIntent(new Intent(Intent.ACTION_VIEW, uri));
   }
 
@@ -299,11 +302,4 @@ public abstract class ResultHandler {
     }
   }
 
-  // TODO: This is only used by the names field, and only the first name will be taken.
-  private static void putExtra(Intent intent, String key, String[] value) {
-    if (value != null && value.length > 0) {
-      putExtra(intent, key, value[0]);
-    }
-  }
-
 }

androidtest/build.xml

@@ -263,10 +263,7 @@ limitations under the License.
     </exec>
   </target>
 
-  <!-- Invoke the proper target depending on whether or not
+  <!-- Invoke the proper target depending on whether or not an assets directory is present. -->
-an assets directory is present. -->
-  <!-- TODO: find a nicer way to include the "-A ${asset-dir}" argument
-only when the assets dir exists. -->
   <target name="package-res">
     <available file="${asset-dir}" type="dir"
                property="res-target" value="and-assets" />

core/src/com/google/zxing/oned/Code128Reader.java

@@ -187,7 +187,8 @@ public final class Code128Reader extends AbstractOneDReader {
           int bestVariance = MAX_AVG_VARIANCE;
           int bestMatch = -1;
           for (int startCode = CODE_START_A; startCode <= CODE_START_C; startCode++) {
-            int variance = patternMatchVariance(counters, CODE_PATTERNS[startCode], MAX_INDIVIDUAL_VARIANCE);
+            int variance = patternMatchVariance(counters, CODE_PATTERNS[startCode],
+                MAX_INDIVIDUAL_VARIANCE);
             if (variance < bestVariance) {
               bestVariance = variance;
               bestMatch = startCode;
@@ -195,7 +196,8 @@ public final class Code128Reader extends AbstractOneDReader {
           }
           if (bestMatch >= 0) {
             // Look for whitespace before start pattern, >= 50% of width of start pattern            
-            if (row.isRange(Math.max(0, patternStart - (i - patternStart) / 2), patternStart, false)) {
+            if (row.isRange(Math.max(0, patternStart - (i - patternStart) / 2), patternStart,
+                false)) {
               return new int[]{patternStart, i, bestMatch};
             }
           }
@@ -228,7 +230,7 @@ public final class Code128Reader extends AbstractOneDReader {
         bestMatch = d;
       }
     }
-    // TODO We're overlooking the fact that the STOP pattern has 7 values, not 6
+    // TODO We're overlooking the fact that the STOP pattern has 7 values, not 6.
     if (bestMatch >= 0) {
       return bestMatch;
     } else {
@@ -313,8 +315,8 @@ public final class Code128Reader extends AbstractOneDReader {
           } else if (code < 96) {
             result.append((char) (code - 64));
           } else {
-            // Don't let CODE_STOP, which always appears, affect whether whether we think the last code
+            // Don't let CODE_STOP, which always appears, affect whether whether we think the last
-            // was printable or not
+            // code was printable or not.
             if (code != CODE_STOP) {
               lastCharacterWasPrintable = false;
             }
@@ -416,13 +418,14 @@ public final class Code128Reader extends AbstractOneDReader {
 
     }
 
-    // Check for ample whitespice following pattern, but, to do this we first need to remember that we
+    // Check for ample whitespice following pattern, but, to do this we first need to remember that
-    // fudged decoding CODE_STOP since it actually has 7 bars, not 6. There is a black bar left to read off.
+    // we fudged decoding CODE_STOP since it actually has 7 bars, not 6. There is a black bar left
-    // Would be slightly better to properly read. Here we just skip it:
+    // to read off. Would be slightly better to properly read. Here we just skip it:
     while (row.get(nextStart)) {
       nextStart++;
     }
-    if (!row.isRange(nextStart, Math.min(row.getSize(), nextStart + (nextStart - lastStart) / 2), false)) {
+    if (!row.isRange(nextStart, Math.min(row.getSize(), nextStart + (nextStart - lastStart) / 2),
+        false)) {
       throw ReaderException.getInstance();
     }
 
@@ -435,8 +438,8 @@ public final class Code128Reader extends AbstractOneDReader {
 
     // Need to pull out the check digits from string
     int resultLength = result.length();
-    // Only bother if, well, the result had at least one character, and if the checksum digit happened
+    // Only bother if the result had at least one character, and if the checksum digit happened to
-    // to be a printable character. If it was just interpreted as a control code, nothing to remove
+    // be a printable character. If it was just interpreted as a control code, nothing to remove.
     if (resultLength > 0 && lastCharacterWasPrintable) {
       if (codeSet == CODE_CODE_C) {
         result.delete(resultLength - 2, resultLength);

core/src/com/google/zxing/oned/ITFReader.java

@@ -29,9 +29,9 @@ import java.util.Hashtable;
 /**
  * <p>Implements decoding of the ITF format.</p>
  *
- * <p>"ITF" stands for Interleaved Two of Five. This Reader will scan ITF barcode with 6, 10 or 14 digits.
+ * <p>"ITF" stands for Interleaved Two of Five. This Reader will scan ITF barcode with 6, 10 or 14
- * The checksum is optional and is not applied by this Reader. The consumer of the decoded value
+ * digits. The checksum is optional and is not applied by this Reader. The consumer of the decoded
- * will have to apply a checksum if required.</p>
+ * value will have to apply a checksum if required.</p>
  *
  * <p><a href="http://en.wikipedia.org/wiki/Interleaved_2_of_5">http://en.wikipedia.org/wiki/Interleaved_2_of_5</a>
  * is a great reference for Interleaved 2 of 5 information.</p>
@@ -126,7 +126,8 @@ public final class ITFReader extends AbstractOneDReader {
    * @param resultString {@link StringBuffer} to append decoded chars to
    * @throws ReaderException if decoding could not complete successfully
    */
-  static void decodeMiddle(BitArray row, int payloadStart, int payloadEnd, StringBuffer resultString) throws ReaderException {
+  static void decodeMiddle(BitArray row, int payloadStart, int payloadEnd,
+      StringBuffer resultString) throws ReaderException {
 
     // Digits are interleaved in pairs - 5 black lines for one digit, and the
     // 5
@@ -283,9 +284,8 @@ public final class ITFReader extends AbstractOneDReader {
    */
   static int[] findGuardPattern(BitArray row, int rowOffset, int[] pattern) throws ReaderException {
 
-    // TODO: This is very similar to implementation in AbstractUPCEANReader. Consider if they can be merged to
+    // TODO: This is very similar to implementation in AbstractUPCEANReader. Consider if they can be
-    // a single method.
+    // merged to a single method.
-
     int patternLength = pattern.length;
     int[] counters = new int[patternLength];
     int width = row.getSize();

core/src/com/google/zxing/qrcode/encoder/Encoder.java

@@ -99,12 +99,12 @@ public final class Encoder {
     encode(content, ecLevel, null, qrCode);
   }
 
-  public static void encode(String content, ErrorCorrectionLevel ecLevel, Hashtable hints, QRCode qrCode)
+  public static void encode(String content, ErrorCorrectionLevel ecLevel, Hashtable hints,
-      throws WriterException {
+      QRCode qrCode) throws WriterException {
 
-    String characterEncoding = hints == null ? null : (String) hints.get(EncodeHintType.CHARACTER_SET);
+    String encoding = hints == null ? null : (String) hints.get(EncodeHintType.CHARACTER_SET);
-    if (characterEncoding == null) {
+    if (encoding == null) {
-      characterEncoding = DEFAULT_BYTE_MODE_ENCODING;
+      encoding = DEFAULT_BYTE_MODE_ENCODING;
     }
 
     // Step 1: Choose the mode (encoding).
@@ -112,7 +112,7 @@ public final class Encoder {
 
     // Step 2: Append "bytes" into "dataBits" in appropriate encoding.
     BitVector dataBits = new BitVector();
-    appendBytes(content, mode, dataBits, characterEncoding);
+    appendBytes(content, mode, dataBits, encoding);
     // Step 3: Initialize QR code that can contain "dataBits".
     int numInputBytes = dataBits.sizeInBytes();
     initQRCode(numInputBytes, ecLevel, mode, qrCode);
@@ -121,9 +121,10 @@ public final class Encoder {
     BitVector headerAndDataBits = new BitVector();
 
     // Step 4.5: Append ECI message if applicable
+    // TODO: Why is this commented out?
     /*
-    if (mode == Mode.BYTE && !DEFAULT_BYTE_MODE_ENCODING.equals(characterEncoding)) {
+    if (mode == Mode.BYTE && !DEFAULT_BYTE_MODE_ENCODING.equals(encoding)) {
-      CharacterSetECI eci = CharacterSetECI.getCharacterSetECIByName(characterEncoding);
+      CharacterSetECI eci = CharacterSetECI.getCharacterSetECIByName(encoding);
       if (eci != null) {
         appendECI(eci, headerAndDataBits);
       }
@@ -213,10 +214,11 @@ public final class Encoder {
   }
 
   /**
-   * Initialize "qrCode" according to "numInputBytes", "ecLevel", and "mode". On success, modify "qrCode".
+   * Initialize "qrCode" according to "numInputBytes", "ecLevel", and "mode". On success,
+   * modify "qrCode".
    */
-  private static void initQRCode(int numInputBytes, ErrorCorrectionLevel ecLevel, Mode mode, QRCode qrCode)
+  private static void initQRCode(int numInputBytes, ErrorCorrectionLevel ecLevel, Mode mode,
-      throws WriterException {
+      QRCode qrCode) throws WriterException {
     qrCode.setECLevel(ecLevel);
     qrCode.setMode(mode);
 
@@ -257,11 +259,13 @@ public final class Encoder {
   static void terminateBits(int numDataBytes, BitVector bits) throws WriterException {
     int capacity = numDataBytes << 3;
     if (bits.size() > capacity) {
-      throw new WriterException("data bits cannot fit in the QR Code" + bits.size() + " > " + capacity);
+      throw new WriterException("data bits cannot fit in the QR Code" + bits.size() + " > " +
+          capacity);
     }
     // Append termination bits. See 8.4.8 of JISX0510:2004 (p.24) for details.
-    // TODO srowen says we can remove this for loop, since the 4 terminator bits are optional if the last byte
+    // TODO: srowen says we can remove this for loop, since the 4 terminator bits are optional if
-    // has less than 4 bits left. So it amounts to padding the last byte with zeroes either way.
+    // the last byte has less than 4 bits left. So it amounts to padding the last byte with zeroes
+    // either way.
     for (int i = 0; i < 4 && bits.size() < capacity; ++i) {
       bits.appendBit(0);
     }
@@ -405,8 +409,8 @@ public final class Encoder {
       }
     }
     if (numTotalBytes != result.sizeInBytes()) {  // Should be same.
-      throw new WriterException("Interleaving error: " + numTotalBytes + " and " + result.sizeInBytes() +
+      throw new WriterException("Interleaving error: " + numTotalBytes + " and " +
-        " differ.");
+          result.sizeInBytes() + " differ.");
     }
   }
 
@@ -436,7 +440,8 @@ public final class Encoder {
   /**
    * Append length info. On success, store the result in "bits".
    */
-  static void appendLengthInfo(int numLetters, int version, Mode mode, BitVector bits) throws WriterException {
+  static void appendLengthInfo(int numLetters, int version, Mode mode, BitVector bits)
+      throws WriterException {
     int numBits = mode.getCharacterCountBits(Version.getVersionForNumber(version));
     if (numLetters > ((1 << numBits) - 1)) {
       throw new WriterException(numLetters + "is bigger than" + ((1 << numBits) - 1));
@@ -447,7 +452,8 @@ public final class Encoder {
   /**
    * Append "bytes" in "mode" mode (encoding) into "bits". On success, store the result in "bits".
    */
-  static void appendBytes(String content, Mode mode, BitVector bits, String encoding) throws WriterException {
+  static void appendBytes(String content, Mode mode, BitVector bits, String encoding)
+      throws WriterException {
     if (mode.equals(Mode.NUMERIC)) {
       appendNumericBytes(content, bits);
     } else if (mode.equals(Mode.ALPHANUMERIC)) {
@@ -509,7 +515,8 @@ public final class Encoder {
     }
   }
 
-  static void append8BitBytes(String content, BitVector bits, String encoding) throws WriterException {
+  static void append8BitBytes(String content, BitVector bits, String encoding)
+      throws WriterException {
     byte[] bytes;
     try {
       bytes = content.getBytes(encoding);
@@ -549,7 +556,8 @@ public final class Encoder {
 
   static void appendECI(CharacterSetECI eci, BitVector bits) {
     bits.appendBits(Mode.ECI.getBits(), 4);
-    bits.appendBits(eci.getValue(), 8); // This is correct for values up to 127, which is all we need now
+    // This is correct for values up to 127, which is all we need now.
+    bits.appendBits(eci.getValue(), 8);
   }
 
 }

core/test/src/com/google/zxing/qrcode/decoder/DecodedBitStreamParserTestCase.java

@@ -34,7 +34,8 @@ public final class DecodedBitStreamParserTestCase extends TestCase {
     builder.write(0xF1, 8);
     builder.write(0xF2, 8);
     builder.write(0xF3, 8);
-    String result = DecodedBitStreamParser.decode(builder.toByteArray(), Version.getVersionForNumber(1)).getText();
+    String result = DecodedBitStreamParser.decode(builder.toByteArray(),
+        Version.getVersionForNumber(1)).getText();
     assertEquals("\u00f1\u00f2\u00f3", result);
   }
 
@@ -45,7 +46,8 @@ public final class DecodedBitStreamParserTestCase extends TestCase {
     builder.write(0xA1, 8);
     builder.write(0xA2, 8);
     builder.write(0xA3, 8);
-    String result = DecodedBitStreamParser.decode(builder.toByteArray(), Version.getVersionForNumber(1)).getText();
+    String result = DecodedBitStreamParser.decode(builder.toByteArray(),
+        Version.getVersionForNumber(1)).getText();
     assertEquals("\uff61\uff62\uff63", result);
   }
 
@@ -58,7 +60,8 @@ public final class DecodedBitStreamParserTestCase extends TestCase {
     builder.write(0xA1, 8);
     builder.write(0xA2, 8);
     builder.write(0xA3, 8);
-    String result = DecodedBitStreamParser.decode(builder.toByteArray(), Version.getVersionForNumber(1)).getText();
+    String result = DecodedBitStreamParser.decode(builder.toByteArray(),
+        Version.getVersionForNumber(1)).getText();
     assertEquals("\u00ed\u00f3\u00fa", result);
   }