Initial commit for json serialization #24
Conversation
|
|
||
| public String toJson() { | ||
| return JsonSerializer.getInstance().toJson(getEventName(), attributes); | ||
| } |
There was a problem hiding this comment.
I don't think the changes here are correct. I think that instead of keeping around another structure, you should instead just convert in this function. It may require deserializing and then converting to json. @pfarner may have a similar opinion, but hopefully he'll see this.
There was a problem hiding this comment.
I removed the extra structure i was using to hold attributes.
|
I'm not a JSON expert, so I'm hoping that there's a good reason to have three different JSON formats. I get why one might have typed and untyped, but how does the default one differ from the typed one? I see that a strings have escaped quotes placed inside their values, but only in some formats. If we have a simple event containing only a string field, here are the JSON formats: X { enc = 1; string = contents; } default JSON (with two copies of the data, one with extra double-quotes, the other without): typed (with extra double-quotes): untyped (no extra double-quotes): It seems like none of these should have the escaped double quotes within their double-quoted values. |
|
It gets even weirder if I use single- or double-quote characters within the string value: Event: default JSON: {"name":"X","typed":{"string":{"type":"string","value":""\u0027\"""},"enc":{"type":"int16","value":"1"}}} {"name":"X","attributes":{"EventName":"X","string":"\u0027"","enc":1}} The single quote is converted to unicode, and the rest of the string contents (particularly the double-quote) are getting escaped twice. I can't imagine that this is by design. |
|
We should also add a "version":1, outside the attributes section, so we can change the format later. |
|
Preston, |
|
@pfarner any further comments on Vikram's changes? Otherwise, I'd like to but a bow on this request, and get it merged, and hopefully pushed to maven central. |
| case UINT32: | ||
| case UINT64: | ||
| case STRING: | ||
| return typeObject.toString(); |
There was a problem hiding this comment.
Minor, but this is an unnecessary line; it could just fall through to the next case.
No description provided.