3

I'm using the builder pattern, have extract repeated code into a 'helper' class but there's one aspect of repeated code I'm still not happy with.

The builder pattern allows one to chain implementation code like this:

Car car = new CarBuilder().Wheels(4).Convertible().Build();

Each of the methods CarBuilder, Wheels and Convertible return the same instance of the builder class (return this) and the Build method return the newly-instantiated Car.

Here's my attempt at a generic builder class:

public class Builder<T> where T : class 
{
    private Func<T, T> func;

    protected void SetInstantiator(Func<T, T> f) => this.func = f;

    protected void Chain(Action<T> action)
    {
        this.ChainFunc(action);
    }

    private ChainFunc(Action<T> action)
    {
        // SNIPPED
    }

    protected T Instantiate() => this.func(null);
}

And here's an implementation of my generic builder:

public class CarBuilder : Builder<Car>
{
    public CarBuilder()
    {
        this.SetInstantiator(c => new Car());
        return this;
    }

    public CarBuilder Wheels(int wheels)
    {
        this.Chain(c => c.SetWheelCount(wheels));
        return this;
    }

    public CarBuilder Convertible()
    {
        this.Chain(c => c.RetractableRoof = true);
        return this;
    }

    public Car Build() => this.Instantiate();
}

What is bothering me is the repeated return this after each call to the Chain method and thought I could push this into the Chain method itself i.e. I want to write code like this:

    public CarBuilder Wheels(int wheels) =>
        this.Chain(c => c.SetWheelCount(wheels));

In the builder class I tried changing the return type from void to Builder:

protected Builder Chain(Action<T> action)
{
    this.ChainFunc(action);
    return this;
}

... but the compiler says the return type has to be Builder<T> i.e.

protected Builder<T> Chain(Action<T> action)
{
    this.ChainFunc(action);
    return this;
}

OK, fair enough, but in my implementation class I now have to do a cast:

    public CarBuilder Wheels(int wheels) =>
        (CarBuilder)this.Chain(c => c.SetWheelCount(wheels));

So again I have repeated code in that all methods must now include a cast. Passing the class type from subtype to supertype doesn't feel right.

I think I might be missing something fundamental here. Can I avoid both repeating the cast and having to 'return this' from every builder implementation method?

petemoloy
  • 695
  • 1
  • 5
  • 9
  • Try to change this protected Builder Chain(Action action) to protected Builder Chain(Action action) – Samvel Petrosov May 12 '17 at 08:28
  • @S.Petrosov: yes, that is a required step I realised I had to do that causes me to have to cast in the implementation class, typo now corrected. – petemoloy May 12 '17 at 08:53

2 Answers2

1

Don't put Chain in the base class. Instead, make it a generic extension method:

public static TBuilder Chain<TBuilder, TObject>(this TBuilder @this, Action<TObject> a)
 where TBuilder: Builder<TObject>
 => ...
Luaan
  • 62,244
  • 7
  • 97
  • 116
  • Funny, I had my `Chain`, `ChainFunc`, etc methods as generic extension methods before thinking, "These should maybe go in a base class...?" – petemoloy May 12 '17 at 08:48
1

One way to keep the logic in the protected scope, is to add a static method that is called instead of the instance method. The static method can use implicit casting to return the type of the caller

Inside Builder<T>

protected void Chain(Action<T> action)
{
    //local chain logic
}

protected static BT Chain<BT>(BT builder, Action<T> action)
    where BT:Builder<T>
{
    builder.Chain(action);
    return builder;
}

Calls inside CarBuilder:

public CarBuilder Wheels(int wheels) => Chain(this , c => c.SetWheelCount(wheels));

public CarBuilder Convertible() => Chain(this, c => c.RetractableRoof = true);
Me.Name
  • 12,259
  • 3
  • 31
  • 48
  • Yes, I was thiking along these lines too i.e. pass `this` into the Chain method so it can pass it back. It just seems strange to me that I can't use `this` in the base class directly, oh well! – petemoloy May 12 '17 at 09:50
  • `protected void Chain` can be changed to `private void Chain`, right? – petemoloy May 12 '17 at 09:50
  • Yep, it can. because the static method is inside the builder class it has access to all private class members – Me.Name May 12 '17 at 09:51
  • and I know what you mean with the `this`, sometimes if just feels better to use this.. etc. The extension method proposed by Luaan can make that happen, if the static class has access to the local methods (can be done via an interface or through some protected internal combinations) – Me.Name May 12 '17 at 09:55
  • `protected static` do not provide any more protection compared to `public static`. – user4003407 May 12 '17 at 20:39
  • @PetSerAl Not sure what you mean with more protection, but it certainly is a different scope (whichi is what I meant with 'protected scope'). protected static is only accessible in the class where it's declared and its inheritors – Me.Name May 13 '17 at 06:02
  • Yes, **its inheritors**. Unless class is `sealed`, anyone who can access the class, can inherit from it and access its `protected` members. That make `protected static` no different from `public static`. In both cases anyone can call the method. – user4003407 May 13 '17 at 07:14
  • @PetSerAl No, `public static` can be called from **outside** the class (or its inheritors), `protected static` cannot. It's not about making it inaccessible, it's about limiting its scope. Sure, someone can inherit from the class to access the method, but that's the whole purpose of the protected scope. You are saying that the protected keyword itself has no purpose. – Me.Name May 13 '17 at 07:58
  • I am saying that `protected` keyword does not limit accessibility of `static` members, compared to how it limit accessibility of non-static members. – user4003407 May 13 '17 at 08:03
  • @PetSerAl Ah, perhaps you are referring to the last comment before yours. That's about an alternative *extension* method as proposed in another answer. Since the extension method would need to be in a static class, thus in another class that does by definition *not* inherit from any class, it cannot access any private or protected instance members, while a static method inside the class itself can, regardless of its own scope – Me.Name May 13 '17 at 08:09
  • I am not referencing any particular comment. I am just saying, that although `protected` keyword limit accessibility of instance members: `public class Base { protected void DoSomething() { } } public class Derived : Base { void DoSomethingGood() { this.DoSomething(); } } public class RogueDerived : Base { static void DoSometingBad(Base b, Derived d) { b.DoSomething(); /*Error here*/ d.DoSomething(); /*and here*/ } }`. It does not apply to `protected static`: any `RogueDerived` free to call them. Thus, `protected static` effectively have the same accessibility as `public static`. – user4003407 May 13 '17 at 16:02