[RFC] Iterator concept for paginated collections #31

Closed
wants to merge 9 commits into
from

Projects

None yet

2 participants

@Kami
Contributor
Kami commented Apr 2, 2013

This is a proposal for modifying all the list methods to expose an iterator interface.

If you like the proposal I will make it more generic and finish the implementation:

  • Move getCollectionIterator to BaseClient
  • Use generics for Container and other typec
  • etc.

Using it would look something like this:

        Iterator<Service> iterator = client.getServicesClient().list(new PaginationOptions(10, null));
        Integer i = 0;

        while (iterator.hasNext()) {
           Service service = iterator.next();
           System.out.println("Service: " + i.toString());
           System.out.println(service.getId());

            i++;
        }
@gdusbabek gdusbabek commented on the diff Apr 2, 2013
...cloud/client/service_registry/clients/BaseClient.java
if (params == null) {
params = new ArrayList<NameValuePair>();
}
if (paginationOptions != null) {
if (paginationOptions.getLimit() != null) {
+ // TODO: Use a Hashmap and make this nicer
+ paramIndex = Utils.getNameIndex(params, "limit");
@gdusbabek
gdusbabek Apr 2, 2013 Contributor

Utils needs import. I think this is what is breaking TravisCI.

@gdusbabek gdusbabek commented on the diff Apr 2, 2013
...cloud/client/service_registry/clients/BaseClient.java
@@ -123,16 +123,31 @@ public void run() {
}
protected ClientResponse performListRequest(PaginationOptions paginationOptions, String path, List<NameValuePair> params, HttpRequestBase method, boolean parseAsJson, Type responseType) throws Exception {
+ Integer paramIndex;
@gdusbabek
gdusbabek Apr 2, 2013 Contributor

Minor nit: paramIndex could be scoped locally to the block in which it is used. Doesn't bother me though.

@gdusbabek gdusbabek commented on the diff Apr 2, 2013
...d/client/service_registry/clients/ServicesClient.java
@@ -45,22 +47,31 @@ public ServicesClient(AuthClient authClient, String apiUrl) {
this.authClient = authClient;
}
- public List<Service> list(PaginationOptions paginationOptions) throws Exception {
+ public AbstractIterator<Service> list(PaginationOptions paginationOptions) throws Exception {
@gdusbabek
gdusbabek Apr 2, 2013 Contributor

Two things: 1) it should just declare public Iterator (use the most generic interface you can get away with). 2) I think it would be more practical if list returned an instance of Iterable. That way you could do stuff like:

    for (Service svc : client.list(options)) {
       // woot!
    }

instead of this:

    Iterator<Service> iterator = client.list(options);
    while (iterator.hasNext()) {
        Service service = iterator.next();
    }
@Kami
Contributor
Kami commented Apr 15, 2013

Closing in favor of #33.

@Kami Kami closed this Apr 15, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment