2

I know there are many variations and related topics to this one here on stack overflow but I haven't found any compelling answers so I'll give it a go myself.

I'm trying to design a builder factory that returns different subclasses of a common builder interface. I want to allow all the implementations to share a common abstract class for code re-use.

Note that I'm not interested in the return type of the build() method, only what types the builders are.

This is what I have so far:

Builder interface with generic for the sub-interfaces:

interface FruitBuilder<T extends FruitBuilder<T>> {
    T taste(String taste);
    T shape(String shape);
    T weight(String weight);

    Fruit build();
}

Some builders have additional methods:

interface GrapesBuilder extends FruitBuilder<GrapeBuilder> {
    GrapesBuilder clusterSize(int clusterSize);
}

Next is to specify a factory that returns the specific builders:

interface FruitBuilderFactory {
    GrapesBuilder grapes();
    AppleBuilder apple();
    LemonBuilder lemon();
}

A user of these interfaces should be able to use it like:

 Fruit grapes = fruitBuilderFactory
    .grapes()
    .weight(4)
    .color("Purple")
    .clusterSize(4)  // Note that the GrapesBuilder type must be accessible here!
    .build();

Most of the logic would go into the abstract class, including advanced build logic:

abstract class BaseFruitBuilder<T extends FruitBuilder<T>> implements FruitBuilder<T> {

   String taste;

   T taste(String taste) {
       this.taste = taste;
       return (T)this;     // Ugly cast!!!!!
   }

   ...

    Fruit build() {
       Fruit fruit = createSpecificInstance();

       // Do a lot of stuff on the fruit instance.

       return fruit;
    }

    protected abstract Fruit createSpecificInstance();
}

Given the base class, it's really simple to implement new builders:

class GrapseBuilderImpl extends BaseFruitBuilder<GrapesBuilder> {
   int clusterSize;
   GrapesBuilder clusterSize(int clusterSize) {
       this.clusterSize = clusterSize;
   }

   protected Fruit createSpecificInstance() {
       return new Grape(clusterSize);
   }
}

This is all compiling and fine (at least my real code). The question if whether or not I can remove the ugly cast to T in the abstract class.

Johan Tidén
  • 664
  • 7
  • 19
  • 1
    It's not very ugly:) `T` intends to be the type of `this`, so `(T)this` is perfectly fine. I would rather use `This` as the name of the type variable, therefore `(This)this` looks even saner. However, there *is* a solution if you really hate the cast; but I don't think it's worth the effort. – ZhongYu Sep 29 '15 at 16:51
  • If all the implementations share the same properties, why can't you just make a `BuilderParameters` class (concrete non-generic non-final class), which you then feed into your builder? The client code would be something like `Fruit grapes = fruitBuilderFactory.grapes().build(GrapesBuilderParams.newInstance().weight(...).color(...).clusterSize(...))`. By doing this, you would eliminate all the genericity from the parameters (so no casts anymore) and lose very little from the customizability side (i.e. builders can't depend on the parameter initialization order, because they don't know it) – Giulio Franco Sep 29 '15 at 17:00
  • 1
    Maybe [getThis()](http://www.angelikalanger.com/GenericsFAQ/FAQSections/ProgrammingIdioms.html#FAQ206) trick will work? – user158037 Oct 16 '15 at 15:19

2 Answers2

2

One option to avoid casting is to define a single abstract method returning T:

abstract class BaseFruitBuilder<T extends FruitBuilder<T>> implements FruitBuilder<T> {

    String taste;

    T taste(String taste) {
       this.taste = taste;
       return returnThis();
    }

    protected abstract T returnThis();

     //...
}

class GrapseBuilderImpl extends BaseFruitBuilder<GrapesBuilder> {
    //...
    @Override
    protected T returnThis() {
        return this;
    }
}

The downside is that you have to trust each subclass to implement the method correctly. Then again, with your approach, there's nothing stopping anyone from declaring a subclass GrapesBuilder extends BaseFruitBuilder<AppleBuilder>, so you'll need to trust subclasses to some extent.

EDIT Just realized this solution was referenced by @user158037's comment. I've used this myself, but never realized it was a known idiom. :-)

Community
  • 1
  • 1
shmosel
  • 49,289
  • 6
  • 73
  • 138
  • Thanks, this may be a cleaner (and hopefully a little safer) than the casting approach. I agree though with the other comments that this will make it (a little) harder to extend the builders, especially subclasses of already concrete builders. – Johan Tidén Jul 11 '16 at 13:29
