henryf1991 commented on code in PR #2887:
URL: https://github.com/apache/avro/pull/2887#discussion_r1667657527
##########
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java:
##########
@@ -802,6 +800,21 @@ private String simpleName(Class<?> c) {
return simpleName;
}
+ /*
+ * Function checks if there is @AvroTypeName annotation on the class. If
present
+ * then returns the value of the annotation else returns the package of the
+ * class
+ */
+ private String getNamespace(Class<?> c) {
+ AvroTypeName avroTypeName = c.getAnnotation(AvroTypeName.class);
+ if (avroTypeName != null) {
+ return avroTypeName.value();
+ }
+ if (c.getEnclosingClass() != null) // nested class
+ return c.getEnclosingClass().getName().replace('$', '.');
Review Comment:
Please find my comments below
1. Classes can be nested further, e.g. a class within a class within a
top-level class. Would this method need recursion? Or should only one level of
nesting be supported? - _To avoid any deviation I have kept the implementation
same as the current implementation (without the namespace annotation) which
only looks at the immediate top enclosing class for forming the namepsace._
2. Without the namespace annotation, the namespace of a top-level class is
its package, and the namespace of a nested class is package plus name of its
enclosing class. In case the enclosing class has the namespace annotation,
should its name also be added to the nested namespace in the same way? - _Thats
a valid point. Happy to change if the reviewer suggests so_
--
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]
