Note. It’s a “spin-off” from my previous question. Not a duplicate — it focuses on a different topic
I got to know builders from Bloch’s Effective Java. However, I made two changes to his design
-
(no biggie) I added a static accessor for the nested builder. Since the builder still has to be
public, direct referencing is still possible -
(yes biggie?) Made the buildable class’s only (
private) constructor accept its own builder and nothing else. That way, I would avoid a big parameter list. Since builders are typically used when there are multiple parameters to pass, it’s a feel-good change that allows me to neatly passthisin thebuild()method and decompose the builder later in the constructor
Some say, it turns my builder into a service locator which is a known anti-pattern. They argue it hides the class’s actual dependencies. I don’t see a problem with this as the constructor is not part the class’s API, so the only party it may be be “hiding” anything from is its nested builder, the constructor’s only client
So what do you think?
- Does that change really turn my builder into a service locator?
- More broadly, is what I did bad?
public class MyClass {
private final String someArg;
// other fields
private MyClass(Builder builder) {
this.someArg = builder.someArg;
// ...
}
public static Builder builder() {
return new Builder();
}
// business logic
public static class Builder {
private String someArg;
public Builder setSomeArg(String someArg) {
this.someArg = someArg;
return this;
}
// other setters
public MyClass build() {
return new MyClass(this);
}
// some instantiating client
MyClass.builder()
.setSomeArg(someArg)
// other setters if necessary
.build();
UPD: Hm, it seems Bloch also passed this in the build() (instead of decomposing the builder and passing “real” args), so item 2 is not a design change after all. It’s strange, ’cause I remember it differently. I wonder whether my memory is failing me or it changed between different editions of the book (probably, the former)
21
I am not going into this Service Locator debate, but looking at Bloch’s original example, which I found here, it seems your “design change” isn’t one. Passing the builder as a constructor parameter to the builded class’ private constructor was exactly Bloch’s suggestion.
The only change which remains is the extra static builder() method, but this is more a matter of taste, since
MyClass.builder();
saves only a few characters of typing when compared to
new MyClass.Builder();
and that’s all it does.
So whatever you showed us here, I think it is not better or worse than Bloch’s original Builder pattern, because it is simply the same.
Seems like we are voting on this 🙂
- Yeah I think it’s like a service locator. Essentially you have the same pattern
…
builder.AddDependency(someDependency)
myclass(Builder b)
{
dependency = b.FindDependency();
}
vs
diContainer.AddDependency(someDependency)
myclass(DiContainer c)
{
dependency = c.FindDependency();
}
- Yeah I think it’s bad.
- You couple the two classes so you can’t add different builders or subclasses without jumping through hoops
- Users can’t not use the builder, say for example it doesn’t play well with their DI framework
- You could just make a normal builder without adding these problems, there would be no downside
- You could just override constructors to get around your feelings about needing a builder anyway
- You have to use nested classes to get the private stuff working
I’m not a fan of builders, they can be cool if you have some extremely complex setup and you enforce all the methods through clever interfaces. But it’s a lot of work for very little gain. They aren’t needed just because you have a lot of construction parameters and they dont help reduce the construction parameters.
You don’t seem to have some compelling problem which is improved by this solution, people will find it easier if you just do things the “normal way” rather than having to work out your builder.
Having said that, if the “normal” way is to use an Options constructor, then fill your boots
myclass = new MyClass(o=> o.UseDependency(someDependency));
I think this fails for the same reasons, but it seems popular these days.
6
The solution from description is self-explanatory, the question from the subject is rhetorical. If you don’t like answers of rhetorical questions end reading here.
NOTE: don’t run following code snippet, it is just to depict the builder usage.
public class MyClass {
private final String someArg;
// other fields
private MyClass(Builder builder) {
this.someArg = builder.someArg;
builder.build(); // this results in out of memory exception
}
public static Builder builder() {
return new Builder();
}
// business logic
public class Builder {
private String someArg;
public Builder setSomeArg(String someArg) {
this.someArg = someArg;
return this;
}
// other setters
public MyClass build() {
return new MyClass(this);
}
}
}
Passing a builder to a constructor is usual business, calling the builder’s factory method build (builder.build()) from the constructor is what must be avoided or to quote the title it is “bad”. If the build method of the builder parameter isn’t called from the constructor then the builder parameter is just a parameter object. I guess the answer is it is “bad” if you make it to be “bad”.
Service locator design pattern or look up instances with a set of particularities.
Builder design pattern or encapsulate the instance construct to take complex logic out of the constructor.
This…
private MyClass(Builder builder) {
this.someArg = builder.someArg;
// ...
}
…is parameter object design pattern, don’t get confused about the naming, the argument could have been of type Visitor and called visitor that wouldn’t have made it a visitor design pattern implementation.
Is it bad to pass builders as constructor arguments?
When the construct logic is plain the sole purpose of passing a parameter object to the constructor is to decouple the constructor’s number of arguments from the multitude of possible arguments by encapsulating them. When the construct logic is complex encapsulate the logic in its own encapsulation (e.g. the factory method of builder design pattern).
1
A builder is supposed to create an instance of A or a subclass of A. Your pattern prevents that. Especially the builder may be defined to return an instance of a subclass of some abstract class A.
What a builder should do: 1. Examine its arguments. 2. Based on the arguments, decide which class to build an instance of. 3. Based on the arguments, create an instance of the class it chose and call its constructor with these arguments. The constructor of the instance that is created should have no knowledge of the builder at all.
The rule that a constructor shouldn’t have many arguments is because many arguments make it possible that 99 constructor calls are correct, and the 100th contains a stupid bug passing the wrong argument. Your builder class has only one constructor call, so that argument goes away.
2