[GitHub] [tinkerpop] amatiushkin commented on a change in pull request #1485: TINKERPOP-848 draft implementation to support default value

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

amatiushkin commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r727717241



##########
File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
##########
@@ -138,9 +143,11 @@ public void readGraph(final InputStream graphInputStream, final Graph graphToWri
                         case GraphMLTokens.DATA:
                             final String key = reader.getAttributeValue(null, GraphMLTokens.KEY);
                             final String dataAttributeName = keyIdMap.get(key);
+                            final String defaultValue = defaultValues.get(key);
 
                             if (dataAttributeName != null) {
-                                final String value = reader.getElementText();
+                                String elementValue =  reader.getElementText();
+                                final String value = elementValue.length() == 0 && defaultValue.length() != 0 ? defaultValue : elementValue;

Review comment:
       `getElementText()` internally creates a `new StringBuilder()` per every call and appends text (`getText()`) of a given element. Attempt to use non-text elements would through various exceptions. I have check multiple implementations of this method, and it seems concrete classes are null-safe.




-- 
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: commits-unsubscribe@tinkerpop.apache.org

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