More concise way to pass, as constructor parameter, an Action that references private data?-CodePudding

This is stripped down from a more complex situation.
The goal is to construct several instances of class SubAction, each of which uses an action to alter how it uses its internal data.

Consider:

    public class SubAction
    {
        private Action<SubAction> _DoIt;

        public SubAction(Action<SubAction> doIt)
        {
            _DoIt = doIt;
        }

        public void DoIt()
        {
            _DoIt(this);
        }

        static public Action<SubAction> GetAction1 => (it) => it.DoSomething(it._Data.Value1);
        static public Action<SubAction> GetAction2 => (it) => it.DoSomething(it._Data.Value2);

        private void DoSomething(string value)
        {
            // ...
        }

        // This gets set by code not shown.
        protected Data _Data;
    }


    public class Data
    {
        public string Value1;
        public string Value2;
    }


    public class SubActionTests
    {
        static SubActionTests()
        {
            var actions = new List<SubAction>
            {
                new SubAction(SubAction.GetAction1),
                new SubAction(SubAction.GetAction2),
            };

            // ... code not shown that calls a method to update each instance's _Data...

            foreach (var subAction in actions)
            {
                subAction.DoIt();
            }
        }
    }

This works, but it seems cumbersome. Specifically:

public Action<SubAction> _DoIt { get; set; }
...
static public Action<SubAction> GetAction1 => (it) => it.DoSomething(it._Data.Value1);
...
new SubAction(SubAction.GetAction1)

If I set DoIt AFTER constructing the object, could simply be:

public Action DoIt { get; set; }
...
public Action GetAction1 => () => DoSomething(_Data.Value1);
...
var it = new SubAction();
it.DoIt = it.GetAction1;

Which has simpler action declarations:

The actions don't need <SubAction>.
`GetAction1,2,3.. declarations are much simpler.

But more verbose instance initialization, because access to it is needed to set DoIt.

Unfortunately it isn't possible to refer to "it" during object initializer, so there doesn't seem to be any way to have BOTH the simpler initialization syntax AND the simpler action-declaration syntax.

Am I overlooking some solution?

ALTERNATIVE: factory method NOTE: This could be approached quite differently, by using an enum to select between the different actions. But that is a different sort of complication; I'm looking for a way to describe these Actions themselves more succinctly.

Specifically, I'm aware there could be a factory method that takes an enum, to hide the complexity:

public enum WhichAction
{
    Action1,
    Action2
}
...

public static CreateSubAction(WhichAction which)
{
    var it = new SubAction();
    switch (which)
    {
    case WhichAction.Action1:
        it.DoIt = it.GetAction1;
        break;
    case WhichAction.Action2:
        it.DoIt = it.GetAction2;
        break;
    }

    return it;
}

The downside of this is that each added action requires editing in multiple places.

ALTERNATIVE: sub-classes

Another alternative is to create multiple sub-classes.
That is what I was doing originally, but that was even more verbose - multiple lines per each new action.
And felt like "overkill".

After all, the approach I've got isn't terrible - its a single line for each new GetAction. It just felt like each of those lines "ought" to be much simpler.

CodePudding user response：

Sadly, from what I understand, I don't think you can make the complexity disappear. You probably need to choose an approach from the ones you suggested (or even other solutions like using a strategy pattern).

Advice

When confronted with a design choice like this. I suggest you optimize for the consumer's side of things. In other words, design your classes to make them simple to use.

In your scenario, that would mean opting for your initial solution or the more complex solutions (factory method, sub-classes, strategy pattern, etc.).

The problem with the second solution is that your object can be in a limbo state when initializing it.

var it = new SubAction();
// Before you set DoIt, the object is not fully initialized.
it.DoIt = it.GetAction1;

Consumers can also forget to set DoIt. When possible, you should probably avoid designs that allow such mistakes.

CodePudding user response：

While I'm still curious whether there are syntax alternatives that would streamline what I showed, so I'll accept an answer that shows a simpler syntax, turns out in my situation, I can easily avoid the need for those actions.

Discussing with a colleague, they pointed out that my current actions all have a similar pattern: get a string, pass it to SubAction.DoSomething.

Therefore I can simplify those actions down to a property that gets the appropriate string:

public abstract string CurrentValue { get; }
...
public virtual void DoIt()
{
    DoSomething(CurrentValue);
}

Given the above, subclasses become so simple they no longer feel like "overkill":

public class SubAction1 : SubAction
{
    protected override string CurrentValue => _Data.Value1;
}
...
// usage
new SubAction1()

That is straightforward; highly readable. And trivial to extend when additional conditions are needed.

There will be more complicated situations that do need to override DoSomething. In those, the "real work" dwarfs what I've shown; so its appropriate to subclass those anyway.