Quick Information/Overview
| Pattern Type |
Behavioral |
| Applicable Language/Framework |
Agnostic OOP |
| Pattern Source |
Gang of Four |
| Difficulty |
Easy – Moderate |
Up Front Definitions
- Invoker: This object services clients by exposing a method that takes a command as a parameter and invoking the command’s execute
- Receiver: This is the object upon which commands are performed – its state is mutated by them
The Problem
Let’s say you get a request from management to write an internal tool. A lot of people throughout the organization deal with XML documents and nobody really likes dealing with them, so you’re tasked with writing an XML document builder. The user will be able to type in node names and pick where they go and whatnot. Let’s also assume (since this post is not about the mechanics of XML) that all XML documents consist of a root node called “Root” and only child nodes of root.
The first request that you get in is the aforementioned adding. So, knowing that you’ll be getting more requests, your first design decision is to create a DocumentBuilder class and have the adding implemented there.
/// This class is responsible for doing document build operations
public class DocumentBuilder
{
/// This is the document that we're doing to be modifying
private readonly XDocument _document;
/// Initializes a new instance of the DocumentBuilder class.
///
public DocumentBuilder(XDocument document = null)
{
_document = document ?? new XDocument(new XElement("Root"));
}
/// Add a node to the document
///
public void AddNode(string elementName)
{
_document.Root.Add(new XElement(elementName));
}
}
//Client code:
var myInvoker = new DocumentBuilder();
myInvoker.AddNode("Hoowa!");
So far, so good. Now, a request comes in that you need to be able to do undo and redo on your add operation. Well, that takes a little doing, but after 10 minutes or so, you’ve cranked out the following:
public class DocumentBuilder
{
/// This is the document that we're doing to be modifying
private readonly XDocument _document;
/// Store things here for undo
private readonly Stack _undoItems = new Stack();
/// Store things here for redo
private readonly Stack _redoItems = new Stack();
/// Initializes a new instance of the DocumentBuilder class.
///
public DocumentBuilder(XDocument document = null)
{
_document = document ?? new XDocument(new XElement("Root"));
}
/// Add a node to the document
///
public void AddNode(string elementName)
{
_document.Root.Add(new XElement(elementName));
_undoItems.Push(elementName);
_redoItems.Clear();
}
/// Undo the previous steps operations
public void Undo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myName = _undoItems.Pop();
_document.Root.Elements(myName).Remove();
_redoItems.Push(myName);
}
}
/// Redo the number of operations given by steps
public void Redo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myName = _redoItems.Pop();
_document.Root.Add(new XElement(myName));
_undoItems.Push(myName);
}
}
}
Not too shabby - things get popped from each stack and added to the other as you undo/redo, and the redo stack gets cleared when you start a new "branch". So, you're pretty proud of this implementation and you're all geared up for the next set of requests. And, here it comes. Now, the builder must be able to print the current document to the console. Hmm... that gets weird, since printing to the console is not really representable by a string in the stacks. The first thing you think of doing is making string.empty represent a print operation, but that doesn't seem very robust, so you tinker and modify until you have the following:
public class DocumentBuilder
{
/// This is the document that we're doing to be modifying
private readonly XDocument _document;
/// This defines what type of operation that we're doing
private enum OperationType
{
Add,
Print
}
/// Store things here for undo
private readonly Stack> _undoItems = new Stack>();
/// Store things here for redo
private readonly Stack> _redoItems = new Stack>();
/// Initializes a new instance of the DocumentBuilder class.
///
public DocumentBuilder(XDocument document = null)
{
_document = document ?? new XDocument(new XElement("Root"));
}
/// Add a node to the document
///
public void AddNode(string elementName)
{
_document.Root.Add(new XElement(elementName));
_undoItems.Push(new Tuple(OperationType.Add, elementName));
_redoItems.Clear();
}
/// Print out the document
public void PrintDocument()
{
Print();
_redoItems.Clear();
_undoItems.Push(new Tuple(OperationType.Print, string.Empty));
}
/// Undo the previous steps operations
public void Undo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myOperation = _undoItems.Pop();
switch (myOperation.Item1)
{
case OperationType.Add:
_document.Root.Elements(myOperation.Item2).Remove();
_redoItems.Push(myOperation);
break;
case OperationType.Print:
Console.Out.WriteLine("Sorry, but I really can't undo a print to screen.");
_redoItems.Push(myOperation);
break;
}
}
}
/// Redo the number of operations given by steps
public void Redo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myOperation = _redoItems.Pop();
switch (myOperation.Item1)
{
case OperationType.Add:
_document.Root.Elements(myOperation.Item2).Remove();
_undoItems.Push(myOperation);
break;
case OperationType.Print:
Print();
_undoItems.Push(myOperation);
break;
}
}
}
private void Print()
{
var myBuilder = new StringBuilder();
Console.Out.WriteLine("\nDocument contents:\n");
using (var myWriter = XmlWriter.Create(myBuilder, new XmlWriterSettings() { Indent = true, IndentChars = "\t" }))
{
_document.WriteTo(myWriter);
}
Console.WriteLine(myBuilder.ToString());
}
}
Yikes, that's starting to smell a little. But, hey, you extracted a method for the print, and you're keeping things clean. Besides, you're fairly proud of your little tuple scheme for recording what kind of operation it was in addition to the node name. And, there's really no time for 20/20 hindsight because management loves it. You need to implement something that lets you update a node's name ASAP.
Oh, and by the way, they also want to be able to print the output to a file instead of the console. Oh, and by the by the way, you know what would be just terrific? If you could put something in to switch the position of two nodes in the file. They know it's a lot to ask right now, but you're a rock star and they know you can handle it.
So, you buy some Mountain Dew and pull an all nighter. You watch as the undo and redo case statements grow vertically and as your tuple grows horizontally. The tuple now has an op code and an element name like before, but it has a third argument that means the new name for update, and when the op-code is swap, the second and third arguments are the two nodes to swap. It's ugly (so ugly I'm not even going to code it for the example), but it works.
And, it's a success! Now, the feature requests really start piling up, and not only are stakeholders using your app, but other programmers have started using your API. There's really no time to reflect on the design now - you have a ton of new functionality to implement. And, as you do it, the number of methods in your builder will grow as each new feature is added, the size of the case statements in undo and redo will grow with each new feature is added, and the logic for parsing your swiss-army knife tuple is going to get more and more convoluted.
By the time this thing is feature complete, it's going to take a 45 page developer document to figure out what on Earth is going on. Time to start putting out resumes and jump off this sinking ship.
So, What to Do?
Before discussing what to do, let's first consider what went wrong. There are two main issues here that have contributed to the code rot. The first and most obvious is the decision to "wing it" with the Tuple solution that is, in effect, a poor man's type. Instead of a poor man's type, why not an actual type? The second issue is a bit more subtle, but equally important -- violation of the open/closed principle.
To elaborate, consider the original builder that simply added nodes to the XDocument and the subsequent change to implement undo and redo of this operation. By itself, this was fine and cohesive. But, when the requirements started to come in about more operations, this was the time to go in a different design direction. This may not be immediately obvious, but a good question to ask during development is "what happens if I get more requests like this?" When the class had "AddNode", "Undo" and "Redo", and the request for "PrintDocument" came in, it was worth noting that you were cobbling onto an existing class. It also would have been reasonable to ask, "what if I'm asked to add more operations?"
Asking this question would have resulted in the up-front realization that each new operation would require another method to be added to the class, and another case statement to be added to two existing methods. This is not a good design -- especially if you know more such requests are coming. Having an implementation where new the procedure for accommodating new functionality is "tack another method onto class X" and/or "open method X and add more code" is a recipe for code rot.
So, let's consider what we could have done when the request for document print functionality. Instead of this tuple thing, let's create another implementation. What we're going to do is forget about creating Tuples and forget about the stacks of string, and think in terms of a command object. Now, at the moment, we only have one command object, but we know that we've got a requirement that's going to call for a second one, so let's make it polymorphic. I'm going to introduce the following interface:
public interface IDocumentCommand
{
/// Document (receiver) upon which to operate
XDocument Document { get; set; }
/// Execute the command
void Execute();
/// Revert the execution of the command
void UndoExecute();
}
This is what will become the command in the command pattern. Notice that the interface defines two conceptual methods - execution and negation of the execution (which should look a lot like "do" and "undo"), and it's also going to be given the document upon which to do its dirty work.
Now, let's take a look at the add implementer:
public class AddNodeCommand : IDocumentCommand
{
private readonly string _nodeName;
private XDocument _document = new XDocument();
public XDocument Document { get { return _document; } set { _document = value ?? _document; } }
/// Initializes a new instance of the AddNodeCommand class.
/// Note the extra parameter here -- this is important. This class essentially conceptually
/// an action, so you're more used to seeing things in method form like this. We pass in the "method" parameters
/// to the constructor because we're encapsulating an action as an object with state
public AddNodeCommand(string nodeName)
{
_nodeName = nodeName ?? string.Empty;
}
public void Execute()
{
Document.Root.Add(new XElement(_nodeName));
}
public void UndoExecute()
{
Document.Root.Elements(_nodeName).Remove();
}
}
Pretty straightforward (in fact a little too straightforward - in a real implementation, there should be some error checking about the state of the document). When created, this object is seeded with the name of the node that it's supposed to create. The document is a setter dependency, and the two operations mutate the XDocument, which is our "receiver" in the command pattern, according to the pattern's specification.
Let's have a look at what our new Builder implementation now looks like before adding print document:
public class DocumentBuilder
{
/// This is the document that we're doing to be modifying
private readonly XDocument _document;
/// Store things here for undo
private readonly Stack _undoItems = new Stack();
/// Store things here for redo
private readonly Stack _redoItems = new Stack();
/// Initializes a new instance of the DocumentBuilder class.
///
public DocumentBuilder(XDocument document = null)
{
_document = document ?? new XDocument(new XElement("Root"));
}
/// Add a node to the document
///
public void AddNode(string elementName)
{
var myCommand = new AddNodeCommand(elementName) { Document = _document };
myCommand.Execute();
_undoItems.Push(myCommand);
_redoItems.Clear();
}
/// Undo the previous steps operations
public void Undo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myCommand = _undoItems.Pop();
myCommand.UndoExecute();
_redoItems.Push(myCommand);
}
}
/// Redo the number of operations given by steps
public void Redo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myCommand = _redoItems.Pop();
myCommand.Execute();
_undoItems.Push(myCommand);
}
}
}
Notice that the changes to this class are subtle but interesting. We now have stacks of commands rather than strings (or, later, tuples). Notice that undo and redo now delegate the business of executing the command to the command object, rather than figuring out what kind of operation it is and doing it themselves. This is critical to conforming to the open/closed principle, as we'll see shortly.
Now that we've performed our refactoring, let's add the print document functionality. This is now going to be accomplished by a new implementation of IDocumentCommand:
public class PrintCommand : IDocumentCommand
{
private XDocument _document = new XDocument();
public XDocument Document { get { return _document; } set { _document = value ?? _document; } }
/// Execute the print command
public void Execute()
{
var myBuilder = new StringBuilder();
Console.Out.WriteLine("\nDocument contents:\n");
using (var myWriter = XmlWriter.Create(myBuilder, new XmlWriterSettings() { Indent = true, IndentChars = "\t" }))
{
Document.WriteTo(myWriter);
}
Console.WriteLine(myBuilder.ToString());
}
/// Undo the print command (which, you can't)
public void UndoExecute()
{
Console.WriteLine("\nDude, you can't un-ring that bell.\n");
}
}
Also pretty simple. Let's now take a look at how we implement this in our "invoker", the DocumentBuilder:
public class DocumentBuilder
{
/// This is the document that we're doing to be modifying
private readonly XDocument _document;
/// Store things here for undo
private readonly Stack _undoItems = new Stack();
/// Store things here for redo
private readonly Stack _redoItems = new Stack();
/// Initializes a new instance of the DocumentBuilder class.
///
public DocumentBuilder(XDocument document = null)
{
_document = document ?? new XDocument(new XElement("Root"));
}
/// Add a node to the document
///
public void AddNode(string elementName)
{
var myCommand = new AddNodeCommand(elementName) { Document = _document };
myCommand.Execute();
_undoItems.Push(myCommand);
_redoItems.Clear();
}
/// Print the document
public void PrintDocument()
{
var myCommand = new PrintCommand() { Document = _document};
myCommand.Execute();
_undoItems.Push(myCommand);
_redoItems.Clear();
}
/// Undo the previous steps operations
public void Undo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myCommand = _undoItems.Pop();
myCommand.UndoExecute();
_redoItems.Push(myCommand);
}
}
/// Redo the number of operations given by steps
public void Redo(int steps)
{
for (int index = 0; index < steps; index++)
{
var myCommand = _redoItems.Pop();
myCommand.Execute();
_undoItems.Push(myCommand);
}
}
}
Lookin' good! Observe that undo and redo do not change at all. Our invoker now creates a command for each operation, and delegate its work to the receiver on behalf of the client code. As we continue to add more commands, we do not ever have to modify undo and redo.
But, we still don't have it quite right. The fact that we need to add a new class and a new method each time a new command is added is still a violation of the open/closed principle, even though we're better off than before. The whole point of what we're doing here is separating the logic of command execution (and undo/redo and, perhaps later, indicating whether a command can currently be executed or not) from the particulars of the commands themselves. We're mostly there, but not quite - the invoker, DocumentBuilder is still responsible for enumerating the different commands as methods and creating the actual command objects. The invoker is still too tightly coupled to the mechanics of the commands.
This is not hard to fix - pass the buck! Let's look at an implementation where the invoker, instead of creating commands in named methods, just demands the commands:
public class DocumentBuilder
{
/// This is the document that the user will be dealing with
private readonly XDocument _document;
/// This houses commands for undo
private readonly Stack _undoCommands = new Stack();
/// This houses commands for redo
private readonly Stack _redoCommands = new Stack();
/// User can give us an xdocument or we can create our own
public DocumentBuilder(XDocument document = null)
{
_document = document ?? new XDocument(new XElement("Root"));
}
/// Executes the given command
///
public void Execute(IDocumentCommand command)
{
if (command == null) throw new ArgumentNullException("command", "nope");
command.Document = _document;
command.Execute();
_redoCommands.Clear();
_undoCommands.Push(command);
}
/// Perform the number of undos given by iterations
public void Undo(int iterations)
{
for (int index = 0; index < iterations; index++)
{
if (_undoCommands.Count > 0)
{
var myCommand = _undoCommands.Pop();
myCommand.UndoExecute();
_redoCommands.Push(myCommand);
}
}
}
/// Perform the number of redos given by iterations
public void Redo(int iterations)
{
for (int index = 0; index < iterations; index++)
{
if (_redoCommands.Count > 0)
{
var myCommand = _redoCommands.Pop();
myCommand.UndoExecute();
_undoCommands.Push(myCommand);
}
}
}
}
And, there we go. Observe that now, when new commands are to be added, all a maintenance programmer has to do is author a new class. That's a much better paradigm. Any bugs related to the mechanics of do/undo/redo are completely separate from the commands themselves.
Some might argue that the new invoker/DocumentBuilder lacks expressiveness in its API (having Execute(IDocumentCommand) instead of AddNode(string) and PrintDocument()), but I disagree:
var myInvoker = new DocumentBuilder();
myInvoker.Execute(new AddNodeCommand("Hoowa"));
myInvoker.Execute(new PrintCommand());
myInvoker.Undo(2);
myInvoker.Execute(new PrintCommand());
Execute(AddCommand(nodeName)) seems just as expressive to me as AddNode(nodeName), if slightly more verbose. But even if it's not, the tradeoff is worth it, in my book. You now have the ability to plug new commands in anytime by implementing the interface, and DocumentBuilder conforms to the open/closed principle -- it's only going to change if there is a bug in the way the do/undo/redo logic is found and not when you add new functionality (incidentally, having only one reason to change also makes it conform to the single responsibility principle).
A More Official Explanation
dofactory defines the command pattern this way:
Encapsulate a request as an object, thereby letting you parameterize clients with different requests, queue or log requests, and support undoable operations.
The central, defining point of this pattern is the idea that a request or action should be an object. This is an important and not necessarily intuitive realization. The natural tendency would be to implement the kind of ad-hoc logic from the initial implementation, since we tend to think of objects as things like "Car" and "House" rather than concepts like "Add a node to a document".
But, this different thinking leads to the other part of the description - the ability to parameterize clients with different requests. What this means is that since the commands are stored as objects with state, they can encapsulate their own undo and redo, rather than forcing the invoker to do it. The parameterizing is the idea that the invoker operates on passed in command objects rather than doing specific things in response to named methods.
What is gained here is then the ability to put commands into a stack, queue, list, etc, and operate on them without specifically knowing what it is they do. That is a powerful ability since separating and decoupling responsibilities is often hard to accomplish when designing software.
Other Quick Examples
Here are some other places that the command pattern is used:
- The ICommand interface in C#/WPF for button click and other GUI events.
- Undo/redo operations in GUI applications (i.e. Ctrl-Z, Ctrl-Y).
- Implementing transactional logic for persistence (thus providing atomicity for rolling back)
A Good Fit – When to Use
Look to use the command pattern when there is a common set of "meta-operations" surrounding commands. That is, if you find yourself with requirements along the lines of "revert the last action, whatever that action may have been." This is an indicator that there are going to be operations on the commands themselves beyond simple execution. In scenarios like this, it makes sense to have polymorphic command objects that have some notion of state.
Square Peg, Round Hole – When Not to Use
As always, YAGNI applies. For example, if our document builder were only ever going to be responsible for adding nodes, this pattern would have been overkill. Don't go implementing the command pattern on any and all actions that your program may take -- the pattern incurs complexity overhead in the form of multiple objects and a group of polymorphs.
So What? Why is this Better?
As we've seen above, this makes code a lot cleaner in situations where it's relevant and it makes it conform to best practices (SOLID principles). I believe that you'll also find that, if you get comfortable with this pattern, you'll be more inclined to offer richer functionality to clients on actions that they may take.
That is, implementing undo/redo or atomicity may be something you'd resist or avoid as it would entail a great deal of complexity, but once you see that this need not be the case, you might be more willing or even proactive about it.
In, using this pattern where appropriate is better because it provides for cleaner code, fewer maintenance headaches, and more clarity.
