Suggestion to support arbitrary chaining with subclasses of FluentIterable

[GoogleCodeExporter]

This is a suggestion for the FluentIterable class - which is clearly intended 
to be extended - and also a general solution for chainability of extensible 
fluent interfaces.  The problem is that the chainable methods on 
FluentIterable<T> return a FluentIterable<T>, and so any chainable methods 
defined on the subclass can't be called after that.  This problem forces 
chained calls to appear in a specific sequence, subclass methods first.  
(Perhaps this is not such a terrible problem, but with IDE auto-complete it's 
very easy to run into.)

I've solved this in my own code with a certain ugly approach to template 
declarations - but the ugliness is restricted to the extends clause of subclass 
declarations.  If the base class is used directly, as FluentIterable likely is, 
then the ugliness may leak out, though it can easily be elided from the returns 
from static methods such as FluentIterable.from().

The solution is to introduce an additional template argument like so:

public abstract class FluentIterable<E, T extends FluentIterable<E,T>>
implements Iterable<E>

Chainable methods should return T.  A protected method can be defined to aid 
this class and all subclasses in providing the correct return (because "return 
this" won't work):

    @SuppressWarnings("unchecked")
    protected final T me() {
        return (T) this;
    }

Chainable methods can then "return me()".

Each subclass merely needs to include itself as the 2nd template argument.  If 
the subclass is also intended for extension, then it can use the same template 
signature, e.g., 

public abstract class MyFluentIterable<E, T extends MyFluentIterable<E,T>> 
extends FluentIterable<E,T>

I've used this and it definitely lets you chain methods arbitrarily.  The only 
drawback I can see is potential confusion from the wonky template signature.  
(I suspect anyone smart enough to want to use FluentIterable in the first place 
could understand it.)

A possible solution for segregating the confusion - and also supporting the 
continued use of the FluentIterable interface as it exists now - is to 
introduce a new base class for FluentIterable<E> using this template signature 
mechanism, and move all of the non-static methods to the base class.  
FluentIterable would retain the same effective class signature, and anyone who 
wanted to gain the benefit of arbitrary chaining could switch to extending that 
base class directly.

Original issue reported on code.google.com by djn...@gmail.com on 2 Oct 2012 at 7:03

[GoogleCodeExporter]

This couldn't work, even if it was worth the extra API complication.

Consider the base FluentIterable class's method skip().  This *has* to return a 
FluentIterable<E>; it has no way of knowing how to create a T result.

A subclass of FluentIterable called Foo should override the methods that should 
return Foo and either wrap the FluentIterables returned by super.method(), or 
it should use another implementation.  There shouldn't be any problem with 
that, no?

If this doesn't work, can you give a *complete* example of a subclass that you 
would write using this technique, so we can compare implementations 
side-by-side?

Original comment by lowas...@google.com on 2 Oct 2012 at 11:26

[GoogleCodeExporter]

I think it's possible to make it work by having the parent class create 
FluentIterables then pass them to an overrideable method that chains that 
_again_ into the desired type.  But in general, while the "extensible builder" 
pattern has no good solution in Java unless the notion of a self-type is added 
(seems unlikely), I think this self-generification approach is not the least of 
all evils.  It's better to just make subclasses override every method to refine 
the return types.  Tedious for the subclasses, but a lot cleaner for users.

Original comment by kevinb@google.com on 3 Oct 2012 at 4:29

• Changed state: WontFix

[GoogleCodeExporter]

Do we in fact expect for people to extend FluentIterable in this way?  My 
understanding was that it was subclassable mostly as a 
microoptimization/micro-readability aid that lets users write:

return new FluentIterable<E>() {
  public Iterator<E> iterator() { ... }
};

...instead of...

return FluentIterable.of(new Iterable<E>() {
  public Iterator<E> iterator() { ... }
});

Original comment by cpov...@google.com on 3 Oct 2012 at 4:32

[GoogleCodeExporter]

I was under the vague impression we wanted to leave room for e.g. FluentSet and 
other collections.

Original comment by wasserman.louis on 3 Oct 2012 at 5:33

[GoogleCodeExporter]

I'm sorry! I described a strategy for implementing extensible builders that 
works for *mutable* builder classes, but of course FluentIterable is immutable. 
 The strategy could be adapted for immutable classes by implementing a factory 
method similar to the "me()" method.  This would work for FluentIterable; the 
method implementations that use from() to construct the result could instead 
call the factory method, and the default implementation of the factory method 
would delegate to from().  This would force subclasses to override the factory 
method, leaking the ugliness a bit more.

I agree that making subclasses override all the methods, though tedious, is 
cleaner for users than the self-generification strategy (as you named it... 
good name).  However, it would be simpler for subclasses to do so if 
FluentIterable methods called a protected non-static factory method instead of 
just calling from() directly.  Then subclasses could merely delegate each 
overridden method to super and do an unchecked cast on the return.

Original comment by djn...@gmail.com on 5 Oct 2012 at 6:44

[GoogleCodeExporter]

>However, it would be simpler for subclasses to do so if FluentIterable methods 
called a protected non-static factory method instead of just calling from() 
directly.  Then subclasses could merely delegate each overridden method to 
super and do an unchecked cast on the return.

Eh?  The subclasses would still have to override everything...it'd just be a 
little shorter, *maybe*.  Is that worth the deliberately dangerous unchecked 
cast?  I don't think it is.

Original comment by lowas...@google.com on 5 Oct 2012 at 7:49

[GoogleCodeExporter]

Unchecked != unsafe in all cases, especially when the design contract is to 
instantiate with a factory (provided by the same code unit doing the cast), but 
I think this is tangential.  Regardless of whiether the cast is checked, a 
factory method would let subclasses override chainable methods without needing 
to put yet another wrapper around the superclass method return; though I 
suppose it is a very lightweight wrapper, as FluentIterable is.

The real question here is whether this class is designed for extension.  If it 
is - and if and when you implement something like a FluentSet extending 
FluentIterable - I suspect you'll want to address this somehow.

A key point about the extensibility of a class like FluentIterable is that the 
purpose of fluent interfaces is to improve the readability (and writability) of 
the code that *uses* the fluent interface.  Making it easy and pleasant to 
extend seems less important.  Making extension possible at all is useful, 
though.

Original comment by djn...@gmail.com on 9 Oct 2012 at 12:47

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:13

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08

• Added labels: Restrict-AddIssueComment-Commit