Would this be a good case for polymorphism.

Question

Sorry if this is a really basic question but I struggling with how I should attack this. I am trying to wrap up some commands for a OLE object, the basic spec looks like this:

Set Window window_id
    //Units is part of the position setter.
    [ Position ( x, y ) [ Units paper_units ] ] 
    [ Width win_width [ Units paper_units ] ] 
    [ Height win_height [ Units paper_units ] ]
    ..... this goes on for about 20+ commands, all optional.

Where anyhting in between [] is optional.

So I need to create a class lets call it "CommandBuilder" that can will have set methods for all these optional setters, thats fine I can handle that, the main problem I'm having is the ToCommandString method that needs to ouput a string which would look somthing like:

Set Window 1 Position (x,y) Units "m" Height 100 Units "m" + what ever else the user added

Just doing a few if's based on variables that get set and joining the string works fine when there is nothing complicated about the variables being set or there are only a few variables but when there are heaps of variables and/or nested values that are also optional, it can make the ToString method very long and complicated + hard to maintain if anything changes.

I was wondering if I could solve this by using polymorphism by doing something like this.

interface ICommand
{
    string ToCommandString();
}

class PositionCommand : ICommand
{
    double X;
    double Y;
    string Units;

    public PositionCommand(double x, double y)
    {
        this.X = x;
        this.Y = y;
    }

    public PositionCommand(double x,double y, string units)
    {
        this.X = x;
        this.Y = y;
        this.Units = units;
    }

    public string ToCommandString()
    {
        //Add more stuff here to handle empty units.
        return String.Format(" Postion ({0},{1})", X.ToString(), Y.ToString());
    }
}
....more command classes.

Then all my set methods in "CommandBuilder" can just create the right command type add it to a list, then the main ToString in "CommandBuilder" method can loop through all the ones that have been set and call ToCommandString and no have to worry about doing any if statments or null checks.

Would this be the correct way to go about this?

P.S. If you need more info I would be glad to add, just didn't want to make it to long first go.

Answer 1

Yes. I think you got it covered well enough.

Answer 2

That sounds reasonable to me. I would definately keep the construction of the ICommand instances internal to the CommandBuilder:

class CommandBuilder
{
  private List<ICommand> _commands = new List<ICommand>();

  public CommandBuilder Position(double x, double y)
  {
    _commands.Add(new PositionCommand(x,y))
    return this;
  }

  ...
}

Rather than just

class CommandBuilder
{
  public void AddCommand(ICommand cmd)
  { ... }
}