gemini-code-assist[bot] commented on code in PR #36839:
URL: https://github.com/apache/beam/pull/36839#discussion_r2535331609
##########
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/schemas/utils/AvroUtils.java:
##########
@@ -1214,6 +1214,16 @@ private static org.apache.avro.Schema getFieldSchema(
return fieldType.getNullable() ? ReflectData.makeNullable(baseType) :
baseType;
}
+ private static final org.apache.avro.Schema INT_AVRO_TYPE =
+ org.apache.avro.Schema.create(Type.INT);
+ private static final org.apache.avro.Schema LONG_AVRO_TYPE =
+ org.apache.avro.Schema.create(Type.LONG);
+ private static final org.apache.avro.Schema FLOAT_AVRO_TYPE =
+ org.apache.avro.Schema.create(Type.FLOAT);
+ private static final org.apache.avro.Schema DOUBLE_AVRO_TYPE =
+ org.apache.avro.Schema.create(Type.DOUBLE);
Review Comment:
For improved maintainability and to reduce boilerplate, consider replacing
these individual constants with a single static `Map` that maps Avro schemas to
their corresponding numeric conversion functions. This would centralize the
logic for numeric conversions and make it more extensible. You will need to
import `java.util.function.Function`.
```suggestion
private static final java.util.Map<org.apache.avro.Schema,
java.util.function.Function<Number, ? extends Number>> NUMERIC_CONVERTERS =
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableMap.of(
org.apache.avro.Schema.create(Type.INT), Number::intValue,
org.apache.avro.Schema.create(Type.LONG), Number::longValue,
org.apache.avro.Schema.create(Type.FLOAT), Number::floatValue,
org.apache.avro.Schema.create(Type.DOUBLE), Number::doubleValue);
```
##########
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/schemas/utils/AvroUtils.java:
##########
@@ -1230,6 +1240,18 @@ private static org.apache.avro.Schema getFieldSchema(
return value;
}
+ // Handle implicit up-cast: use avroSchema
+ if (INT_AVRO_TYPE.equals(typeWithNullability.type)) {
+ return ((Number) value).intValue();
+ } else if (LONG_AVRO_TYPE.equals(typeWithNullability.type)) {
+ return ((Number) value).longValue();
+ } else if (FLOAT_AVRO_TYPE.equals(typeWithNullability.type)) {
+ return ((Number) value).floatValue();
+ } else if (DOUBLE_AVRO_TYPE.equals(typeWithNullability.type)) {
+ return ((Number) value).doubleValue();
+ }
Review Comment:
This `if-else if` chain can be simplified by using the `NUMERIC_CONVERTERS`
map (as suggested in another comment). This makes the code more concise and
easier to extend with new numeric types in the future.
```java
if (NUMERIC_CONVERTERS.containsKey(typeWithNullability.type)) {
return NUMERIC_CONVERTERS.get(typeWithNullability.type).apply((Number)
value);
}
```
--
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]
