Updates to ADBA and AoJ

[Douglas Surber]

Here’s the fix to forEach

    @Override
    public default void forEach(Consumer<? super Column> action) {
      do {
        action.accept(this);
        if (!hasNext()) break;
        next();
      } while (true);
    }

I modified iterator() to return this.clone().

    @Override
    public default Column iterator() {
      return this.clone();
    }

These changes will show up in the next push.

I share your unease about a self iterating mutable, but I think it is useful. It is well defined and easily understood, once one realizes that there is something to understand. It does push the boundaries of the Principle of Least Surprise. Still, I think currying the get method in this way is a good thing. It supports many different access patterns, all of which are used in real apps. Which violates the One Way is Enough guideline, but that is violated as soon as we add both named and indexed access. I don’t know if Column should go into the final version, but I intend to keep it until there is compelling evidence that we should do something else.

By all means I want to hear everyone’s feed back on this change.

Douglas

> On Jun 25, 2018, at 4:40 AM, Mark Rotteveel <mark at lawinegevaar.nl> wrote:
> 
> Douglas,
> 
> On the asyncjdbc-dev mailing list, Alexander Kjäll pointed out that the Result.Column.forEach() fails to move the iterator forward, which means it results in an infinite loop.
> 
> You may want to consider not overriding the default method from the Iterable interface, but I think the mutable self-iteration causes problems here.
> 
> Related to that, I'm not entirely sure I like the fact that this is a self-iterating mutable. I fear that - especially in an asynchronous context - this can result in hard to diagnose bugs, also the fact that without explicit action this is only-once iterable maybe confusing as well.
> 
> Mark
> -- 
> Mark Rotteveel