patternjavaMinor
Streaming pages of a different size than a cache's pages
Viewed 0 times
sizethandifferentcachestreamingpages
Problem
I need to write a service to return objects in pages of size 20. The caching of these objects is done in pages of size 60.
To abstract the logic of streaming differently-sized pages, I wrote this
```
public abstract class PagingAdapter implements Iterable> {
private final int callerPageSize;
private final int sourcePageSize;
private final LoadingCache> localCache =
CacheBuilder.newBuilder().build(new CacheLoader>() {
@Override
public List load(Integer sourcePageNumber) throws Exception {
return fetchPageFromSource(sourcePageNumber);
}
});
public PagingAdapter(int callerPageSize, int sourcePageSize) {
Preconditions.checkArgument(callerPageSize > 0, "callerPageSize must be greater than 0.");
Preconditions.checkArgument(sourcePageSize > 0, "sourcePageSize must be greater than 0.");
this.callerPageSize = callerPageSize;
this.sourcePageSize = sourcePageSize;
}
/**
* Returns a List of objects from the source for the specified page.
*
* If the returned List is smaller in size than the source page size, that
* means there are no further source pages.
*
* @param sourcePageNumber The requested page.
* @return The List of objects.
* @throws Exception
*/
protected abstract List fetchPageFromSource(int sourcePageNumber) throws Exception;
/**
* Returns a List of objects for the specified page, as defined by the caller
* page size.
*
* If the returned List is smaller in size than the caller page size, that
* means there are no further pages available.
*
* @param callerPageNumber The requested page.
* @return The List of objects.
*/
public List getPage(int callerPageNumber) {
Preconditions.checkArgument(callerPageNumber > 0, "callerPageNumber must be greater than 0.");
List callerPage
To abstract the logic of streaming differently-sized pages, I wrote this
PagingAdapter class:```
public abstract class PagingAdapter implements Iterable> {
private final int callerPageSize;
private final int sourcePageSize;
private final LoadingCache> localCache =
CacheBuilder.newBuilder().build(new CacheLoader>() {
@Override
public List load(Integer sourcePageNumber) throws Exception {
return fetchPageFromSource(sourcePageNumber);
}
});
public PagingAdapter(int callerPageSize, int sourcePageSize) {
Preconditions.checkArgument(callerPageSize > 0, "callerPageSize must be greater than 0.");
Preconditions.checkArgument(sourcePageSize > 0, "sourcePageSize must be greater than 0.");
this.callerPageSize = callerPageSize;
this.sourcePageSize = sourcePageSize;
}
/**
* Returns a List of objects from the source for the specified page.
*
* If the returned List is smaller in size than the source page size, that
* means there are no further source pages.
*
* @param sourcePageNumber The requested page.
* @return The List of objects.
* @throws Exception
*/
protected abstract List fetchPageFromSource(int sourcePageNumber) throws Exception;
/**
* Returns a List of objects for the specified page, as defined by the caller
* page size.
*
* If the returned List is smaller in size than the caller page size, that
* means there are no further pages available.
*
* @param callerPageNumber The requested page.
* @return The List of objects.
*/
public List getPage(int callerPageNumber) {
Preconditions.checkArgument(callerPageNumber > 0, "callerPageNumber must be greater than 0.");
List callerPage
Solution
I like your usage of Guava, and overall your code seems really good. I might actually use it myself somewhere.
The code could use some more Javadoc though, especially on the class itself and the constructor. Explaining those
Possible extension
Currently, you leave some work for the user of your code to divide it into some fixed page size. What if that programmer is very lazy and don't want to do that? An optional constructor could be helpful to indicate that technically, you support that feature.
However, if you add that optional constructor; the users will have to send your code a List of size one. That's not optimal. Perhaps a class to create pagination without some previously already existing pagination would be useful? (This is actually what I expected from your code when I first saw the class name of it)
A possible bug
Your variable
What if
Adding one extra string to the second page of your example causes it to be completely ignored for some
Also, if
Possible Solution: Throw an exception if the
Even though this might be by design, it is very important to document this behavior.
Another possible problems might encounter when using your class is that if
Remember that when writing code for other people to use, they might not use it in the way that is intended.
The code could use some more Javadoc though, especially on the class itself and the constructor. Explaining those
callerPageSize and sourcePageSize parameters is very important.Possible extension
Currently, you leave some work for the user of your code to divide it into some fixed page size. What if that programmer is very lazy and don't want to do that? An optional constructor could be helpful to indicate that technically, you support that feature.
public PagingAdapter(int callerPageSize) {
this(callerPageSize, 1);However, if you add that optional constructor; the users will have to send your code a List of size one. That's not optimal. Perhaps a class to create pagination without some previously already existing pagination would be useful? (This is actually what I expected from your code when I first saw the class name of it)
A possible bug
Your variable
sourcePageSize confused me a bit at first, even though it sounds like it should be equal to sourcePage.size() there is absolutely nothing that forces it to be so.What if
fetchPageFromSource returns a list that does not correspond to the sourcePageSize that the class got? I can tell you what happens:Adding one extra string to the second page of your example causes it to be completely ignored for some
callerPageSizes, and for other values of callerPageSize the first absolute index directly after is ignored.Also, if
fetchPageFromSource accidentally returns a list of size less that sourcePageSize before it actually is over, then that effectively cuts the list so that the last items does not get seen. By removing the item "20" in your example, the list always stopped at 19 for values of callerPageSize.Possible Solution: Throw an exception if the
fetchPageFromSource method returns a list of a bigger size than what was expected. Solving the problem of an incorrect smaller list is more difficult because of your optimization of smaller lists == no more items.Even though this might be by design, it is very important to document this behavior.
Another possible problems might encounter when using your class is that if
fetchPageFromSource returns null; this causes InvalidCacheLoadException (caused by Guava). This should either be fixed or documented. Returning null might be a way for a programmer to try and tell your code: "This page does not exist".Remember that when writing code for other people to use, they might not use it in the way that is intended.
Code Snippets
public PagingAdapter(int callerPageSize) {
this(callerPageSize, 1);Context
StackExchange Code Review Q#12433, answer score: 2
Revisions (0)
No revisions yet.