jsr367-experts@jsonb-spec.java.net

[jsr367-experts] Re: [2-DefaultMapping] Proposal

From: Przemyslaw Bielicki <pbielicki_at_gmail.com>
Date: Tue, 17 Feb 2015 20:40:52 +0100

Hi Martin,

thanks for sharing this.

First comment is the same I made to MartiNG: I would appreciate your test
in a form of JUnit test. You put a lot of effort in this and you will have
to rewrite / refactor a lot of code to make it unit test after all. Yes, I
know it should not and does not run but I just don't see the value of
main() method instead of proper JUnit class(es) - (you even implemented
your own assertEquals method - don't want to be impolite but it just does
not make sense :).

I don't understand these lines:

Map<String, Object> map =
(LinkedHashMap<String,Object>)jsonb.fromJson("{\"name\":\"unknown
object\"}", Object.class);

Collection<Object> collection =
(ArrayList<Object>)jsonb.fromJson("[{\"value\":\"first\"},
{\"value\":\"second\"}]", Object.class);

Why did you use Object.class parameter instead of Map.class and
Collection.class accordingly?

This is kind of verbose (IMO unnecessarily):

EnumSet<Language> languageEnumSet1 =
(EnumSet<Language>)jsonb.fromJson("[\"Slovak\", \"English\"]",
DefaultMapping.class.getField("languageEnumSet").getType());

EnumMap<Language, String> languageEnumMap1 = (EnumMap<Language,
String>)jsonb.fromJson("[\"Slovak\" : \"sk\", \"Czech\" : \"cz\"]",
DefaultMapping.class.getField("languageEnumMap").getType());

I would change it to:

EnumSet<Language> languageEnumSet1 = jsonb.fromJson("[\"Slovak\",
\"English\"]", languageEnumSet.getClass());
EnumMap<Language, String> languageEnumMap1 = (EnumMap<Language,
String>)jsonb.fromJson("[\"Slovak\" : \"sk\", \"Czech\" : \"cz\"]",
languageEnumMap.getClass());

this also requires adding static keyword to languageEnumSet and
languageEnumMap fields.

In the inheritance test the input and output JSONs are not the same:
{"age":5, "dog":{"name":"Rex"}}
vs.
{"age":5,"name":"Rex"}

it's a typo?

Expect few more comments from my side. I'm short in time this week, thus I
need to split my email into smaller pieces.

Cheers,
Przemyslaw

On Fri, Feb 13, 2015 at 8:26 PM, Martin Vojtek <martin.vojtek_at_oracle.com>
wrote:

> Hi,
>
> I have pushed new branch default_mapping, which contains initial examples
> for default mapping.
>
> There are also comments, which contain information about concepts like
> null handling and default field access strategy.
>
> Default mapping of dates and generics handling is not part of this commit
> and will be addressed separately.
>
> Looking forward to your feedback.
>
> Thank you,
> Martin Vojtek