Is it strange for a Builder object to have getter methods?

Question

I have a fairly complex immutable data type that I'm using a builder object to instantiate. Currently, I have a setup where I parse a file, setting various fields in my builder, and then build the object. However, the files I'm using are error-prone, so I want to do some verification on my unbuilt objects first. And it's important to note that I don't want to throw an exception if an object doesn't pass verification, as that doesn't mean it's invalid, just incorrect.

The way I'm accustomed to seeing builders, though, is that they're "write-only". For example, Guava's ImmutableList.Builder class does not allow you to view the data you've submitted to it, leading me to use ArrayLists as a builder on the occasion that I need to edit the data before freezing it.

Alternatively, I suppose I could just go ahead and build my objects with errors in them, then create a new builder pre-loaded with the data of the built object, inspect the data, edit, and rebuild. This seems wasteful and inelegant, though.

Is it considered a code smell to put getter methods on your builder objects? Intuitively, it does seem a little strange to me, but more than once I've been faced with this type of problem, which often prompts me to keep an accessible copy of some of the data I'm passing to the builder so I can look at it later. Simply having a getter would make things easier.

Answer 1

I think there is nothing wrong per se to have getters in your builder class allowing to inspect which data was passed in. As Robert Harvey stated, getters are not part of the pattern, but sometimes a pragmatic solution is better than sticking to some doctrine. And there is not "the one and only correct way" to implement a pattern, design patterns always give you some degrees of freedom how to implement them.

However, you should be careful not to assign too many different responsibilities to that one class (and if a builder class has getters, this might be an indication for this). If your builder starts to become a repository for the input data, a validator for the data and the builder itself, then it has too many responsibilities, and you should consider to split it up into different classes.

Answer 2

I would certainly raise an eyebrow at getters in a builder, and it looks like a violation of the single-responsibility pattern.

It sounds like you need a separate validation component prior to the builder. So your chain of processing would be read the file, validate the params, and feed to the builder. The alternative (and this would be a good practise generally) is to code your final object such that it performs post-conditions upon construction, and throw an exception if it's been fed incorrect parameters. That's a good practise generally since you don't want to be able to construct such an object incorrectly, builder or no builder.

Answer 3

One significant use case for builders arises when you are provisioning the testing process. You frequently have to build up non-trivial data structures, many of which are immutable when they are passed into the code under test, and a builder is the natural pattern to use.

Testing setup consists of creating a builder and setting some starting properties. Individual tests then will mutate the builder in various directions until the point where the final data structure is built. Because the builder contents is cumulative, some steps in the process may need to know values set in earlier stages.

Short answer: if building a data structure takes more than one phase, then getters are a natural way to provide local knowledge in subsequent phases.

Answer 4

I would do factory methods in the object where non of the parameters can be null. That is where valadition should be. Those can throw an execption.

As for the builder, replace with abstract factory pattern and let it throw an execption. For loading, have a list of the factories. Try build the object and if it's no longer null and no exception thrown, return objects.

If that's too slow, make loading on its own thread and add to a list. Then have the loader pass the list to a class that does something.

Answer 5

Consider:

public abstract class Entity {
    public static abstract class Builder<T extends Entity, B extends Builder<T, B>> {
        private final T object;
        private final B thisBuilder;

        public Builder() {
            this.object = createObject();
            this.thisBuilder = thisBuilder();
        }

        protected abstract T createObject();
        protected abstract B thisBuilder();

        public T build() {
            return getObject();
        }

        protected T getObject() {
            return this.object;
        }
    }
}

The above superclass for business objects contains a Builder class that provides a way to use the data within business objects to initialise, but doesn't necessarily expose data via public accessors.

Here's a class that uses the Builder class:

public final class Address extends Entity {
    private String city;

    protected synchronized String getCity() {
        return this.city == null ? "" : this.city;
    }

    protected void setCity(String city) {
        this.city = city;
    }

    public static final class Builder extends Entity.Builder<Address, Builder> {
        @Override
        protected Builder thisBuilder() {
            return this;
        }

        @Override
        protected Address createObject() {
            return new Address();
        }

        public Builder withCity(String city) {
            getObject().setCity(city);
            return thisBuilder();
        }
    }
}

Now you can build an Address instance as follows:

Address address = new Address.Builder()
  .withCity( "Vancouver" )
  .build();

No data has been exposed outside of the package as no accessors are exposed within the Builder class. Also, no data has been duplicated between the Builder class and the object being built (as some other designs show).

A disadvantage to this approach is slight overlap of functionality. That is, the withXYZ methods of the builder delegate data transfer via the setXYZ accessors of the class to be built.

Inheritance without having to care about method call order is possible. For example:

public abstract class AbstractDate extends Entity {
    private Date date;

    protected synchronized Date getDate() {
        return this.date == null ? new Date() : this.date;
    }

    protected void setDate(Date date) {
        this.date = date;
    }

    protected static abstract class Builder
    <T extends AbstractDate, B extends Builder<T, B>> extends Entity.Builder<T, B> {
        public B withDate(int year, int month, int day) {
            getObject().setDate(createDate(year, month, day));
            return thisBuilder();
        }
    }
}

Aside, using a with prefix for method names can help with autocomplete.

Is it strange to have accessor methods inside a Builder? Rather, perhaps, they aren't necessary.