Hi Kevin,

both the test cases and the new deserializer classes look good. I just pushed 
all your changes into the apache branch: 
https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/jsonMetadata

Also thanks for applying the feedback I gave earlier :) 

Best Regards,
Christian

-----Original Message-----
From: Kevin Ratnasekera [mailto:djkevincr1...@gmail.com] 
Sent: Sonntag, 26. Juli 2015 18:49
To: Amend, Christian
Cc: dev@olingo.apache.org
Subject: Re: Current Status

Hi Christian,
I have made changes according to your feedback regarding actions and
functions. I have new test cases for these. I have added following
deserializers with new commits, singleton , action imports, function
imports, navigational property bindings, and Type definitions. Please
see relevant commits for those. Only left with properties, navigation
properties and complex type deserializers left to be done. It will be
completed in coming days. I will add test cases for what already done.
Regards
Kevin

On 7/23/15, Amend, Christian <christian.am...@sap.com> wrote:
> Hi Kevin,
>
> I saw your commits in your github repository. The code looks good but there
> are some small changes I would make:
>
>
> 1.       fullqualifiedName.contains(“Collection”) is not the correct way to
> check if a type is a collection. To be on the safe side you need to ask:
> fullqualifiedName.startsWith(“Collection(”) This is the only way to be sure
> that the type you have is a collection as any name can contain “Collection”
> somewhere. If you want to be completely compliant you would even have to ask
> if fullqualifiedName.endsWith(“)”) but I think this is too much and the
> startsWith is enough of a marker.
>
> 2.       Collection.length() is called multiple times. You could save the
> result in a variable or just use the integer number. Although I like the
> fact that using the length method specifically shows what you are doing. So
> I would save it in  a constant. Also I would use “Collection(“.lenght()
> since the “(“ character is part of the definition.
>
> 3.       The statement fullqualifiedName.replace(schemaAlias,
> schemaNamespace) is not necessary. Just parse the document as is. The
> association between alias and namespace is done in the ClientCsdlEdmProvider
> getAliasInfo. There we build a mapping which is later used to resolve the
> type.
>
> I have also created a diff file and will push your changes to our Apache
> repository:
> https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/jsonMetadata
>
> If you have any questions let me know ☺
>
> Best Regards,
> Christian
>
> From: Kevin Ratnasekera [mailto:djkevincr1...@gmail.com]
> Sent: Dienstag, 21. Juli 2015 06:13
> To: Amend, Christian; Christian Amend
> Subject: Current Status
>
> Hi Christian,
> From this point onwards, I will follow the same approach as I did, If you
> are unhappy with anything please let me know. I have tried several
> alternative changes, but basically all follows the same approach either
> token or building the tree. I have added a very small commit to the make
> changes related to remove the FullQualifiedName logic. I have already worked
> out Actions and Functions once tested I will push the changes.
> Regards
> Kevin
>

Reply via email to