Hey Oleg,
Le 21 févr. 2015 12:29, "Oleg Tsal-Tsalko" <oleg.tsalko_at_gmail.com> a écrit :
>
> Hi guys,
>
> I'm from Kiev JUG and we (together with Olena Syrota and Andrii Rodionov)
decided to take part in JSON-B spec efforts.
> Hopefully our involvement will be useful for our collective success.
> We've reviewed work done so far and have some comments/suggestions that
I'll send in separate email a bit later.
>
> Right now just couple of my thoughts near default mapping examples
provided by Martin Vojtek and discussion above:
> 1) Agree that we should address and discuss parametrized types usage
separately from main basic scenarios. However it might be really good idea
to pass TypeToken<T> or smith similar instead of Class<T> params to
fromJson(...) methods to pass type information in runtime as proposed in
separate email thread.
Yes but rather both instead of only typetoken, as the common scenario is
deser to a pojo.
> 2) Reg basic scenarios, I can see two diff ways how unmarshalling will be
used in general by end users:
>>
>> a) To unmarsall JSON snippet into concrete domain class (as supported by
current methods in Jsonb interface)
>>
>> b) To unmarshall JSON into generic/default objects structure. I like 2
proposed variants here:
>>>
>>> public <T> T fromJson(String str) throws JsonbException; // assume
Object.class as a type
Would be a shortcut for a unique use case not so common...but why not...
>>>
>>> public JSONObject fromJson(String str) throws JsonbException; // Not
necessary exactly JSONObject class but just an idea
Hum, it is already covered by the existing methods.
> Thus adding whole lot of specific methods like:
>>
>> public <T> List<T> fromJsonList(String str, Class<T> type) throws
JsonbException;
>>
>> public <T> Map<String, T> fromJsonMap(String str, Class<T> type) throws
JsonbException;
>>
>> public <E extends Enum<E>> EnumSet<E> fromJsonEnumSet(String str,
Class<E> type) throws JsonbException;
>>
>> public <E extends Enum<E>, T> EnumMap<E, T> fromJsonEnumMap(String str,
Class<E> enumType, Class<T> valueType) throws JsonbException;
>
> looks overcomplicated and redundant for me and I don't see much benefit
from it as for end users.
>
Ouch, missed those... I agree 100%, don't add this kind of methods.
> Thank you,
> Oleg
>
> 2015-02-18 14:26 GMT+02:00 Martin Vojtek <martin.vojtek_at_oracle.com>:
>>
>> 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>
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
>>>
>>>
>>
>