7

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.

  • How many methods would be on your builder? In other words, how many setters would you have? – Robert Harvey May 15 '16 at 22:04
  • @RobertHarvey I have 10 fields that I'm setting. – codebreaker May 15 '16 at 22:06
  • 1
    Not an exact duplicate, but has a definitive answer to your question, if you read it carefully: programmers.stackexchange.com/q/241309 – Robert Harvey May 15 '16 at 22:17
  • 1
    See the post I just linked. As I suspected, the right way to deal with this is to throw an exception. – Robert Harvey May 15 '16 at 22:18
  • 1
    You never answered my question about telescoping constructors. I don't think you need a builder at all; you just need an ordinary DTO with getters and setters. – Robert Harvey May 16 '16 at 1:44
8

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 to 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.

  • 2
    Meh, getters are not really part of the pattern, and if your builder has them, I suspect that it already has too many responsibilities (in addition to now becoming a leaky abstraction). That said, I've never had any objections to DTO's, even ones having validation mechanisms. – Robert Harvey May 15 '16 at 22:20
  • Just to be clear, the builder itself isn't doing the validation. My file-parsing thing is totally decoupled from the builder, it's just that it would be useful to be able to inspect the data in the builder in certain situations (like this one). – codebreaker May 16 '16 at 0:14
  • @RobertHarvey: see my edit. – Doc Brown May 16 '16 at 8:30
  • @DocBrown: I have no issue with the getters. My issue is the Builder pattern. For the life of me, I can't imagine why anyone would ever use one, other than possibly for convenience. There isn't anything you can do in a builder that you can't do better using a constructor, and if your constructor has too many parameters, that's a design problem. The problem of Telescoping Constructors is solved in C# using optional parameters and parameter arrays. Perhaps the examples provided in Wikipedia and other sites are just too trivial for me to see the benefit. – Robert Harvey May 16 '16 at 14:39
  • 1
    @RobertHarvey: I remember a situation where the created object was in component A, the builder in component B, and the builder was using other objects only available in the context of B. So moving the builder logic to the constructor was simply not possible because it would create a cyclic dependency between A and B. – Doc Brown May 16 '16 at 16:23
1

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.

  • 1
    A class trying to build an object should absolutely look for and report problems. Going to one class to build an object and on failure going to another class for a report of problems would be ridiculous. – gnasher729 May 16 '16 at 10:19
0

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.

0

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.

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.