You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@chemistry.apache.org by Florent Guillaume <fg...@nuxeo.com> on 2010/09/09 14:54:08 UTC
various small cleanups
Hi,
A few things:
1.
Currently our iterators (CollectionIterator, CollectionPageIterator)
have a next() method that returns null at the end of the iteration.
This is wrong and contrary to the Iterator contract, they should
return NoSuchElementException in that case.
Anyone opposed to fixing this?
2.
Also I'd like to do small renamings like AbstractPageFetch ->
AbstractPageFetcher and PageFetchResult -> FetchedPage or maybe even
Page, and also rename getPage to getItems (or getList).
3.
ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
that just modify the iterable in place (for instance the JCR
RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
and TermDocs.skipTo), and in the use cases I've seen it's no use
creating a new object. Is it ok to change this?
4.
PageFetchResult has:
public BigInteger getTotalItems()
public Boolean getHasMoreItems()
whereas ItemIterable has:
long getTotalNumItems();
boolean getHasMoreItems();
I guess we have to keep Boolean and BigInteger to reflect the domain
model here, but I'll sync the naming.
Comments?
Florent
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
Re: various small cleanups
Posted by David Caruana <da...@alfresco.com>.
> When you do ItemIterable.getPage() you get an iterable for the current
> page. Is it the case that this iterable's iterator is intended to
> iterate only over the page as it was defined at the time of its
> creation, and that the underlying iterable (CollectionPageIterable) is
> never supposed to stray beyond the bounds of this page? That would
> make most sense to me as I don't see a use case where you'd want to
> "escape" from the page. Only the main ItemIterable can give you new
> pages. Can someone confirm that?
Yes, your understanding is correct.
Dave
On 7 Oct 2010, at 16:09, Florent Guillaume wrote:
> Hi,
>
> I fixed most of this except the next() methods and the in-place skipTo().
> http://svn.apache.org/viewvc?view=revision&revision=1005481
>
> Before fixing the next() I want to make sure I understand the intended
> use of CollectionIterable vs CollectionPageIterable.
>
> When you do ItemIterable.getPage() you get an iterable for the current
> page. Is it the case that this iterable's iterator is intended to
> iterate only over the page as it was defined at the time of its
> creation, and that the underlying iterable (CollectionPageIterable) is
> never supposed to stray beyond the bounds of this page? That would
> make most sense to me as I don't see a use case where you'd want to
> "escape" from the page. Only the main ItemIterable can give you new
> pages. Can someone confirm that?
>
> Florent
>
>
> On Thu, Sep 9, 2010 at 2:54 PM, Florent Guillaume <fg...@nuxeo.com> wrote:
>> Hi,
>>
>> A few things:
>>
>> 1.
>> Currently our iterators (CollectionIterator, CollectionPageIterator)
>> have a next() method that returns null at the end of the iteration.
>> This is wrong and contrary to the Iterator contract, they should
>> return NoSuchElementException in that case.
>> Anyone opposed to fixing this?
>>
>> 2.
>> Also I'd like to do small renamings like AbstractPageFetch ->
>> AbstractPageFetcher and PageFetchResult -> FetchedPage or maybe even
>> Page, and also rename getPage to getItems (or getList).
>>
>> 3.
>> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
>> that just modify the iterable in place (for instance the JCR
>> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
>> and TermDocs.skipTo), and in the use cases I've seen it's no use
>> creating a new object. Is it ok to change this?
>>
>> 4.
>> PageFetchResult has:
>> public BigInteger getTotalItems()
>> public Boolean getHasMoreItems()
>> whereas ItemIterable has:
>> long getTotalNumItems();
>> boolean getHasMoreItems();
>> I guess we have to keep Boolean and BigInteger to reflect the domain
>> model here, but I'll sync the naming.
>>
>> Comments?
>>
>> Florent
>>
>> --
>> Florent Guillaume, Director of R&D, Nuxeo
>> Open Source, Java EE based, Enterprise Content Management (ECM)
>> http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
>>
>
>
>
> --
> Florent Guillaume, Director of R&D, Nuxeo
> Open Source, Java EE based, Enterprise Content Management (ECM)
> http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
Re: various small cleanups
Posted by Florent Guillaume <fg...@nuxeo.com>.
Hi,
I fixed most of this except the next() methods and the in-place skipTo().
http://svn.apache.org/viewvc?view=revision&revision=1005481
Before fixing the next() I want to make sure I understand the intended
use of CollectionIterable vs CollectionPageIterable.
When you do ItemIterable.getPage() you get an iterable for the current
page. Is it the case that this iterable's iterator is intended to
iterate only over the page as it was defined at the time of its
creation, and that the underlying iterable (CollectionPageIterable) is
never supposed to stray beyond the bounds of this page? That would
make most sense to me as I don't see a use case where you'd want to
"escape" from the page. Only the main ItemIterable can give you new
pages. Can someone confirm that?
Florent
On Thu, Sep 9, 2010 at 2:54 PM, Florent Guillaume <fg...@nuxeo.com> wrote:
> Hi,
>
> A few things:
>
> 1.
> Currently our iterators (CollectionIterator, CollectionPageIterator)
> have a next() method that returns null at the end of the iteration.
> This is wrong and contrary to the Iterator contract, they should
> return NoSuchElementException in that case.
> Anyone opposed to fixing this?
>
> 2.
> Also I'd like to do small renamings like AbstractPageFetch ->
> AbstractPageFetcher and PageFetchResult -> FetchedPage or maybe even
> Page, and also rename getPage to getItems (or getList).
>
> 3.
> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
> that just modify the iterable in place (for instance the JCR
> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
> and TermDocs.skipTo), and in the use cases I've seen it's no use
> creating a new object. Is it ok to change this?
>
> 4.
> PageFetchResult has:
> public BigInteger getTotalItems()
> public Boolean getHasMoreItems()
> whereas ItemIterable has:
> long getTotalNumItems();
> boolean getHasMoreItems();
> I guess we have to keep Boolean and BigInteger to reflect the domain
> model here, but I'll sync the naming.
>
> Comments?
>
> Florent
>
> --
> Florent Guillaume, Director of R&D, Nuxeo
> Open Source, Java EE based, Enterprise Content Management (ECM)
> http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
>
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
Re: various small cleanups
Posted by David Caruana <da...@alfresco.com>.
+1
Dave
On 9 Sep 2010, at 15:47, Klevenz, Stephan wrote:
> ok, then I don't see an issue with that.
>
> +1
>
> Stephan
>
> -----Original Message-----
> From: Florent Guillaume [mailto:fg@nuxeo.com]
> Sent: Donnerstag, 9. September 2010 16:24
> To: chemistry-dev@incubator.apache.org
> Subject: Re: various small cleanups
>
> On Thu, Sep 9, 2010 at 3:52 PM, Klevenz, Stephan
> <st...@sap.com> wrote:
>> Florent wrote:
>>> 3.
>>> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
>>> that just modify the iterable in place (for instance the JCR
>>> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
>>> and TermDocs.skipTo), and in the use cases I've seen it's no use
>>> creating a new object. Is it ok to change this?
>>
>> to 3. I'm not sure if I understand correctly. The reason to return an iterable is to allow dotted expressions to have some "from-to" semantic of a range: ItemIterable<CmisObject> i = folder.getChildren().skipTo(2).getPage(3);
>
> Returning an object for chained expressions is all right, but what I
> want to ensure is that the contract of the method allows it to return
> "this" after modifying its internal state, and to change the
> implementation to that effect to avoid constructing new objects every
> time.
>
> Florent
>
> --
> Florent Guillaume, Director of R&D, Nuxeo
> Open Source, Java EE based, Enterprise Content Management (ECM)
> http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
RE: various small cleanups
Posted by "Klevenz, Stephan" <st...@sap.com>.
ok, then I don't see an issue with that.
+1
Stephan
-----Original Message-----
From: Florent Guillaume [mailto:fg@nuxeo.com]
Sent: Donnerstag, 9. September 2010 16:24
To: chemistry-dev@incubator.apache.org
Subject: Re: various small cleanups
On Thu, Sep 9, 2010 at 3:52 PM, Klevenz, Stephan
<st...@sap.com> wrote:
>Florent wrote:
>> 3.
>> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
>> that just modify the iterable in place (for instance the JCR
>> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
>> and TermDocs.skipTo), and in the use cases I've seen it's no use
>> creating a new object. Is it ok to change this?
>
> to 3. I'm not sure if I understand correctly. The reason to return an iterable is to allow dotted expressions to have some "from-to" semantic of a range: ItemIterable<CmisObject> i = folder.getChildren().skipTo(2).getPage(3);
Returning an object for chained expressions is all right, but what I
want to ensure is that the contract of the method allows it to return
"this" after modifying its internal state, and to change the
implementation to that effect to avoid constructing new objects every
time.
Florent
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
Re: various small cleanups
Posted by Florent Guillaume <fg...@nuxeo.com>.
On Thu, Sep 9, 2010 at 3:52 PM, Klevenz, Stephan
<st...@sap.com> wrote:
>Florent wrote:
>> 3.
>> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
>> that just modify the iterable in place (for instance the JCR
>> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
>> and TermDocs.skipTo), and in the use cases I've seen it's no use
>> creating a new object. Is it ok to change this?
>
> to 3. I'm not sure if I understand correctly. The reason to return an iterable is to allow dotted expressions to have some "from-to" semantic of a range: ItemIterable<CmisObject> i = folder.getChildren().skipTo(2).getPage(3);
Returning an object for chained expressions is all right, but what I
want to ensure is that the contract of the method allows it to return
"this" after modifying its internal state, and to change the
implementation to that effect to avoid constructing new objects every
time.
Florent
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
RE: various small cleanups
Posted by "Klevenz, Stephan" <st...@sap.com>.
Hi Florent,
Some comments:
to 1. +1
to 2. +1
to 3. I'm not sure if I understand correctly. The reason to return an iterable is to allow dotted expressions to have some "from-to" semantic of a range: ItemIterable<CmisObject> i = folder.getChildren().skipTo(2).getPage(3);
to 4. The paging mechanism is quite complex and I will look into this later.
Regards,
Stephan
-----Original Message-----
From: Florent Guillaume [mailto:fg@nuxeo.com]
Sent: Donnerstag, 9. September 2010 14:54
To: List-Chemistry
Subject: various small cleanups
Hi,
A few things:
1.
Currently our iterators (CollectionIterator, CollectionPageIterator)
have a next() method that returns null at the end of the iteration.
This is wrong and contrary to the Iterator contract, they should
return NoSuchElementException in that case.
Anyone opposed to fixing this?
2.
Also I'd like to do small renamings like AbstractPageFetch ->
AbstractPageFetcher and PageFetchResult -> FetchedPage or maybe even
Page, and also rename getPage to getItems (or getList).
3.
ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
that just modify the iterable in place (for instance the JCR
RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
and TermDocs.skipTo), and in the use cases I've seen it's no use
creating a new object. Is it ok to change this?
4.
PageFetchResult has:
public BigInteger getTotalItems()
public Boolean getHasMoreItems()
whereas ItemIterable has:
long getTotalNumItems();
boolean getHasMoreItems();
I guess we have to keep Boolean and BigInteger to reflect the domain
model here, but I'll sync the naming.
Comments?
Florent
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
Re: various small cleanups
Posted by Florent Guillaume <fg...@nuxeo.com>.
They can wait, not problem.
The only critical one is the missing NoSuchElementException but that
can be worked around until after the release.
Thanks!
Florent
On Thu, Sep 9, 2010 at 3:03 PM, Florian Müller
<fl...@alfresco.com> wrote:
> Hi Florent,
>
> Gab and I try to get a first release out of the door. We are walking through
> all the legal stuff at the moment in order to build Apache compliant
> packages.
>
> Can your changes wait or should we hold on with release until these topics
> have been discussed and the code has changed?
>
>
> Cheers,
>
> Florian
>
>
>
> On 09/09/2010 13:54, Florent Guillaume wrote:
>>
>> Hi,
>>
>> A few things:
>>
>> 1.
>> Currently our iterators (CollectionIterator, CollectionPageIterator)
>> have a next() method that returns null at the end of the iteration.
>> This is wrong and contrary to the Iterator contract, they should
>> return NoSuchElementException in that case.
>> Anyone opposed to fixing this?
>>
>> 2.
>> Also I'd like to do small renamings like AbstractPageFetch ->
>> AbstractPageFetcher and PageFetchResult -> FetchedPage or maybe even
>> Page, and also rename getPage to getItems (or getList).
>>
>> 3.
>> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
>> that just modify the iterable in place (for instance the JCR
>> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
>> and TermDocs.skipTo), and in the use cases I've seen it's no use
>> creating a new object. Is it ok to change this?
>>
>> 4.
>> PageFetchResult has:
>> public BigInteger getTotalItems()
>> public Boolean getHasMoreItems()
>> whereas ItemIterable has:
>> long getTotalNumItems();
>> boolean getHasMoreItems();
>> I guess we have to keep Boolean and BigInteger to reflect the domain
>> model here, but I'll sync the naming.
>>
>> Comments?
>>
>> Florent
>>
>
>
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
Re: various small cleanups
Posted by Florian Müller <fl...@alfresco.com>.
Hi Florent,
Gab and I try to get a first release out of the door. We are walking
through all the legal stuff at the moment in order to build Apache
compliant packages.
Can your changes wait or should we hold on with release until these
topics have been discussed and the code has changed?
Cheers,
Florian
On 09/09/2010 13:54, Florent Guillaume wrote:
> Hi,
>
> A few things:
>
> 1.
> Currently our iterators (CollectionIterator, CollectionPageIterator)
> have a next() method that returns null at the end of the iteration.
> This is wrong and contrary to the Iterator contract, they should
> return NoSuchElementException in that case.
> Anyone opposed to fixing this?
>
> 2.
> Also I'd like to do small renamings like AbstractPageFetch ->
> AbstractPageFetcher and PageFetchResult -> FetchedPage or maybe even
> Page, and also rename getPage to getItems (or getList).
>
> 3.
> ItemIterable.skipTo returns a new iterable. I'm used to skipTo methods
> that just modify the iterable in place (for instance the JCR
> RangeIterator.skip or JCR2 EventJournal.skipTo or Lucene Spans.skipTo
> and TermDocs.skipTo), and in the use cases I've seen it's no use
> creating a new object. Is it ok to change this?
>
> 4.
> PageFetchResult has:
> public BigInteger getTotalItems()
> public Boolean getHasMoreItems()
> whereas ItemIterable has:
> long getTotalNumItems();
> boolean getHasMoreItems();
> I guess we have to keep Boolean and BigInteger to reflect the domain
> model here, but I'll sync the naming.
>
> Comments?
>
> Florent
>