HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Collection of Actions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
actionscollectionstackoverflow

Problem

I am trying to create a class to queue up a series of commands. (Not really a queue as events happen based on time). Each command has a callback (Action) that get's called. However, each Action has a different set of parameters (signature), so I am using an object[] to pass them and then have to do lots of casting.

Example: the Queue class:

class RfxQueue {
    public enum CMD {DISPLAY, REPLACE_TMPLT, SCALE_DVE, etc };
    private Dictionary> actions = new Dictionary>();
    // Struct to hold command info
    public struct QItem
    {
        public CMD cmd;           // The CMD to exec
        public long ttd;          // Time to be executed
        public object[] param;    // Parameters to Action
    }
    private List q = new List();   // The queue of events

    // Create a new callback
    public void addAction(CMD cmd, Action action)
    {
        actions.Add(cmd, action);
    }

    // Add to Q
    public void pushEvent(CMD Cmd, float Delay, params object[] Param)
    {
        q.Add(new QItem() { cmd = Cmd, param = Param, ttd = DateTime.Now.AddMilliseconds(Delay * 1000).Ticks });
    }

    // This is actually done in a thread. Simplified here
    private void execEvent()
    {
        QItem item;
        // item is pulled from q here
        actions[item.cmd].Invoke(item.param);
    }
}


And the owner class creates events like this:

// "q" is a RfxQueue and the internal Invoke is required to run on main thread
q.events.Add(RfxQueue.CMD.SCALE_DVE, new Action(o =>
{
    dispatcher.Invoke(new Action((a,b,c) =>
    {
        _cgManager.ScaleDVE(a, b, c);
    }), (string)o[0], (string)o[1], (bool)o[2]);
}));
q.events.Add(RfxQueue.CMD.PLAY_ANIMATION, new Action(o =>
{
    dispatcher.Invoke(new Action((v, s) => {
        _cgManager.playAnimation(v, s);
    }), (VideoTemplate)o[0], (string)o[1]);
}));


Lastly, an event is added like this:

```
// Play animation "inserton" in 3 seconds
q.add(RfxQueue.CMD.PLAY_ANIMATION, template, 3f, "inserton");

Solution

Starting from minor issues - please follow naming conventions (variables should be camelCased and methods PascalCased, do add access modifiers, give variables meaningful names).

Your design breaks Open-Closed principle: Queue knows all the possible commands it can handle. In case when you need to add a new command you'll have to change the queue rather than add new code.

Also it breaks Single Responsibility Principle since queue knows not only about scheduling the execution but also serves as a repository for all possible actions to be run for commands.

Both issues as well as those you have mentioned (type casting, different signatures) can be easily solved if you create a separate class per command type. Command will encapsulate parameters required for execution as well as the action to be executed:

public interface ICommand
{
    void Execute();
}

public class ScaleDveCommand : ICommand
{
    public CgManager CgManager { get; set; }
    public string FirstString { get; set; } //TODO:Rename to meaningful name
    public string SecondString { get; set; } //TODO:Rename to meaningful name
    public bool SomeBool { get; set; } //TODO:Rename to meaningful name

    public void Execute()
    {
        dispatcher.Invoke(() => CgManager.ScaleDVE(FirstString, SecondString, SomeBool));
    }
}

public class PlayAnimationCommand : ICommand
{
    public CgManager CgManager { get; set; }
    public VideoTemplate VideoTemplate { get; set; }
    public string SomeMeaningfulNameHere { get; set; } //TODO:Rename to meaningful name

    public void Execute()
    {
        dispatcher.Invoke(() => { CgManager.playAnimation(VideoTemplate, SomeMeaningfulNameHere); });
    }
}


And your "queue" class will look like:

public class RfxQ
{
    private readonly SortedList _queue = new SortedList();   // The queue of events

    public void PushEvent(ICommand command, TimeSpan delay)
    {
        _queue.Add(DateTime.Now.Add(delay).Ticks, command);
    }

    // This is actually done in a thread. Simplified here
    private void ExecEvent()
    {
        ICommand command;
        // item is pulled from _queue here
        command.Execute();
    }
}


And usage will look like:

q.pushEvent(new PlayAnimationCommand
    {
        CgManager = _cgManager,
        VideoTemplate = template,
        SomeMeaningfulNameHere = "inserton"
    }, TimeSpan.FromSeconds(3));

Code Snippets

public interface ICommand
{
    void Execute();
}

public class ScaleDveCommand : ICommand
{
    public CgManager CgManager { get; set; }
    public string FirstString { get; set; } //TODO:Rename to meaningful name
    public string SecondString { get; set; } //TODO:Rename to meaningful name
    public bool SomeBool { get; set; } //TODO:Rename to meaningful name

    public void Execute()
    {
        dispatcher.Invoke(() => CgManager.ScaleDVE(FirstString, SecondString, SomeBool));
    }
}

public class PlayAnimationCommand : ICommand
{
    public CgManager CgManager { get; set; }
    public VideoTemplate VideoTemplate { get; set; }
    public string SomeMeaningfulNameHere { get; set; } //TODO:Rename to meaningful name

    public void Execute()
    {
        dispatcher.Invoke(() => { CgManager.playAnimation(VideoTemplate, SomeMeaningfulNameHere); });
    }
}
public class RfxQ
{
    private readonly SortedList<long, ICommand> _queue = new SortedList<long, ICommand>();   // The queue of events

    public void PushEvent(ICommand command, TimeSpan delay)
    {
        _queue.Add(DateTime.Now.Add(delay).Ticks, command);
    }

    // This is actually done in a thread. Simplified here
    private void ExecEvent()
    {
        ICommand command;
        // item is pulled from _queue here
        command.Execute();
    }
}
q.pushEvent(new PlayAnimationCommand
    {
        CgManager = _cgManager,
        VideoTemplate = template,
        SomeMeaningfulNameHere = "inserton"
    }, TimeSpan.FromSeconds(3));

Context

StackExchange Code Review Q#20587, answer score: 7

Revisions (0)

No revisions yet.