-1

I am working with multiple commands that are executed in order (and possibly have sub commands as well) which can be desribed like this:

interface ICommand
{
    void Execute();
}

abstract class AbstractCommand : ICommand
{
    protected List<ICommand> ChildCommands { get; }

    public virtual void Execute()
    {
        foreach (var childCommand in ChildCommands)
            childCommand.Execute(arguments);
    }
}

class SampleCommand : AbstractCommand
{
    public override void Execute()
    {
        base.Execute();
    }
}

These commands are executed from another class that stores them:

class Generator
{
    private Grid[,] Layout { get; }
    private List<ICommand> Commands { get; }

    public void RunCommands()
    {
        foreach (var command in Commands)
            command.Execute();
    }
}

The intention is that the commands can freely manipulate the Layout matrix and possibly some other objects I will introduce at a later stage. This would require me to pass them by reference but I noticed with already 10+ commands refactoring will get unbearable once I decide to expose another object from Generator to all ICommands. Therefore I considered to introduce a class CommandArguments which stored the Layout by reference and possibly more and change the signature of Execute() to Execute(CommandArguments).

Unfortunately I don't like the thought of juggling with the arguments by ref and considered to introduce an interface IGenerator to access the objects via methods and change the signature to Execute(IGenerator) like this:

interface IGenerator
{
    void DoSomething();
}

class SampleCommand : AbstractCommand
{
    public override void Execute(IGenerator gen)
    {
        gen.DoSomething();
        base.Execute(gen);
    }
}

class Generator : IGenerator
{
    private Grid[,] Layout { get; }
    private List<ICommand> Commands { get; }

    public void RunCommands()
    {
        foreach (var command in Commands)
            command.Execute(this);
    }

    public void DoSomething()
    {
        /* do something with Layout here */
    }
}

I'd like to know whether using this interface is better than using the CommandArguments class and whether there is something else I can improve.

EDIT

Since I got asked for more explanation here is more information: The code is used in a generator for procedurally generated levels. The necessary data is stored in the Generator class while the generation is described by various rules like AddRoom, AddHub, AddEvent that are refered to as the commands. However I am considering to implement multiple generators that behave slightly different (e.g. interior and exterior levels have different generators) while the generators can expose certain methods they have in common to the rules/commands in order to prevent both direct access to the underlying objects and making sure to handle them properly.

  • You need to provide a more concrete example than this. Foobar examples are good for demonstrating architectural scaffolding, but they don't tell us anything about your specific software requirements. – Robert Harvey Sep 23 '17 at 17:39
  • Also, you need to explain to us what you mean by "better." Without some idea about what specific characteristics you're trying to improve, we have no way to tell you if some other way is "better" or not. – Robert Harvey Sep 23 '17 at 17:41
  • @RobertHarvey I just added slightly more information in the newest paragraph. – Christian Ivicevic Sep 24 '17 at 17:41
0

I have an idea the type of application you are writing. If it is a type of graphical editor, the command arguments should be immutable properties (or fields) contained in the command arguments. If there is some global data needed, to be manipulated, it should be passed into the command execute method in some container objects. Unfortunately it is difficult to create state objects (state of the editor) that are very specific to a command when doing this type of work. The best you can do is have a high level object that contains sub objects that are specific to one area of functionality.

  • I just added more information in my question that might help you understand what I am using it for. – Christian Ivicevic Sep 24 '17 at 17:42
  • Unfortunately I understand your question even less now. The interface you mention adding, doesn't seem to make any difference functionally, except that you are trying to expose a slice of the Generator properties or fields? In this case, yes it is better to use a child instance and pass that instead. Prefer instances over interfaces when possible, for clarity and to limit implementation (an interface can be implemented by anything). – Frank Hileman Sep 26 '17 at 19:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.