Uploaded image for project: 'Apache Avro'
  1. Apache Avro
  2. AVRO-1680

Problems with code snippet in Decoder#readMapStart javadocs

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Trivial
    • Resolution: Unresolved
    • Affects Version/s: 1.7.7
    • Fix Version/s: None
    • Component/s: doc, java
    • Labels:

      Description

      https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/Decoder.java javadocs ( http://avro.apache.org/docs/1.7.7/api/java/org/apache/avro/io/Decoder.html#readMapStart() ) say:

          Map m = new HashMap();
          Record reuse = new Record();
          for(long i = in.readMapStart(); i != 0; i = in.readMapNext()) {
            for (long j = 0; j < i; j++) {
              String key = in.readString();
              reuse.intField = in.readInt();
              reuse.boolField = in.readBoolean();
              m.put(key, reuse);
            }
         }
      

      This can be improved in two ways:
      1) Javadoc ate the generic arguments. This can be fixed by wrapping into {@code}.
      2) The mutable record object is being reused; as a result, the map will have the same shared object mapped to every key. I don't think this is likely to be the user's intention, so a new Record should be created on every iteration.

      Actually in 3 ways.
      3) a much better name for "i" would be "numRecords", because otherwise it seems like "j" and "i" have similar roles (indices over some containers), which they don't - "i" is not an index into any container, only j is. Then "j" can be renamed to "i".

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              kirpichov Eugene Kirpichov
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated: