scientistproject/Scientist.net

Provide a fluent interface

davezych opened this issue · 8 comments

Fluent interfaces are fairly standard in .NET APIs now. We set this up in Shience and it worked well. We chained our calls like this:

var userHasAccess = Scientist.Science<bool>("experiment-name")
    .Use(() => IsCollaborator(user))
    .Try(() => HasAccess(user))
    .WithContext(new { userId = user.Id })
    .Execute(); //Or ExecuteAsync()

To keep consistent with the way the Scientist api works now, we could do something like:

return Scientist.Science<bool>("experiment-name", experiment =>
    {
        experiment.Use(() => IsCollaborator(user))
                  .Try(() => HasAccess(user))
                  .WithContext(new { userId = user.Id })
    });

Makes for a cleaner api.

I would argue that in addition to cleanliness it makes it easier to understand what's going on

I'm a little divided on this. I think in some cases, fluent interfaces can be confusing. For example, take the following class:

public class Shape {
  public void Rotate() {...}
}

When you see this:

var shape = new Shape();
shape.Rotate();

You rightly expect that Rotate mutates the instance of Shape.

But if you define Rotate like this:

public class Shape {
  public Shape Rotate() {...}
}

And saw this:

var shape = new Shape();
var rotated = shape.Rotate();

Intuitively, I would expect that shape is the original Shape and rotated is the rotated shape. In other words, when I see a void method I expect the method to mutate the object. But if I see a method that returns the same type that the method belongs to, I expect the method to not mutate the object but return a new instance.

So to finish my thought, the fact that we mutate the experiment instance makes me think the API of Experiment should not be fluent.

And to continue my stream of consciousness, I've never been that much of a fan of fluent interfaces because I think they've been overused and often make code harder to understand in an effort to make it more readable.

That being said, one area that fluent interfaces often shine is in cases where you have a builder pattern. And as I think about this, Scientist seems to fit well with that pattern as we are building up an experiment.

I've tried to stay close to the Ruby API because I want the documentation for Scientist to be easily translatable to the version we have here. At the same time, I want to make sure we stay true to C# idioms. I feel what we have today strikes a nice balance.

Also, I don't like to make major API decisions without a bit of real-world usage data. Let's play around with the current approach. If you have some co-workers who haven't used this, maybe have them try it out as an ad-hoc usability test and report back any comments.

I'll play around with it too.

So, tl;dr, I'm open to changing the API, but I want to play with the current one to get a sense of where its weak points are. I welcome more input on this. 😄

@davezych it just occurred to me we could have an overload of Scientist.Science that returns an object we build on that would give us the Shience API. I don't want two different approaches to building up an experiment, but if you wouldn't mind, perhaps implement the Shience approach in a branch, push it up as a PR. That way, rather than debate theoreticals, I can play around with both implementations and get a feel for them both. What do you think?

@haacked sounds good. I'll do that over the next few days.

there are two way of implementing a fluent interface. one is with mutating the state like the StringBuilder class does it. another, and in my opinion much cleaner approach, is to make or assume the type to be immutable and instead of mutating it, returning a new one with the changed properties.

example:

var shape = new Shape(); 
var rotatedShape = shape.Rotate(Math.Pi); 
Assert.That(shape, Is.Not.EqualTo(rotatedShape)); 

Closing per request.