Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

do-while with Java8-Optional

I'm frequently using the do-while-checkNextForNull-getNext looping pattern (don't know if there is an official name for it) in some of my projects. But in Java8, the use of Optional is considered as cleaner code than checking for null references in client-code. But when using Optional in this looping pattern, the code gets a bit verbose and ugly, but because Optional has some handy methodS, I would expect that there must exist a cleaner way than the one I came up with below.

Example:

Given the following class.

class Item {
    int nr;

    Item(nr) {
        this.nr = nr;
        // an expensive operation
    }

    Item next() {
        return ...someCondition....
            ? new Item(nr + 1)
            : null;
    }
}

In which the first item always has nr==1 and each item determines the next item, and you don't want to create unnecessary new items.

I can use the following looping do-while-checkNextForNull-getNext pattern in client-code:

Item item = new Item(1);
do {
    // do something with the item ....
} while ((item = item.next()) != null);

With Java8-Optional, the given class becomes:

class Item {
    ....

    Optional<Item> next() {
        return ...someCondition....
            ? Optional.of(new Item(nr + 1))
            : Optional.empty();
    }
}

And then the do-while-checkNextForNull-getNext looping pattern becomes a bit ugly and verbose:

Item item = new Item(1);
do {
    // do something with the item ....
} while ((item = item.next().orElse(null)) != null);

The orElse(null)) != null part feels uncomfortable.

I have looked for other kind of loops, but haven't found a better one. Is there a cleaner solution?

Update:

It is possible to use a for-each loop while at the same time avoiding null-references (the use of null-references is considered as a bad practice). This solution has been proposed by Xavier Delamotte, and doesn't need Java8-Optional.

Implementation with a generic iterator:

public class Item implements Iterable<Item>, Iterator<Item> {
    int nr;

    Item(int nr) { 
        this.nr = nr;
        // an expensive operation
    }

    public Item next() {
        return new Item(nr + 1);
    }

    public boolean hasNext() {
        return ....someCondition.....;
    }

    @Override
    public Iterator<Item> iterator() {
        return new CustomIterator(this);
    }
}

and

class CustomIterator<T extends Iterator<T>> implements Iterator<T> {
    T currentItem;
    boolean nextCalled;

    public CustomIterator(T firstItem) {
        this.currentItem = firstItem;
    }

    @Override
    public boolean hasNext() {
        return currentItem.hasNext();
    }

    @Override
    public T next() {
        if (! nextCalled) {
            nextCalled = true;
            return currentItem;
        } else {
            currentItem = currentItem.next();
            return currentItem;
        }
    }
}

Then client code becomes very simple/clean:

for (Item item : new Item(1)) {
    // do something with the item ....
}

Although this may be seen as a violation of the Iterator contract because the new Item(1) object is included in the loop, whereas normally, the for loop would immediately call next() and thus skipping the first object. In other words: for the first object, next() is violated because it returnS the first object itself.

like image 949
Devabc Avatar asked Feb 26 '15 11:02

Devabc


2 Answers

You can do something like this :

Optional<Item> item = Optional.of(new Item(1));
do {
    Item value = item.get();
    // do something with the value ....
} while ((item = value.next()).isPresent());

or (to avoid the extra variable) :

Optional<Item> item = Optional.of(new Item(1));
do {
    // do something with item.get() ....
} while ((item = item.get().next()).isPresent());
like image 94
Eran Avatar answered Sep 20 '22 23:09

Eran


in Java8, the use of Optional is considered as cleaner code than checking for null references in client-code

No, it is the other way around: Optional can be used where it helps write cleaner code. Where it doesn't, just stick to the old idiom. Do not feel any pressure to use it if your existing idiom looks fine—and it does, in my opinion. As an example, this would be good usage of the Optional:

item.next().map(Object::toString).ifPresent(System.out::println);

Since you need to break out of the loop on the first non-present Optional, this doesn't really help.

However, I assume your true interest is more general: leveraging the features of Java 8 for your code. The abstraction you should pick is the Stream:

itemStream(() -> new Item(1)).forEach(item -> { ... all you need ... });

And, naturally, you can now go wild with stream processing:

itemStream(() -> new Item(1)).filter(item.nr > 3).mapToInt(Item::nr).sum();

This is how you would construct the stream:

import java.util.Spliterators;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

public class ItemSpliterator extends Spliterators.AbstractSpliterator<Item>
{
  private Supplier<Item> supplyFirst;
  private Item lastItem;

  public ItemSpliterator(Supplier<Item> supplyFirst) {
    super(Long.MAX_VALUE, ORDERED | NONNULL);
    this.supplyFirst = supplyFirst;
  }

  @Override public boolean tryAdvance(Consumer<? super Item> action) {
    Item item;
    if ((item = lastItem) != null)
      item = lastItem = item.next();
    else if (supplyFirst != null) {
      item = lastItem = supplyFirst.get();
      supplyFirst = null;
    }
    else return false;
    if (item != null) {
      action.accept(item);
      return true;
    }
    return false;
  }

  public static Stream<Item> itemStream(Supplier<Item> supplyFirst) {
    return StreamSupport.stream(new ItemSpliterator(supplyFirst), false);
  }
}

With this you are a tiny step away from the ability to seamlessly parallelize your computation. Since your item stream is fundamentally sequential, I suggest looking into my blog post on this subject.

like image 41
Marko Topolnik Avatar answered Sep 20 '22 23:09

Marko Topolnik