Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

When implementing the Builder Pattern, I often find myself confused with when to let building fail and I even manage to take different stands on the matter every few days.

First some explanation:

  • With failing early I mean that building an object should fail as soon as an invalid parameter is passed in. So inside the SomeObjectBuilder.
  • With failing late I mean that building an object only can fail on the build() call that implicitely calls a constructor of the object to be built.

Then some arguments:

  • In favor of failing late: A builder class should be no more than a class that simply holds values. Moreover, it leads to less code duplication.
  • In favor of failing early: A general approach in software programming is that you want to detect issues as early as possible and therefore the most logical place to check would be in the builder class' constructor, 'setters' and ultimately in the build method.

What is the general concensus about this?

share|improve this question
8  
I don't see any advantage to failing late. What someone says a builder class "should" be doesn't take precedence over good design, and catching bugs early is always better than catching bugs late. –  Doval May 28 at 11:50
3  
Another way to look at this is that the builder may not know what's valid data. Failing early in this case is more about failing as soon as you know there's an error. Not failing early, would be the builder returning a null object when there was a problem in build(). –  Chris May 28 at 18:46
    
If you don't add a way to issue a warning and offer means to fix within the builder there is no point in failing late. –  Mark Jun 2 at 12:22

4 Answers 4

up vote 23 down vote accepted

Let's look at the options, where we can place the validation code:

  1. Inside the setters in builder.
  2. Inside the build() method.
  3. Inside the constructed entity: it will be invoked in build() method when the entity is being created.

Option 1 allows us to detect problems earlier, but there can be complicated cases when we can validate input only having the full context, thus, doing at least part of validation in build() method. Thus, choosing option 1 will lead to inconsistent code with part of validation being done in one place and another part being done in other place.

Option 2 isn't significantly worse than option 1, because, usually, setters in builder are invoked right before the build(), especially, in fluent interfaces. Thus, it's still possible to detect a problem early enough in most cases. However, if the builder is not the only way to create an object, it will lead to duplication of validation code, because you'll need to have it everywhere where you create an object. The most logical solution in this case will be to put validation as close to created object as possible, that is, inside of it. And this is the option 3.

From SOLID point of view, putting validation in builder also violates SRP: the builder class already has responsibility of aggregating the data to construct an object. Validation is establishing contracts on its own internal state, it's a new responsibility to check the state of another object.

Thus, from my point of view, not only it's better to fail late from design perspective, but it's also better to fail inside the constructed entity, rather than in builder itself.

UPD: this comment reminded me of one more possibility, when validation inside the builder (option 1 or 2) makes sense. It does make sense if the builder has its own contracts on the objects it is creating. For example, assume that we have a builder that constructs a string with specific content, say, list of number ranges "1-2,3-4,5-6". This builder may have a method like "addRange(int min, int max)". The resulting string does not know anything about these numbers, neither it should have to know. The builder itself defines the format of the string and constraints on the numbers. Thus, the method addRange must validate the input numbers and throw an exception if max is less than min.

That said, the general rule will be to validate only the contracts defined by the builder itself.

share|improve this answer
    
I think that it's worth noting that while Option 1 may lead to "inconsistent" checking times, it can still be viewed as consistent if everything is "as early as possible." It's a little bit easier to make "as early as possible" more definite if the variant of the builder, the StepBuilder, is used instead. –  Joshua Taylor May 28 at 17:55
    
If a URI builder throws an exception if a null string is passed, is this a violation of SOLID? Rubbish –  Gusdor May 29 at 9:32
    
@Gusdor yes, if it throws an exception itself. However, from user point of view, all options look like an exception is thrown by a builder. –  Ivan Gammel May 29 at 17:11
    
So why not have a validate() that is called by build()? That way there is little duplication, consistency and no SRP violation. It also makes it possible to validate the data without attempting to build, and the validation is close to the creation. –  StellarVortex May 31 at 9:42
    
@StellarVortex in this case it will be validated twice - once in the builder.build(), and, if the data is valid and we proceed to constructor of object, in that constructor. –  Ivan Gammel Jun 2 at 12:10

Invalid values which are invalid because they are not tolerated should be made known immediately in my opinion. In other words, if you accept only positive numbers, and a negative number is passed in, there is no need to have to wait until build() is called. I wouldn't consider these the types of problems you would "expect" to have happen, since it is a prerequisite to calling the method to begin with. In other words, you would not likely depend on the failing of setting certain parameters. More likely you would presume the parameters are correct or you'd perform some check yourself.

