EndzeitBegins commented on code in PR #9151:
URL: https://github.com/apache/nifi/pull/9151#discussion_r1705799043
##########
nifi-extension-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/bigquery/proto/ProtoUtils.java:
##########
@@ -46,7 +48,21 @@ public static DynamicMessage
createMessage(Descriptors.Descriptor descriptor, Ma
switch (field.getType()) {
case MESSAGE:
if (field.isRepeated()) {
- Collection collection = value.getClass().isArray() ?
Arrays.asList((Object[]) value) : (Collection) value;
+ Collection collection = null;
Review Comment:
As all possible code paths provide a value for this, it can be declared
`final` and need not to be initialised.
And while you didn't choose the name `collection` here I'd like to use this
change as an opportunity to provide a more meaningful name for this variable.
All code paths should lead to a collection of `Map<String, Object>` objects,
thus we may as well define the type here instead of using a raw collection.
```suggestion
final Collection<Map<String, Object>> valueMaps;
```
##########
nifi-extension-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/bigquery/proto/ProtoUtils.java:
##########
@@ -46,7 +48,21 @@ public static DynamicMessage
createMessage(Descriptors.Descriptor descriptor, Ma
switch (field.getType()) {
case MESSAGE:
if (field.isRepeated()) {
- Collection collection = value.getClass().isArray() ?
Arrays.asList((Object[]) value) : (Collection) value;
+ Collection collection = null;
+ if (value.getClass().isArray()) {
+ collection = Arrays.asList((Object[]) value);
+ } else if (value instanceof HashMap) {
+ collection = new ArrayList();
+ for (Object entryObj : ((HashMap<?, ?>)
value).entrySet()) {
+ Map.Entry<?, ?> entry = (Map.Entry<?, ?>) entryObj;
+ Map<String, Object> map = new HashMap<>();
+ map.put("key", entry.getKey());
+ map.put("value", entry.getValue());
+ collection.add(map);
+ }
+ } else {
+ collection = (Collection) value;
+ }
collection.forEach(act -> builder.addRepeatedField(field,
createMessage(field.getMessageType(), (Map<String, Object>) act, tableSchema)));
Review Comment:
As I've changed the type of the former `collection` field, the unchecked
cast must be altered slightly.
However, we can drop the cast inside the `foreach` this way.
```suggestion
} else {
valueMaps = (Collection<Map<String, Object>>) value;
}
valueMaps.forEach(act -> builder.addRepeatedField(field,
createMessage(field.getMessageType(), act, tableSchema)));
```
##########
nifi-extension-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/bigquery/proto/ProtoUtils.java:
##########
@@ -46,7 +48,21 @@ public static DynamicMessage
createMessage(Descriptors.Descriptor descriptor, Ma
switch (field.getType()) {
case MESSAGE:
if (field.isRepeated()) {
- Collection collection = value.getClass().isArray() ?
Arrays.asList((Object[]) value) : (Collection) value;
+ Collection collection = null;
+ if (value.getClass().isArray()) {
+ collection = Arrays.asList((Object[]) value);
Review Comment:
The original code casts all types of array into an `Object[]`. This only
works for arrays with non-primitive items and thus the explicit `isArray` check
can be replaced with a simpler (and more useful) `instanceof`.
This way we can use pattern-matching available in more modern Java and avoid
an explicit additional cast.
```suggestion +1
if (value instanceof Object[] arrayValue) {
valueMaps = Arrays.stream(arrayValue)
.map(item -> (Map<String, Object>)
item).toList();
```
--
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]
