38

When using method chaining like:

var car = new Car().OfBrand(Brand.Ford).OfModel(12345).PaintedIn(Color.Silver).Create();

there may be two approaches:

  • Reuse the same object, like this:

    public Car PaintedIn(Color color)
    {
        this.Color = color;
        return this;
    }
    
  • Create a new object of type Car at every step, like this:

    public Car PaintedIn(Color color)
    {
        var car = new Car(this); // Clone the current object.
        car.Color = color; // Assign the values to the clone, not the original object.
        return car;
    }
    

Is the first one wrong or it's rather a personal choice of the developer?


I believe that he first approach may quickly cause the intuitive/misleading code. Example:

// Create a car with neither color, nor model.
var mercedes = new Car().OfBrand(Brand.MercedesBenz).PaintedIn(NeutralColor);

// Create several cars based on the neutral car.
var yellowCar = mercedes.PaintedIn(Color.Yellow).Create();
var specificModel = mercedes.OfModel(99).Create();

// Would `specificModel` car be yellow or of neutral color? How would you guess that if
// `yellowCar` were in a separate method called somewhere else in code?

Any thoughts?

svick
  • 10,137
  • 1
  • 39
  • 53

8 Answers8

41

I'd put the fluent api to it's own "builder" class seperate from the object it is creating. That way, if the client doesn't want to use the fluent api you can still use it manually and it doesn't pollute the domain object (adhering to single responsibility principle). In this case the following would be created:

  • Car which is the domain object
  • CarBuilder which holds the fluent API

The usage would be like this:

var car = CarBuilder.BuildCar()
    .OfBrand(Brand.Ford)
    .OfModel(12345)
    .PaintedIn(Color.Silver)
    .Build();

