kbendick commented on a change in pull request #4036:
URL: https://github.com/apache/iceberg/pull/4036#discussion_r799852098
##########
File path: core/src/main/java/org/apache/iceberg/util/JsonUtil.java
##########
@@ -134,6 +146,20 @@ public static String getStringOrNull(String property,
JsonNode node) {
.build();
}
+ public static void writeIntegerIf(boolean condition, String key, Integer
value, JsonGenerator generator)
+ throws IOException {
+ if (condition) {
+ generator.writeNumberField(key, value);
+ }
+ }
Review comment:
Nit:
When I see the name of this function in the code as it's used currently,
it's unclear if the field is written with a null value or simply skipped (it's
skipped). But when I see it used in code, like the following I'm unsure if I'm
going to get `{ "max-snapshot-age-ms": null }` or `{ }`.
```java
JsonUtil.writeLongIf(ref.maxSnapshotAgeMs() != null, "max-snapshot-age-ms",
ref.maxSnapshotAgeMs(), generator);
```
Maybe naming it `writeIntegerFieldIf` would help? Having `Field` in the name
signifies to me that we won't be writing the field if the condition doesn't
evaluate to true.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
