todmorrison commented on a change in pull request #6629:
URL: https://github.com/apache/geode/pull/6629#discussion_r657308067
##########
File path:
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
##########
@@ -216,7 +216,8 @@ public static void writeArrayAsJson(JsonGenerator
generator, Object value, Strin
writeObjectArrayAsJson(generator, (Object[]) value, pdxField);
} else {
throw new IllegalStateException(
- "PdxInstance returns unknwon pdxfield " + pdxField + " for type " +
value);
+ "PdxInstance returns unsupported type " + value.getClass()
Review comment:
I am okay with just saying "for field null" if the field doesn't have a
name. I've changed the message to:
```
} else if (value.getClass().isArray()) {
throw new IllegalStateException(
"The pdx field " + pdxField + " is an array whose component type "
+ value.getClass().getComponentType()
+ " can not be converted to JSON.");
} else {
throw new IllegalStateException(
"Expected an array for pdx field " + pdxField + ", but got an
object of type "
+ value.getClass());
}
```
Does that work?
As for the last "else if" on line 215, I agree it could be improved
(similarly GEODE-9392 suggests there may be room to improve
PdxToJSON.writeValue as well). In fact, using similar reasoning, I wonder if
writeArrayAsJson couldn't be simplified to just loop and delegate and eliminate
almost all of the conditionals? However, I'm trying to limit the scope here to
making the existing messages more useful unless someone wants to discuss doing
the full overhaul with me.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
