I'm currently studying design patterns and I'm currently looking at the command pattern.
Here is my current code:
// this is the receiver
class Calculator : IReceiver
{
int x;
int y;
CommandOptions command;
public Calculator(int x, int y)
{
this.x = x;
this.y = y;
}
public void SetAction(CommandOptions command)
{
this.command = command;
}
public int GetResult()
{
int result = 0;
switch(this.command)
{
case CommandOptions.ADD:
result = this.x + this.y;
break;
case CommandOptions.SUBTRACT:
result = this.x - this.y;
break;
case CommandOptions.MULTIPLY:
result = this.x * this.y;
break;
}
return result;
}
}
// command
abstract class Command
{
protected IReceiver receiver;
public Command(IReceiver receiver)
{
this.receiver = receiver;
}
public abstract int Execute();
}
class AddCommand : Command
{
public AddCommand(IReceiver receiver) : base(receiver)
{
}
public override int Execute()
{
reciever.SetAction(CommandOptions.ADD);
return receiver.GetResult();
}
}
enum CommandOptions
{
ADD,
SUBTRACT,
MULTIPLY
}
interface IReceiver
{
void SetAction(CommandOptions command);
int GetResult();
}
class Program
{
static void Main(string[] args)
{
IReceiver receiver = new Calculator(500, 25);
//#Issue:The SetAction() method of the receiver is accessible.
//receiver.SetAction(CommandOptions.ADD);
receiver.SetAction(CommandOptions.MULTIPLY);
Command command = null;
Console.Write("Enter option 1-3: ");
int commandOption = int.Parse(Console.ReadLine());
switch(commandOption)
{
case 1:
command = new AddCommand(receiver);
break;
case 2:
command = new SubtractCommand(receiver);
break;
case 3:
command = new MultiplyCommand(receiver);
break;
default:
command = new AddCommand(receiver);
break;
}
Console.WriteLine(command.Execute());
Console.ReadKey();
}
}
Notice that in my main method, I can access the SetAction
method of the receiver which is capable of setting which command to use.
My question is: does my implementation violate the purpose of the command pattern, and is my implementation wrong because I'm able to access it in my client code? If so, how can I improve this implementation.
I took a shot of text-editing (i.e haven't run it, expect syntax errors :) ) your code. Here's how I would model your problem.
Some points-
1) Have the command do the action. In your case, you have command classes, but you calculator holds the logic for computation. Rather, encapsulate the command action within the command class itself
2) I've put a factory to map the command option to the command and saved a few lines by removing the break
s since I can return the command.
3) The IReceiver now holds the values which is passed on to Command. In this case, since our operators are all binary, I've just used X and Y. Can be an array or any other complex type for other cases.
4) Enum isn't required, unless you absolutely want it.
Edit On re-looking, I think an even better solution would be to not register the receiver with the commands, instead pass on the parameters while invoking the command.
//this is the receiver
class Calculator : IReceiver
{
int y;
int x;
public Calculator(int x, int y)
{
this.x = x;
this.y = y;
}
public int Calculate(int commandOption)
{
Command command = new CommandFactory().GetCommand(commandOption);
return command.Execute(x , y);
}
}
//command
interface ICommand
{
int Execute(int x, int y);
}
class AddCommand : Command
{
public override int Execute(int x, int y)
{
return x + y;
}
}
class MultiplyCommand : Command
{
public override int Execute(int x, int y)
{
return x * y;
}
}
class SubtractCommand : Command
{
public override int Execute(int x, int y)
{
return x - y;
}
}
interface IReceiver
{
int X {get; set;}
int Y {get; set;}
int Calculate(int commandOption);
}
public class CommandFactory
{
public GetCommand(int commandOption)
{
switch(commandOption)
{
case 1:
return new AddCommand();
case 2:
return new SubtractCommand();
case 3:
return new MultiplyCommand();
default:
return new AddCommand();
}
}
}
class Program
{
static void Main(string[] args)
{
IReceiver receiver = new Calculator(500, 25);
//#Issue:The SetAction() method of the receiver is accessible.
//receiver.SetAction(CommandOptions.ADD);
//Receiver no longer exposes SetAction
//receiver.SetAction(CommandOptions.MULTIPLY);
Console.Write("Enter option 1-3: ");
int commandOption = int.Parse(Console.ReadLine());
Console.WriteLine(receiver.Calculate(commandOption));
Console.ReadKey();
}
}