What is the best way to deal with optional delegates in a c# constructor?

Question

I'm refactoring a function that takes in an optional delegate through the constructor. The delegate is run when events are triggered within the class. If the delegate isn't passed then a local default function is used instead:

public class Foo
{
    int    _memberVariable;
    readonly Action _onEventOne;
    readonly Action _onEventTwo;

    public Foo(Action onEventOne, Action onEventTwo = null)
    {
        _memberVariable = 0;

        _onEventOne = onEventOne;
        _onEventTwo = onEventTwo ?? DefaultEventTwo;

        _onEventOne();
    }

    private void DefaultEventTwo()
    {
        ++_memberVariable;
    }
}

I'm looking to remove the default value (this is a public interface so overloading would be preferable), and this is in production code so I don't want to change the interface unless I must.

In an ideal world I'd use constructor chaining:

public Foo(Action onEventOne) : this(onEventOne, DefaultEventTwo)
{
    //CS0120 An object reference is required for the non-static field, method, or property 'Foo.DefaultEventTwo()
}

(I understand why this doesn't work, just giving an example of the kind of solution I would use if this weren't a constructor).

Because the delegates are readonly I can't set them in a shared initialize type function.

Is there a better way to handle a case like than just passing in null and then catching it in the main constructor? It doesn't feel very elegant, and I'd like to be able to catch a null Action as an exception ideally (for if an external caller used null instead of using the overloaded constructor). I could remove the readonly from the delegates but again it doesn't feel like a great solution as they are really readonly.

Any thoughts would be appreciated.

Answer 1

The only way I could get this working was by getting it working ugly (in my own opinion).

You have to deliver a static method, but that static method could use a reference to this to get the actual method out.

This is what I came up with.

public Foo(Action onEventOne) : this(onEventOne, self => self.DefaultEventTwo)
{
    //CS0120 An object reference is required for the non-static field, method, or property 'Foo.DefaultEventTwo()
}

public Foo(Action onEventOne, Action onEventTwo = null) : this(onEventOne, self => onEventTwo)
{ }

// private constructor, just for the sake of getting it working
private Foo(Action onEventOne, Func<Foo, Action> onEventTwo = null)
{
    _memberVariable = 0;

    _onEventOne = onEventOne;
    _onEventTwo = onEventTwo(this); // <--

    _onEventOne();
}

self => self.DefaultEventTwo is the static function to get the action. That function is used in the call to onEventTwo(this) to get the default event of this instance.

Answer 2

Did I miss anything?

public class Foo
{
    int    _memberVariable;
    readonly Action _onEventOne;
    readonly Action _onEventTwo;

    public Foo(Action onEventOne): this(onEventOne, null) { }
    public Foo(Action onEventOne, Action onEventTwo)
    {
        _memberVariable = 0;

        _onEventOne = onEventOne;
        _onEventTwo = onEventTwo ?? DefaultEventTwo;

        _onEventOne();
    }

    private void DefaultEventTwo()
    {
        ++_memberVariable;
    }
}

All you have to do is delete the default-value and create a new constructor which has only one argument. Now in the most detailed constructor (the original one) check if the provided value is null and if so set _onEventTwo to DefaultEventTwo.

To avoid that anyone uses the reduced constructor simply make it internal.

EDIT: Concerning the exception-handling. What about using an internal constructor as "main"-one that all the others call with a param indicating where the call came from:

internal Foo(Action onEventOne): this(onEventOne, null, true) { }
// public API: NULL not allwoed as param
public Foo(Action onEventOne, Action onEventTwo) : this(onEventOne, onEventTwo, false) { }
internal Foo(Action onEventOne, Action onEventTwo, bool internalUse)
{
    _memberVariable = 0;

    _onEventOne = onEventOne;
    if(onEventTwo == null)
    {
        if(!internalUse) throw new ArgumentNullException("onEventTwo");
        else this._onEventTwo = DefaultEventTwo;
    }
    _onEventOne();
}