4

Using the Builder pattern there is always the question whether the fields do have a default value or not? I cannot find a reliable source where that's clearly defined...

The problem is readability: What is Car.Builder().build() returning? I always need to check the specific implementation of the Builder to see which default values are used. Shouldn't Builder be used to create complex objects which do not have a simple default state by definition?

An alternative would be to check whether all mandatory fields are set inside of the build() method:

fun build() : Car {
    return if (doors != null && hp != null) Car(doors, hp, color) // color can be null
    else throw IllegalArgumentException("Door count and HP is mandatory!")
}

...or is this considered bad practice?

mbo
  • 4,611
  • 2
  • 34
  • 54

2 Answers2

3

You didn't find a general answer to your question, because there is none. It depends on the context the builder is used in or the object's details that the builder is constructing.

Sometimes it makes perfect sense to return a default object, where all of its members are initialized to default values. But then this default object must be valid. E.g. to build a logger object, it would be valid to return a logger that logs to the console with a default formatting and default log level. Someone could instantly use it.

But sometimes a pure default object doesn't make sense. Creating a completely configured default HTTP client is not sensible, because it won't deliver the minimum expected behavior, since the target URL might be something unexpected. In this case you could write a constructor for the builder class that takes an URL as parameter (maybe some data object as well) and then preconfigure the client object with default values (e.g. default request header, default timeout, default buffer size, etc). This object then would satisfy the minimum usage expectations. Every subsequent call of a setter will then overwrite the default, where each setter should check for validity of the arguments before accepting them.

But you should always, when possible, try to use defaults over exceptions. When an object's construction requires mandatory information, then make this public by putting them in a constructor. This way you are safe that your object is always in a valid and useful state. In case of the HTTP client builder you could throw an exception if the URL is malformed, to give the developer a hint that his code that constructs the URL might have flaws. Maybe read Best practices for exceptions.

With the solution in mind to write constructors in order to collect all required parameters (that cannot be set to useful default values), your build() finalize method can and should always and at any time return a valid and useful object. This makes your builder convenient. (Otherwise the user of the builder would be forced to read documentation to know which setters to call. Everybody knows that you don't like to write documentations. Everybody knows that you don't like to read documentation before the use of some classes. Everybody feels the same).

BionicCode
  • 1
  • 4
  • 28
  • 44
  • Great explanation, thank you! No worries, I'll accept the answer as soon as some more people agree on this – mbo Jun 01 '19 at 12:24
2

I would not put default values into Car class, but instead put default values to concrete builders.

class SedanBuilder {
    var doors = 4
    var wheels = 4
    var driver = null // we have to set driver

    ... setters ...
    fun build() {

        return Car(wheels, doors, driver)
    }
}

and another builder can use another default values

class SchumachersCarBuilder {
    var doors = 4
    var wheels = 4
    var driver = Person("Michael")

    ... setters ...
    fun build() {

        return Car(wheels, doors, driver)
    }
}

And of course, you have to check all mandatory params inside Car constructor

Bukharov Sergey
  • 9,767
  • 5
  • 39
  • 54
  • Then we can remove the Exceptions from the `build()` or not? – mbo May 31 '19 at 15:14
  • Yes, probably you are right, we do not need to double the logic in 2 classes – Bukharov Sergey May 31 '19 at 15:39
  • If the attribute driver is a requirement and cannot be replaced by a default driver e.g. `DefaultPerson`, then better put this into the builder's constructor. This makes the use of the builder easier and more intuitive instead of introducing additional classes. Keep it simple. – BionicCode Jun 01 '19 at 08:28
  • @BionicCode you are probably right. But this is primitive domain model, and I just want to show, that it is better to keep something open in the Car, and have possibility to have different builders. – Bukharov Sergey Jun 01 '19 at 10:54
  • Everything is possible. We know. But not everything is suitable. In your example I have to know all the different builder types (or specializations). If you need a different default driver you would implement another builder (according to your pattern). In addition the first implementation returns an object where the driver is 'null'. Unexpected exception is thrown here. Not nice. This defeats the purpose of default values. Using the builder's constructor leaves everything 'open' as you said, but without having multiple builders just to satisfy different default values. That's too complex. – BionicCode Jun 01 '19 at 11:48
  • Your builder is responsible to check its own state before creating the result type. Each setter has to validate the passed in value before storing it. The builder should check for obvious violations like 'null' arguments and should not depend on the result type to implement null checks or in your case detects a possible negative number of doors. This ensures that the builder can return a safe and useful result object. So mandatory parameters should be validated by the builder. – BionicCode Jun 01 '19 at 11:56
  • Because the builder knows more about the context than the result class (`Car`in your example). If the context would be racing cars than the `Car` constructor will most likely not fail when passing in a `TruckEngine`. But the builder requires or expects some `RacingCarEngine` and should validate the Èngine` parameter before constructing the actual `Car`. – BionicCode Jun 01 '19 at 12:04
  • Since writing code that doesn't throw exceptions should be the goal, 'null' is never a useful default value. – BionicCode Jun 01 '19 at 12:08