However, for more complicated issues which aren't as easily validated may be better off being made known when you call build(). A good example of this might be using the connection information you provide to establish a connection to a database. In this case, while you technically could check for such conditions, it is no longer intuitive and it only complicates your code. As I see it, these are also the types of issues which might actually happen and that you cannot really anticipate until you try it. It's sort of the difference between matching a string with a regular expression to see if it could be parsed as an int and simply trying to parse it, handling any potential exceptions which may occur as a consequence.

I generally dislike throwing exceptions when setting parameters since it means having to catch any exception thrown, so I tend to favor validation in build(). So for this reason, I prefer using RuntimeException since again, errors in parameters passed should not generally happen.

However, this is more of a best practice than anything. I hope that answers your question.

share|improve this answer

Given that you use Java, consider authoritative and detailed guidance provided by Joshua Bloch in the article Creating and Destroying Java Objects (bold font in below quote is mine):

Like a constructor, a builder can impose invariants on its parameters. The build method can check these invariants. It is critical that they be checked after copying the parameters from the builder to the object, and that they be checked on the object fields rather than the builder fields (Item 39). If any invariants are violated, the build method should throw an IllegalStateException (Item 60). The exception's detail method should indicate which invariant is violated (Item 63).

Another way to impose invariants involving multiple parameters is to have setter methods take entire groups of parameters on which some invariant must hold. If the invariant isn't satisfied, the setter method throws an IllegalArgumentException. This has the advantage of detecting the invariant failure as soon as the invalid parameters are passed, instead of waiting for build to be invoked.

Note according to editor explanation on this article, "items" in above quote refer to rules presented in Effective Java, Second Edition.

The article doesn't dive deep into explaining why this is recommended, but if you think of it, the reasons are pretty apparent. Generic tip on understanding this is provided right there in the article, in the explanation how builder concept is connected to that of constructor - and class invariants are expected to be checked in constructor, not in any other code that may precede / prepare its invocation.

For a more concrete understanding for why checking invariants prior to invoking a build would be wrong, consider a popular example of CarBuilder. Builder methods may be invoked in an arbitrary order and as a result, one can't really know whether particular parameter is valid until the build.

Consider that sports car can't have more than 2 seats, how could one know if setSeats(4) is okay or not? It's only at the build when one can know for sure whether setSportsCar() was invoked or not, meaning whether to throw TooManySeatsException or not.

share|improve this answer

As far as I know, the general practice (not sure if there's consensus) is to fail as early as you can feasibly discover an error. This also makes it more difficult to unintentionally misuse your API.

If it is a trivial attribute that can be checked on input, such as a capacity or length which should be non-negative, then you're best to fail immediately. Holding off the error increases the distance between mistake and feedback, which makes it more difficult to find the source of the problem.

If you have the misfortune of being in a situation where the validity of an attribute depends on others, then you have two choices:

  • Require that both (or more) attributes be supplied simultaneously (i.e. single method invocation).
  • Test validity as soon as you know no more changes are incoming: when build() or so is called.

As with most things, this is a decision made in a context. If the context makes it awkward or complicated to fail early, a trade-off can be made to defer checks to a later time, but fail-fast should be the default.

share|improve this answer
    
So to summarize, you are saying that it is reasonable to validate as early as possible everything that could have been covered in an object/primitive type? Like unsigned, @NonNull, etc. –  skiwi May 28 at 12:57
2  
@skiwi Pretty much, yes. Domain checks, null checks, that sort of thing. I wouldn't advocate putting much more in it than that: builders are generally simple things. –  JvR May 28 at 15:33
    
It may be worth noting that if the validity of one parameter depends upon the value of another, one can only legitimately reject a parameter value if one knows that the other is "really" established. If it is permissible to set a parameter value multiple times [with the last setting taking precedence], then in some cases the most natural way to set up an object may be to set parameter X to a value which is invalid given the present value of Y, but before calling build() set Y to a value which would make X valid. –  supercat May 28 at 20:55
    
If e.g. one is building a Shape and the builder has WithLeft and WithRight properties, and one wishes to adjust a builder to construct an object in a different place, requiring that WithRight be called first when moving an object right, and WithLeft when moving it left, would add needless complexity compared with allowing WithLeft to set the left edge to the right of the old right edge provided that WithRight fixes the right edge before build is called. –  supercat May 28 at 20:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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