Question

I recently came up to an interesting question, what should fluent methods return? Should they change state of current object or create a brand new one with new state?

In case this short description is not very intuitive here's an (unfortunaltely) lengthy example. It is a calculator. It performs very heavy calculations and that's why he returns results via async callback:

public interface ICalculator {
    // because calcualations are too lengthy and run in separate thread
    // these methods do not return values directly, but do a callback
    // defined in IFluentParams
    void Add(); 
    void Mult();
    // ... and so on
}

So, here's a fluent interface which sets parameters and callbacks:

public interface IFluentParams {
    IFluentParams WithA(int a);
    IFluentParams WithB(int b);
    IFluentParams WithReturnMethod(Action<int> callback);
    ICalculator GetCalculator();
}

I have two interesting options for this interface implementation. I will show both of them and then I'll write what I find good and bad each of them.

So, first is a usual one, which returns this:

public class FluentThisCalc : IFluentParams {
    private int? _a;
    private int? _b;
    private Action<int> _callback;

    public IFluentParams WithA(int a) {
        _a = a;
        return this;
    }

    public IFluentParams WithB(int b) {
        _b = b;
        return this;
    }

    public IFluentParams WithReturnMethod(Action<int> callback) {
        _callback = callback;
        return this;
    }

    public ICalculator GetCalculator() {
        Validate();
        return new Calculator(_a, _b);
    }

    private void Validate() {
        if (!_a.HasValue)
            throw new ArgumentException("a");
        if (!_b.HasValue)
            throw new ArgumentException("bs");
    }
}

Second version is more complicated, it returns a new object on each change in state:

public class FluentNewCalc : IFluentParams {
    // internal structure with all data
    private struct Data {
        public int? A;
        public int? B;
        public Action<int> Callback;

        // good - data logic stays with data
        public void Validate() {
            if (!A.HasValue)
                throw new ArgumentException("a");
            if (!B.HasValue)
                throw new ArgumentException("b");
        }
    }

    private Data _data;

    public FluentNewCalc() {
    }

    // used only internally
    private FluentNewCalc(Data data) {
        _data = data;
    }

    public IFluentParams WithA(int a) {
        _data.A = a;
        return new FluentNewCalc(_data);
    }

    public IFluentParams WithB(int b) {
        _data.B = b;
        return new FluentNewCalc(_data);
    }

    public IFluentParams WithReturnMethod(Action<int> callback) {
        _data.Callback = callback;
        return new FluentNewCalc(_data);
    }

    public ICalculator GetCalculator() {
        Validate();
        return new Calculator(_data.A, _data.B);
    }

    private void Validate() {
        _data.Validate();
    }
}

How do they compare:

Pro first (this) version:

  • easier and shorter

  • commonly used

  • seems to be more memory-efficient

  • what else?

Pro second (new) version:

  • stores data in separate container, allows to separate data logic and all handling

  • allows us to easily fix part of data and then fill in other data and handle it separately. Take a look:

        var data = new FluentNewCalc()
            .WithA(1);
    
        Parallel.ForEach(new[] {1, 2, 3, 4, 5, 6, 7, 8}, b => {
            var dt = data
                .WithB(b)
                .WithReturnMethod(res => {/* some tricky actions */});
    
            // now, I have another data object for each value of b, 
            // and they have different callbacks.
            // if I were to do it with first version, I would have to create each 
            // and every data object from scratch
            var calc = dt.GetCalculator();
            calc.Add();
        });
    

What could be even better in second version?

  • I could implement WithXXX method like this:

    public IFluentParams WithXXX(int xxx) {
        var data = _data;
        data.XXX = xxx;
        return new FluentNewCalc(data);
    }
    

    and make _data readonly (i.e. immutable) which some smart people say is good.

So the question is, which way do you think is better and why? P.S. I used c# but the could well apply to java.

Was it helpful?

Solution

When I am trying to answer such a question in my application design I always think about what a person using my code in his application would expect.

Take, for instace, the C# DateTime type. It is a struct and therefore immutable. When you ask for

var today = DateTime.Now;
var tomorrow = today.AddDays(1);

what would you expect if you didn't know that DateTime is immutable? I would not expect that today is suddenly tomorrow, that would be chaos.

As for your example, I would imagine that numbers are being processed using only one instance of the calculator, unless I decide otherwise. It makes sense, right? When I am writing an equation, I don't write each expression on a new line. I write it all together along with a result and then I jump to the next line in order to separate concerns.

So

var calc = new Calculator(1);
calc.Add(1);
calc.PrintCurrentValue(); // imaginary method for printing of a current value of equation

makes perfect sense to me.

OTHER TIPS

I tend to assume fluent methods will return this. However, you raise a good point regarding mutability that caught me out when testing. Sort of using your example, I could do something like:

var calc = new Calculator(0);
var newCalc = calc.Add(1).Add(2).Mult(3);
var result = calc.Add(1);

When reading the code, I think many people would assume result would be 1 as they'd see calc + 1. Of cause with a mutable fluent system, the answer would be different as the Add(1).Add(2).Mult(3) would be applied.

Immutable fluent systems are harder to implement though, requiring more complex code. It seems a highly subjective thing as to whether the immutability benefit outweighs the work required to implement them.

Were it not for type inference, one could "get the best of both worlds" by implementing not just an immutable FluentThing class defined in the API, but another, mutable, FluentThingInternalUseOnly which supported widening conversion to FluentThing. The Fluent members on FluentThing would construct a new instance of FluentThingInternalUseOnly and have that latter type as their return type; the members of FluentThingInternalUseOnly would operate on, and return, this.

This, if one said FluentThing newThing = oldFluentThing.WithThis(4).WithThat(3).WithOther(57);, the WithThis method would construct a new FluentThingInternalUseOnly. That same instance would be modified and returned by WithThat and WithOther; the data from it it would then be copied to a new FluentThing whose reference would be stored in newThing.

The major problem with this approach is that if someone says dim newThing = oldFluentThing.WithThis(3);, then newThing wouldn't hold a reference to an immutable FluentThing, but a mutable FluentThingInternalUseOnly, and that thing would have no way of knowing that a reference to it had been persisted.

Conceptually, what's needed is a way of having FluentThingInternalUseOnly be sufficiently public that it can be used as a return type from a public function, but not so public as to permit outside code from declaring variables of its type. Unfortunately I don't know any way to do this, though perhaps some trick involving Obsolete() tags might be possible.

Otherwise, if the objects being acted upon are complicated but the operations are simple, the best one may be able to do is have the fluent interface methods return an object which holds a reference to the object upon which it was called along with information about what should be done to that object [chaining fluent methods would effectively build a linked list] and a lazily-evaluated reference to an object upon which all appropriate changes had been applied. If one called newThing = myThing.WithBar(3).WithBoz(9).WithBam(42), a new wrapper object would be created at each step of the way, and the first attempt to use newThing as a thing would have to construct a Thing instance with three changes applied to it, but the original myThing would be untouched, and it would only be necessary to make one new instance of Thing rather than three.

I guess it would all depend on your usecase.

Most of the time when I use a Builder it is for a single-thread to manipulate mutable data. Therefore, returning this is preferred since there isn't the extra overhead and memory of returning new instances everywhere.

However, many of my Builders have a copy() method that returns a new instance with the current same values for the times when I need to support your "Pro second" use cases

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top