tilgalas commented on code in PR #32081:
URL: https://github.com/apache/beam/pull/32081#discussion_r1816516273
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/values/RowWithGetters.java:
##########
@@ -42,45 +42,43 @@
* the appropriate fields from the POJO.
*/
@SuppressWarnings("rawtypes")
-public class RowWithGetters extends Row {
- private final Object getterTarget;
- private final List<FieldValueGetter> getters;
+public class RowWithGetters<T extends @NonNull Object> extends Row {
+ private final T getterTarget;
+ private final List<FieldValueGetter<T, ?>> getters;
private @Nullable Map<Integer, @Nullable Object> cache = null;
RowWithGetters(
- Schema schema, Factory<List<FieldValueGetter>> getterFactory, Object
getterTarget) {
+ Schema schema, Factory<List<FieldValueGetter<T, ?>>> getterFactory, T
getterTarget) {
super(schema);
this.getterTarget = getterTarget;
this.getters =
getterFactory.create(TypeDescriptor.of(getterTarget.getClass()), schema);
}
@Override
@SuppressWarnings({"TypeParameterUnusedInFormals", "unchecked"})
- public <T> @Nullable T getValue(int fieldIdx) {
+ public @Nullable Object getValue(int fieldIdx) {
Review Comment:
I guess it's probably a matter of taste - my view is that this class can't
possibly know what type of value a particular getter is going to return, so
it's not commiting to anything by making the method generic - "I only know it's
an Object, if you need a specific type, you'll have to cast it yourself" - I
should probably also change the type of the getters list to
`List<FieldValueGetter<T, Object>>` but then it would probably open another can
of worms of covariant types, that is whether a, say, `FieldValueGetter<T,
String>` is a `FieldValueGetter<T, Object>` - the current type of
`List<FieldValueGetter<T, ?>>` is slightly incorrect as the `?` wildcard means
an unknown type, but which is at least shared by all members of the list (which
is not true)
I can understand the approach where the caller requests a specific type via
a generic type parameter and is happy to accept the method throwing a
ClassCastException if they had wrong assumptions about the getter - let me know
if you want me to make it back a generic method
--
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]
