2

I'm building a JavaBean (only fields and getters/setters) using the builder pattern.

For the sake of this example, assume this is our bean:

public class Pizza {
  private int size;
  private boolean cheese;
  private boolean pepperoni;
  private boolean bacon;

  private Pizza(Builder builder) {
    size = builder.size;
    cheese = builder.cheese;
    pepperoni = builder.pepperoni;
    bacon = builder.bacon;
  }

  public static class Builder {
    //required
    private final int size;

    //optional
    private boolean cheese = false;
    private boolean pepperoni = false;
    private boolean bacon = false;

    public Builder(int size) {
      this.size = size;
    }

    public Builder cheese(boolean value) {
      cheese = value;
      return this;
    }

    public Builder pepperoni(boolean value) {
      pepperoni = value;
      return this;
    }

    public Builder bacon(boolean value) {
      bacon = value;
      return this;
    }

    public Pizza build() {
      return new Pizza(this);
    }
  }
}

Taken from here.

Now I've been trying to ensure that all of the fields in Pizza are non-null, with reflection, iterating over the fields of Pizza and checking they aren't null, but it appears (and I could be wrong here) that my fields aren't set before the check occurs. This code by Jon Skeet is what I altered to check the non-nullness of my fields (and instead of counting, I'm throwing exceptions).

I then tried to check the fields of my builder instead, but I have extra fields in the builder (for instance, I have an XMLParser field which may be null). Subsetting the builder fields by the pizza fields doesn't work as they have different 'package paths' (?), e.g. org.GiusepesPizzaria.pizza.size vs org.GiusepesPizzaria.builder.size

Is there a better way to check this? Before implementing the reflection method, I used this sort of construct:

if(builder.size ==null){
    throw new BadPizzaException("Eh, what're ya doin'?"+
             " Pizza Size was not set correctly");
}else{
    size=builder.size;
}

But it ends up, if you have say ~10 fields to check, long winded, and clutters what should be a simple class.

So that's what I've tried. Is there a better method?

Community
  • 1
  • 1
AncientSwordRage
  • 7,086
  • 19
  • 90
  • 173
  • Whether the fields are set should not be a case of "maybe". Once you set them, they are set. Make sure you are testing the right fields, though: those in the `Builder` and not in `Pizza`. – Marko Topolnik May 15 '13 at 11:59
  • @MarkoTopolnik I was replicating the methods form memory. When I get a chance I'll go an update my question. The general question still stands though. – AncientSwordRage May 15 '13 at 12:24
  • 1
    Generally reflection is a good approach for massive `null`-checking. – Marko Topolnik May 15 '13 at 12:27

2 Answers2

2

An interesting pattern to ensure that all variables are set is to use the Step Builder Pattern where the first setter only allows you to set the second, the second only allows the third and so on. When you're at the last step you can build the class and by then you'll know that all methods have been called.

A short extract from that post:

Panino solePanino = PaninoStepBuilder.newBuilder()
        .paninoCalled("sole panino")
        .breadType("baguette")
        .fish("sole")
        .addVegetable("tomato")
        .addVegetable("lettece")
        .noMoreVegetablesPlease()
        .build();

Where you must start with what the panino is called and follow it up with the bread type.

jontejj
  • 2,800
  • 1
  • 25
  • 27
0

Try this:

public class Pizza
{
    private final boolean bacon;
    private final boolean cheese;
    private final boolean pepperoni;
    private final int size;

    private Pizza()
    {
        throw new UnsupportedOperationException();
    }

    Pizza(
        final int theSize,
        final boolean theCheese,
        final boolean thePepperoni,
        final boolean theBacon)
    {
        bacon = theBacon;
        cheese = theCheese;
        pepperoni = thePepperoni;
        size = theSize;
    }
}

// new file.
public class PizzaBuilder
{
    private boolean bacon;
    private boolean cheese;
    private boolean pepperoni;
    private int size;

    public PizzaBuilder()
    {
        size = 9; // default size.
    }

    public void setHasBacon()
    {
        bacon = true;
    }

    public void setHasNoBacon()
    {
        bacon = false;
    }

    public void setHasCheese()
    {
        cheese = true;
    }

    public void setHasNoCheese()
    {
        cheese = false;
    }

    public void setHasPepperoni()
    {
        pepperoni = true;
    }

    public void setHasNoPepperoni()
    {
        pepperoni = false;
    }

    public void setSizeNineInch()
    {
        size = 9;
    }

    public void setSizeTwelveInch()
    {
        size = 12;
    }

    public Pizza buildPizza()
    {
        return new Pizza(size, cheese, pepperoni, bacon);
    }
}

With the builder above, there is no chance that the builder will ever produce an invalid pizza.

Assumption: only 9 and 12 inch pizza's are supported. add more setSize as needed.

The builder uses what I refer to as NMSetters. This style setter allows you to set values but does not expose the implementation of said value. It seems likely that this is not an original invention on my part.

DwB
  • 37,124
  • 11
  • 56
  • 82