1

You're using what are commonly referred to as self-types, but seem to be somewhat muddling what T refers to. You have a FruitBuilder interface that has a generic type T, which should represent the type the builder will return. Instead you seem to be using it to represent the type of the builder itself, which likely isn't necessary (if it is, see below for a more complex suggestion).

Be careful and intentional with generics; the fact that they are abstract concepts makes them easy to confuse. In your case I would suggest the following interface:

interface FruitBuilder<F extends Fruit> {
  FruitBuilder<F> taste(...);
  FruitBuilder<F> shape(...);
  FruitBuilder<F> weight(...);
  F build();
}

Individual builders now declare the type they will eventually build, rather than their own type:

interface FruitBuilderFactory {
  GrapesBuilder grapes(); // define a concrete subtype to add methods
  FruitBuilder<Apple> apple();
}

And now every FruitBuilder clearly is a builder of F instances, and each of your builder methods can cleanly return this and your build() method will return an object of the expected generic type, letting you write something like:

Grape grape = FruitBuilderFactory.grape()....build();

For most sub-class builder patterns this is all you need. Even if you need to define additional builder methods for certain types you can still use this structure. Consider Guava's ImmutableCollection.Builder<E>, and the ImmutableMultiset.Builder<E> subtype that extends it and provides additional methods. Notice also that they associate the builder directly with the type, rather than in a common builder-factory class. Consider replicating that structure yourself (e.g. Grape.Builder, Apple.Builder, etc.).


In rare cases you do need to use self-types and represent the builder as a generic type as well. This creates a complex type structure but does show up in practice in certain places such as Truth's Subject<S extends Subject<S,T>,T> (which has significantly more complex semantics than most builders). Notice that it has two generics; S represents itself while T represents the type actually being worked with. If your FruitBuilder needed to be this complex you'd use a similar pattern:

interface FruitBuilder<B extends FruitBuilder<B, F>, F> {
  B taste(...);
  B shape(...);
  B weight(...);
  F build();
}
dimo414
  • 47,227
  • 18
  • 148
  • 244
  • Guava has to override each method in each subclass to return the current type. Not very elegant. – shmosel Jul 10 '16 at 20:02
  • @shmosel it's a design decision. Since there's a finite number of implementations and the builders are intended to be easy for callers to work with it's cleaner to hide the inelegance in the implementation. Using a self-type would pass some of the burden onto callers, which in my opinion would be far less elegant. Notice for instance the necessary type signature to construct an [`IterableSubject`](https://google.github.io/truth/api/0.28/com/google/common/truth/Truth.html#assertThat-java.lang.Iterable-). This works for Truth's use case but with builders would make callers lives harder. – dimo414 Jul 10 '16 at 20:17
  • How exactly does the self type make it more difficult for callers? – shmosel Jul 10 '16 at 20:23
  • @shmosel if a self-typed instance needs to be assigned to a variable the caller then has to properly declare that type, which at worst is tricky and at best is simply extra typing. This isn't an issue for `IterableSubject` since it's intended to be used fluently (therefore rarely assigned), but forcing callers to declare a type like `IterableSubject extends IterableSubject,T,C>,T,C>` would be a hassle. Like I said, it's a design decision so you can make good arguments for either option, but in practice you the user are unimpeded by Guava's decision to avoid self-typing. – dimo414 Jul 10 '16 at 20:45
  • Most builders are meant to be used fluently, and they can usually be assigned by their concrete type (otherwise they wouldn't be able to add methods). – shmosel Jul 10 '16 at 20:48
  • @shmosel yes, but there are many cases where builders are held for longer than a single expression, for instance if you need to add values in a loop. I'm not sure what your point is; you *can* use self-types, but you often do not *need* to, and type signatures that avoid self-types are simpler to reason about. Does any of that not resonate with you? You can always [ask the Guava devs](https://groups.google.com/forum/#!forum/guava-discuss) directly why they felt self-types weren't desirable in this particular case. – dimo414 Jul 10 '16 at 20:57
  • "Instead you seem to be using it to represent the type of the builder itself, which likely isn't necessary". The whole premise of the question is that the builder type is _necessary_ and the return type is not. – Johan Tidén Jul 11 '16 at 13:34
  • And at the bottom of my answer I describe how to structure a self-typed builder properly. – dimo414 Jul 11 '16 at 16:03