Should everything be buildable?

Question

You have some class that performs a certain job

public class MyClass {
    public MyClass() {
       // ...
    }
    // ...
}

Then, a spec change comes around. The responsibility of the class is the same, but you realize it needs an extra parameter

    public MyClass(String someArg)

But you have existing clients that call the no-arg, so you make it call the new constructor with a default parameter value. The init code that used to be in the no-arg is now moved to the constructor with args

    public MyClass() {
        this(null);
    }

Then, a new spec change. The class now has to have ServiceA to do that

    public MyClass(String someArg, ServicA serviceA)

But, again, we have clients that call the old constructors. We move the init logic and provide some default value for the new parameter once again. Unlike someArg, it's a required dependency so MyClass will have to come up with some default ServiceA implementation. Passing nulls will no longer do

    public MyClass(String someArg) {
        this(someArg, new ServiceAImpl());
    }

Some clients don't need to pass someArg but they still need to pass the required ServiceA. Those clients will have to pass null explicitly

    MyClass myClass = new MyClass(null, new BellsAndWhistlesServiceA());

I can go on. But the problem is already evident

It's going to snowball into a mess

1. The class now manages its defaults. It's a DI responsibility. No class should ever manage its defaults as it in effect scatters configuration all over the codebase making configuration changes difficult
2. Ugly, risky calls like this one are now a matter of time

    new MyClass("123", new ServiceAImpl2(), null, null, null, new WhateverElse(), null, 123, null)

Builders address that. A buildable class has only one declared dependency – on its builder. Now and forever. You can add optional dependencies through that builder easily. No incompatible changes to the class's API

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();

It's not without drawbacks. It's still unclear how you add required dependencies. You either have to i) add that to the builder constructor and get compilation errors in clients; ii) or add "mandatory" setters and get runtime exceptions (because clients would be calling build() on builders that are now considered invalid)

And the issue with "distributed" DI is still not resolved. If builders manage their respective class's defaults, is it any better? No, it's not

Still, it may still be a good idea to make all newly created classes buildable by default. Even if they don't (yet) require any inputs at all for their initialization

So what do you think? Should every newly created type be buildable by default (at least, in Java)?

Answer 1

This might be an XY problem. Making things "buildable" isn't the issue. All those optional constructor parameters are the real problem. Lumping a builder on top of this mess only adds a layer of frosting on top of a mud cake.

---

I'm always weary of optional dependencies in a class constructor. I like constructors that do two things:

1. Declare the minimal dependencies necessary to use the object.
2. Ensure the object is fully usable.

Optional dependencies in a constructor make me wonder why an argument is needed. When do I need this? Now? Later? Can I pass null now? What if I call a method later and the null suddenly becomes a problem? How will I debug the failure? Is there a default implementation, and if so, what does it do? What side effects does it have? So many things run through my head.

I would also question why MyClass is being initialized all over the place instead of being registered in a DI container and passed around like every other common dependency. This might not be solvable, depending on the structure of the existing code base.

I see a number of ways forward. I'm not sure if they will fix the design of this class, but it should improve the design enough so you can see the next improvement that can be made.

• Identify the methods where the optional dependency is used. If it is not used prolifically, consider making this a method parameter instead of a constructor parameter. This won't work if you need to guarantee the dependency is the same object upon each method invocation.
• Utilize the Null Object Pattern as the default implementation for optional dependencies, and document this in comments for the constructor.
• Design the class for inheritance so a concrete sub type declares the additional dependencies, which will be required. This allows for selective specialization of MyClass without impacting existing clients. Be mindful of the Liskov Substitution Principle so passing a sub type to a method requiring MyClass doesn't result in unexpected behavior.
• Consider splitting MyClass into completely separate classes with no inheritance relationship to one another. Perhaps the use of this class has diverged enough that copying and pasting code will result in fewer maintenance headaches simply because these things can evolve on their own without affecting each other.
  • Common logic can be moved into other classes that are shared between the two new classes. Code reuse is the issue to solve, in this case.
  • You can introduce interfaces for methods that do not initialize these classes, but still need to use them provided the interface is cohesive with all the sub types.

The one thing I strongly caution against is using a "builder" as described in the question. A builder isn't a builder because of what you name it. It is a builder because of how it is used, and the code example is not using it as a builder. What you describe in the question is a Service Locator, a known anti-pattern for this use case. It only serves to hide the true dependencies of MyClass.

I'm not sure how to fix this, but it does seem like this class is doing too much, and has become a dumping ground for logic. If this is true, then MyClass could be on the fast track to becoming a God Object. I would start by trying to split this thing up into additional classes relevant to a use case.

Answer 2

Both builder pattern (unless it is a complex multistage one) and most DI containers are examples of late binding. They do their compliance checks in runtime. They deprive both service and clients of the most important benefit of a strong type system - design time checks.

IMO backward compatibility that occasionally fails in runtime is worse than no backward compatibility.

Answer 3

Suggestions

1. Utilize versioning so you don't have to keep making these backwards compatible changes.

   // version 1.0
   public MyClass()
   // version 2.0
   public MyClass(String someArg, ServicA serviceA)
   

2. Use a method other than the constructor to inject dependencies.

   // version 1.0
   public MyClass()
   
   // version 1.1
   public MyClass() {
        this(null);
   }
   public MyClass(String someArg)
   public SetServicA(ServicA serviceA)
   

3. Maybe use stand-alone functions instead of a class.
4. See if there is another programming language that handles this kind of problem better. With Go I don't run into this anymore.