add boolean Page.hasNextPage()

[gavinking]

A Page always knows if there are more results. But the only way to really get that information is to call page.nextPageRequest(). Thus, the idiom for iteration is:

Page<Book> page = library.books("%", pageRequest);
...
while (page.nextPageRequest() != null) {
    page = library.books("%", slice.nextPageRequest());
    ...
}

which is quite nonbeautiful.

This proposal adds a last() method to Page, so that the idiom becomes:

Page<Book> page = library.books("%", pageRequest);
...
while (!page.last()) {
    page = library.books("%", slice.nextPageRequest());
    ...
}

Now, if it were my call I would also add:

• last() to Slice, not just Page, and
• first() to KeysetAwareSlice

but I wanted to see how you guys really react to just this.

[gavinking]

Actually last() is a terrible name, it sounds like you're asking for the last page. Since we're not using JavaBeans naming, I'm not sure what to call it. hasNextPage(), I suppose. (With the opposite polarity.)

Page<Book> page = library.books("%", pageRequest);
...
while (page.hasNextPage()) {
    page = library.books("%", slice.nextPageRequest());
    ...
}

[njr-11]

I haven't looked at the code changes yet, but I like this better than the proposal to make nextPageRequest return an Optional. Some suggestions for names are:

hasNext()
and
hasPrevious()

which will line up with the our existing method names nextPageRequest and previousPageRequest,
and also exactly match the method names that are used on java.util.Iterator and java.util.ListIterator.
This pattern lines up with the Iterator pattern of needing to check if the next exists before obtaining it.
Similarly, we can have nextPageRequest/previousPageRequest raise NoSuchElementException if there is not a next/previous (meaning hasNext or hasPrevious returned false or would have returned false if invoked), which will eliminate the possibility of nulls.

That said, we do need to write the documentation to allow for the possibility that a page might come back empty. For example, if using the slice pattern where the total is not known and we currently have a full page and so return a next page request, the next page might actually come back empty because there actually weren't any more results. This should be fine - the user just needs to be aware to allow for the possibility of the final page being empty.

[gavinking]

Yes, hasNext() is fine by me. Shorter. Which is good.

[njr-11]

This should be put onto Slice rather than Page (unless we get rid of Slice as another issue discusses) because the pattern of avoiding null PageRequest applies to Slice as well.

The JavaDoc for hasNext is already well written and it just needs a minor adjustment to allow for the scenario where the next page needs to be attempted to find out whether there are more results.

If we add this, we should also include the corresponding hasPrevious() for keyset cursor based pagination.

[gavinking]

This should be put onto Slice rather than Page (unless we get rid of Slice as another issue discusses) because the pattern of avoiding null PageRequest applies to Slice as well.

If we add this, we should also include the corresponding hasPrevious() for keyset cursor based pagination.

Yes, I know, this was just a straw man.

[gavinking]

I just put hasNext() and hasPrevious() on the places they belong. Please review.

[otaviojava]

Using false to both makes it unclear if it is empty or the database does not know the previous page.

Are we sure to proceed this way?

I mean, if I receive false I need to check anyway...

Maybe an UnsupportedOperationException when it does not know

[njr-11]

Using false to both makes it unclear if it is empty or the database does not know the previous page.

Are we sure to proceed this way?

I mean, if I receive false I need to check anyway...

Maybe an UnsupportedOperationException when it does not know

Otavio, are you sure you are reading this correctly? When the value is false is when you should never try to request the page because it is known to not exist. An empty page has no cursors and therefore cannot have a previous. In the other case it is stated there is no previous page. If you get false and then ask for previousPageableRequest anyways, you can expect to get an exception from the latter. There is no point in trying.

[otaviojava]

That was my bad. Thank you for clarifying it.

[otaviojava]

The same point here.

Include the UnsupportedOperationException if the database does not know.

[njr-11]

No -that wrecks the whole pattern. The point is that if the provider does not know if there is going to be a next page, then we want the user to request it to find out and the user should be able to do that without having to experience exceptions.

[gavinking]

@otaviojava I think the idea here is that if you have a full last page of results you might not know that it is the last page. What @njr-11 is trying to permit here is that there be an "empty last page". Which I think is fine and correct. Providers can in principle use a "fancy" implementation that avoids this, but I don't think we should mandate such fanciness.