[GitHub] [avro] opwvhk commented on a diff in pull request #1688: AVRO-3374: special cases for qualified name

[GitBox <gi...@apache.org>]

opwvhk commented on code in PR #1688:
URL: https://github.com/apache/avro/pull/1688#discussion_r899055830


##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -751,8 +751,32 @@ public void writeName(Names names, JsonGenerator gen) throws IOException {
     }
 
     public String getQualified(String defaultSpace) {
-      return (space == null || space.equals(defaultSpace)) ? name : full;
+      return this.shouldWriteFull(defaultSpace) ? full : name;
     }
+
+    /**
+     * Determine if full name must be written. There are 2 cases for true :
+     * defaultSpace != from this.space or name is already a Schema.Type (int, array
+     * ...)
+     *
+     * @param defaultSpace : default name space.
+     * @return true if full name must be written.
+     */
+    private boolean shouldWriteFull(String defaultSpace) {
+      if (space != null && space.equals(defaultSpace)) {
+        try {
+          // name is a 'Type', so namespace must be written (int should return true, Int
+          // should return false).
+          return Type.valueOf(this.name.toUpperCase(Locale.ENGLISH)).name.equals(this.name);
+        } catch (IllegalArgumentException ex) {
+          // namespace can be omitted, (default & name is not a type)
+          return false;
+        }

Review Comment:
   This code catches the exception as flow control, which is quite expensive. I think we should prefer a comparison loop instead:
   ```
         for (Type type : Type.values()) {
           if (type.name.equals(name)) {
             return true;
           }
         }
         return false;
   ```



-- 
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: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org