The CarBuilder class would look like this (I'm using C# naming convention here):

public class CarBuilder {

    private Car _car;

    /// Constructor
    public CarBuilder() {
        _car = new Car();
        SetDefaults();
    }

    private void SetDefaults() {
        this.OfBrand(Brand.Ford);
          // you can continue the chaining for 
          // other default values
    }

    /// Starts an instance of the car builder to 
    /// build a new car with default values.
    public static CarBuilder BuildCar() {
        return new CarBuilder();
    }

    /// Sets the brand
    public CarBuilder OfBrand(Brand brand) {
        _car.SetBrand(brand);
        return this;
    }

    // continue with OfModel(...), PaintedIn(...), and so on...
    // that returns "this" to allow method chaining

    /// Returns the built car
    public Car Build() {
        return _car;
    }

}

Note that this class will not be thread safe (each thread will need it's own CarBuilder instance). Also note that, even though fluent api is a really cool concept, it probably is overkill for the purpose of creating simple domain objects.

This deal is more useful if you're creating an API for something much more abstract and has more complex set up and execution, which is why it works great in unit testing and DI frameworks. You can see some other examples under the Java section of the wikipedia Fluent Interface article with persistance, date handling and mock objects.


EDIT:

As noted from the comments; you could make the Builder class a static inner class (inside Car) and Car could be made immutable. This example of letting Car be immutable seems a bit silly; but in a more complex system, where you absolutely don't want to change the contents of the object that is built, you might want to do it.

Below is one example of how to do both the static inner class and how to handle an immutable object creation that it builts:

// the class that represents the immutable object
public class ImmutableWriter {

    // immutable variables
    private int _times; private string _write;

    // the "complex" constructor
    public ImmutableWriter(int times, string write) {
        _times = times;
        _write = write;
    }

    public void Perform() {
        for (int i = 0; i < _times; i++) Console.Write(_write + " ");
    }

    // static inner builder of the immutable object
    protected static class ImmutableWriterBuilder {

        // the variables needed to construct the immutable object
        private int _ii = 0; private string _is = String.Empty;

        public void Times(int i) { _ii = i; }

        public void Write(string s) { _is = s; }

        // The stuff is all built here
        public ImmutableWriter Build() {
            return new ImmutableWriter(_ii, _is);
        }

    }

    // factory method to get the builder
    public static ImmutableWriterBuilder GetBuilder() {
        return new ImmutableWriterBuilder();
    }
}

The usage would be the following:

var writer = ImmutableWriter
                .GetBuilder()
                .Write("peanut butter jelly time")
                .Times(2)
                .Build();

writer.Perform();
// console writes: peanut butter jelly time peanut butter jelly time 

Edit 2: Pete in the comments made a blog post about using builders with lambda functions in the context of writing unit tests with complex domain objects. It is an interesting alternative to make the builder a bit more expressive.

In the case of CarBuilder you need to have this method instead:

public static Car Build(Action<CarBuilder> buildAction = null) {
    var carBuilder = new CarBuilder();
    if (buildAction != null) buildAction(carBuilder);
    return carBuilder._car;
}

Which can be used as this:

Car c = CarBuilder
    .Build(car => 
        car.OfBrand(Brand.Ford)
           .OfModel(12345)
           .PaintedIn(Color.Silver);
Spoike
  • 14,771
10

That depends.

Is your Car an Entity or a Value object? If the car is an entity, then object identity is of importance, so you should return the same reference. If the object is a value object, it should be immutable, meaning the only way is to return a new instance every time.

An example of the latter would be DateTime class in .NET which is a value object.

var date1 = new DateTime(2012,1,1);
var date2 = date1.AddDays(1);
// date2 now refers to Jan 2., while date1 remains unchanged at Jan 1.

However if the model is an entity, I like Spoike's answer on using a builder class to build you object. In other words, that example you gave only makes sense IMHO if the Car is a value object.

Pete
  • 9,016
6

Create a separate static inner builder.

Use normal constructor arguments for required parameters. And fluent api for optional.

Don't create a new object when setting colour, unless you rename the method NewCarInColour or something similar.

I would do something like this witht he brand as required and the rest optional (this is java, but yours looks like javascript, but pretty sure they are interchangeable with a bit of nit picking) :

Car yellowMercedes = new Car.Builder(Brand.MercedesBenz).PaintedIn(Color.Yellow).create();

Car specificYellowModel =new Car.Builder(Brand.MercedesBenz).WithModel(99).PaintedIn(Color.Yellow).create();
NimChimpsky
  • 4,670
4

The most important thing is that whatever the decision you choose, it is clearly stated in the method name and/or comment.

There is no standard, sometimes the method will return a new object (most of the String methods do so) or will return this object for chaining purpose or for memory efficiency).

I once designed a 3D Vector object and for every math operation I had both methods implemented. For instant the scale method :

Vector3D scaleLocal(float factor){
    this.x *= factor; 
    this.y *= factor; 
    this.z *= factor; 
    return this;
}

Vector3D scale(float factor){
    Vector3D that = new Vector3D(this); // clone this vector
    return that.scaleLocal(factor);
}
XGouchet
  • 844
  • 7
  • 14
3

I see a few problems here that I think might be confusing... Your first line in the question:

var car = new Car().OfBrand(Brand.Ford).OfModel(12345).PaintedIn(Color.Silver).Create();

You're calling a constructor (new) and a create method... A create() method would almost always be a static method or a builder method, and the compiler should catch it in a warning or error to let you know, either way, this syntax is either wrong or has some terrible names. But later on, you don't use both, so let's look at that.

// Create a car with neither color, nor model.
var mercedes = new Car().OfBrand(Brand.MercedesBenz).PaintedIn(NeutralColor);

// Create several cars based on the neutral car.
var yellowCar = mercedes.PaintedIn(Color.Yellow).Create();
var specificModel = mercedes.OfModel(99).Create();

Again with the create though, just not with a new constructor. Thing is, I think you're looking for a copy() method instead. So if that's the case, and it's just a poor name, let's look at one thing... you call mercedes.Paintedin(Color.Yellow).Copy() - It should be easy to look at that and tell it's being 'painted' before being copied - just a normal flow of logic, to me. So put the copy first.

var yellowCar = mercedes.Copy().PaintedIn(Color.Yellow)

to me, it is easy to see there that you are painting the copy, making your yellow car.

Drake Clarris
  • 401
  • 3
  • 9
1

The first approach does have the drawback you mention, but as long as you make it clear in the docs any half-competent coder shouldn't have problems. All of the method-chaining code I've personally worked with has worked this way.

The second approach obviously has the drawback of being more work. You also have to decide whether the copies you return are going to do shallow or deep copies: which is best may vary from class to class or method to method, so you'll either be introducing inconsistency or compromising on the best behaviour. It's worth noting that this is the only option for immutable objects, like strings.

Whatever you do, don't mix and match within the same class!

vaughandroid
  • 7,609
1

I'd rather think just like the "Extension Methods" mechanism.

public Car PaintedIn(this Car car, Color color)
{
    car.Color = color;
    return car;
}
Amir Karimi
  • 1,232
0

This is a variation on the above methods. The differences are that there are static methods on the Car class that match the method names on the Builder, so you don't need to explicitly create a Builder:

Car car = Car.builder().ofBrand(Brand.Ford).ofColor("Green")...

You can use the same method names that you use on the chained builder calls:

Car car = Car.ofBrand(Brand.Ford).ofColor("Green")...

Also, there is a .copy() method on the class that returns a builder populated with all the values from the current instance, so you can create a variation on a theme:

Car red = car.copy().paintedIn("Red").build();

Finally, the .build() method of the builder checks that all required values have been provided and throws if any are missing. It might be preferable to require some values on the constructor of the builder and allow the rest to be optional; in that case, you'd want one of the patterns in the other answers.

public enum Brand {
    Ford, Chrysler, GM, Honda, Toyota, Mercedes, BMW, Lexis, Tesla;
}

public class Car {
    private final Brand brand;
    private final int model;
    private final String color;

    public Car(Brand brand, int model, String color) {
        this.brand = brand;
        this.model = model;
        this.color = color;
    }

    public Brand getBrand() {
        return brand;
    }

    public int getModel() {
        return model;
    }

    public String getColor() {
        return color;
    }

    @Override public String toString() {
        return brand + " " + model + " " + color;
    }

    public Builder copy() {
        Builder builder = new Builder();
        builder.brand = brand;
        builder.model = model;
        builder.color = color;
        return builder;
    }

    public static Builder ofBrand(Brand brand) {
        Builder builder = new Builder();
        builder.brand = brand;
        return builder;
    }

    public static Builder ofModel(int model) {
        Builder builder = new Builder();
        builder.model = model;
        return builder;
    }

    public static Builder paintedIn(String color) {
        Builder builder = new Builder();
        builder.color = color;
        return builder;
    }

    public static class Builder {
        private Brand brand = null;
        private Integer model = null;
        private String color = null;

        public Builder ofBrand(Brand brand) {
            this.brand = brand;
            return this;
        }

        public Builder ofModel(int model) {
            this.model = model;
            return this;
        }

        public Builder paintedIn(String color) {
            this.color = color;
            return this;
        }

        public Car build() {
            if (brand == null) throw new IllegalArgumentException("no brand");
            if (model == null) throw new IllegalArgumentException("no model");
            if (color == null) throw new IllegalArgumentException("no color");
            return new Car(brand, model, color);
        }
    }
}
David Conrad
  • 864
  • 8
  • 9