users@jsonb-spec.java.net

[jsonb-spec users] [jsr367-experts] Re: [2-DefaultMapping] Proposal

From: Martin Vojtek <martin.vojtek_at_oracle.com>
Date: Wed, 18 Feb 2015 13:26:36 +0100

Hi Przemyslaw,

thank you very much for going through the examples.

comments below.

Cheers,
Martin

> On 17 Feb 2015, at 20:40, Przemyslaw Bielicki <pbielicki_at_gmail.com> wrote:
>
> 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 :).
>

according to JUnit, I am not sure if it is correct to define standard in terms of non-standard tools. I will also participate in reference implementation and JUnit is the most appropriate choice for testing framework. These examples serve the purpose of discussion ground, and maybe will not be part of the published standard, so this rule could be relaxed. I need to think about it a bit.

> 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?
>

The reason behind Object.class is that I want to show support for Object.class. The alternative is to throw some Exception for this case. There is also use case, when I want to allow user to specify Object.class, and unmarshal whatever valid json. If this json is list, the result will be Collection (specifically ArrayList of Objects), or Map (specifically LinkedHashMap<String, Object>).

> 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 this case, getClass() returns concrete class (e.g. RegularEnumSet). Reflection deals with class files, so the result is EnumSet.

Intention of this code was to try to not to deal with generics, but to show I want to provide generic type. Unfortunately use of reflection in this case is also wrong, because the result will be EnumSet<E>.

I can put there some not supported hack like this, but the discussion about generics should be in separate thread:

ParameterizedType parameterizedType = (ParameterizedType)new HashSet<EnumSet<Language>>() {}.getClass().getGenericSuperclass();
EnumSet<Language> languageEnumSet = (EnumSet<Language>)jsonb.fromJson("[\"Slovak\", \"English\"]", parameterizedType.getActualTypeArguments()[0]);

Maybe the best way is to put there getClass() proposed by you. (and not deal with generics in default mapping examples).

> 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?
>

It’s a typo, should be

Dog animal = jsonb.fromJson("{\"age\":5, \"name\":\"Rex\"}}", Dog.class);


> 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 <mailto: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
>