Sounds reasonable. Lets see what other thoughts if any.
2015-02-22 21:45 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
> Yeah I get your point, and thats why I first was ok with that. But after
> thinking of it again, I am not sure that those methods help.
>
> 1) Having shortcuts to deser to DOM structures. Yes the user code might be
> a bit shorter, but we will need to add shortcut for all possible root
> values (with last rfc update litteral values are allowed as root), all with
> different names. In the end it might just get more cumbersome, even for the
> end user.
> 2) Having a method that does a unchecked cast and does the deser based on
> the json content (or some other parameters that are not being passed during
> invokation by the user). Or the user knows the type that he expects, then
> he can just pass it, or he doesn't and in that case he expects an Object.
> There is also the case where he expects some root type, but knows that the
> concrete type might be a sub class, in that case it does not help either
> (as fromJson(json, class) does it). Thus I don't think this method should
> be added. But maybe I miss some use case where it would help?
>
> Cheers,
> Eugen
>
> 2015-02-22 11:15 GMT-08:00 Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>:
>
> Hi Eugen,
>>
>> Hmm. Actually you are right that in both cases we can reuse existing
>> methods to get same effect.
>> Also the same approach has been taken in Gson lib, however I thought to
>> have at least one shortcut method that will deserialize JSON into some
>> generic data structure / object might be convenient...
>>
>> Thank you,
>> Oleg
>>
>> 2015-02-22 20:19 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>>
>>> It can be achieved with jsonb.fromJson(json, JsonObject.class).
>>>
>>> About "Would be a shortcut for a unique use case not so common...but why
>>> not... " I changed my mind :P adding this method is not useful. We see T in
>>> your example, so the user knows what type he wants, but then why don't pass
>>> it to fromJson.
>>> Le 22 févr. 2015 00:35, "Oleg Tsal-Tsalko" <oleg.tsalko_at_gmail.com> a
>>> écrit :
>>>
>>> Hi Eugen,
>>>>
>>>> What do you mean that below already covered by existing methods?
>>>>
>>>> >>> public JSONObject fromJson(String str) throws JsonbException; //
>>>> Not necessary exactly JSONObject class but just an idea
>>>>
>>>> 2015-02-21 23:10 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>>>>
>>>>> 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
>>>>> >>>
>>>>> >>>
>>>>> >>
>>>>> >
>>>>>
>>>>
>>>>
>>
>