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


##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java:
##########
@@ -1515,36 +1516,39 @@ 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.
    */
   public Object newArray(Object old, int size, Schema schema) {
-    if (old instanceof GenericArray) {
-      ((GenericArray<?>) old).reset();
-      return old;
-    } else if (old instanceof Collection) {
-      ((Collection<?>) old).clear();
-      return old;
-    } else {
-      if (schema.getElementType().getType() == Type.INT) {
-        return new PrimitivesArrays.IntArray(size, schema);
-      }
-      if (schema.getElementType().getType() == Type.BOOLEAN) {
-        return new PrimitivesArrays.BooleanArray(size, schema);
-      }
-      if (schema.getElementType().getType() == Type.LONG) {
-        return new PrimitivesArrays.LongArray(size, schema);
-      }
-      if (schema.getElementType().getType() == Type.FLOAT) {
-        return new PrimitivesArrays.FloatArray(size, schema);
-      }
-      if (schema.getElementType().getType() == Type.DOUBLE) {
-        return new PrimitivesArrays.DoubleArray(size, schema);
+    final var logicalType = schema.getElementType().getLogicalType();
+    final var conversion = getConversionFor(logicalType);
+    final var optimalValueType = PrimitivesArrays.optimalValueType(schema, 
logicalType,
+        conversion == null ? null : conversion.getConvertedType());

Review Comment:
   Nice implementation, but IMHO there are two issues with it:
   1. the determination of the logical type and conversion and belongs inside 
as they're not used elsewhere
   2. it's used only here, but located in the class `PrimitiveArrays`
   
   Let's move the method `optimalValueType` here.
   
   It's a bit of a tricky choice, as determining an array element type is 
coupled with both the schema and the arrays, but IMHO it belongs more with the 
former: reading data into a type also happens for maps and record properties.



-- 
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