mkeskells commented on code in PR #3307:
URL: https://github.com/apache/avro/pull/3307#discussion_r1948748755


##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java:
##########
@@ -1515,34 +1515,45 @@ else if (value instanceof Utf8) {
 
   }
 
-  /*
+  /**
    * Called to create new array instances. Subclasses may override to use a
-   * different array implementation. By default, this returns a {@link
-   * GenericData.Array}.
+   * different array implementation. By default, this returns a
+   * {@link GenericData.Array}.
+   * 
+   * @param old    the old array instance to reuse, if possible. If the old 
array
+   *               is an appropriate type, it may be cleared and returned.
+   * @param size   the size of the array to create.
+   * @param schema the schema of the array elements.

Review Comment:
   Note to reviewers
   I have changed the spec of this method, specifically to 
   - check the schema of the returned `GenericContainer` (if one is returned)
   
   I am unsure of what other `Collection`s may be passed. Should a set be 
appropriate. What if the collection had different semantics? (this however is 
existing behaviour and unchanged by this PR)
   
   Comparison of the schema is via `==` not `.equals` as I thought .equals 
would be potentially expensive, and inappropriate to have in a performance 
optimisation.
   I appreciate that this is very opinionated, and would welcome comments from 
developer who know Avro better that I do



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to