patterncsharpMinor
Collection of Actions
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:
And the owner class creates events like this:
Lastly, an event is added like this:
```
// Play animation "inserton" in 3 seconds
q.add(RfxQueue.CMD.PLAY_ANIMATION, template, 3f, "inserton");
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:
And your "queue" class will look like:
And usage will look like:
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.