views:

587

answers:

8

Once I had a discussion about design, relative to the command pattern. My peer stated that a command object should not return the status (successful, unsuccessful, and why) after the .execute() method is called. The reason is that you should not be concerned if the command gets executed or not, because the command must contain no state. You must however check after the invocation if the command had the expected effect. Another point he argued was that on the Gang of Four, the command pattern does not present this case (of returning status).

I claimed the opposite point. The GoF does not present this case, but a pattern can be modeled to your needs. If a command is unsuccessful, the invoking client must receive a proof of the status, and eventually deploy an appropriate reaction. By forcing the client to check if the action achieved success was error prone and produced duplicated code. Moreover, there are cases where the command produces a result (eg. a command that adds a line to a plot, will somehow have the line id to return to the client), and pretending to have commands with no state meant that you had to "fish out" the new object identifier from the data model.

In the end, we reached a compromise by not returning status but keeping the id's of newly created objects in the command object, and the application worked pretty well anyway, but I am now curious to know your opinion as well.

Edit: I decided to add a small bounty. My hope is to enrich the question with some real case stories of the Command pattern in production environments.

Edit 2: Bumping the question before the end of the bounty (tomorrow)

+4  A: 

This is definitely debatable, but it clearly shows the two styles of thinking:

  • Check if something is okay, and then proceed accordingly
  • Proceed anyway and deal with it if something bad happens

I don't think one way is better than the other. For example in Java, it's typically best to not abuse your exception handling and to take care of any possible problems prior to simply throwing your hands (and exceptions) in the air. With Python, it's more preferable to just go ahead and attempt to do whatever, regardless of status code, and let any exception simply be dealt with accordingly.

It really is up to you as to whether or not you would like the command pattern to return a status.

AlbertoPL
+7  A: 

I don't have Design Patterns: Elements of Reusable Object-Oriented Software in front of me at the moment, but I'm pretty sure the authors even say that the design patterns they present are a model that can be modified to fit a specific situation.

This question cuts to the core of what a design pattern is - a template. It's not something that must be implemented by-the-book. You identified a case where a logical modification to the pattern as presented in the book would have helped the application, and that's perfectly fine, especially once you weigh the benefits and costs.

Thomas Owens
My answer would have been quite similar to this if Thomas hadn't answered first. Good answer. A pattern is a guide, not a hard and fast rule.
Odd
+3  A: 

Could the issue here be that the command will be executed by some executor class that will have no direct knowledge of what the command actually does.

If we are talking about adding a return type to the execute method there is a potential for exposing implementation specific return types to the executor. By this I mean that you are opening a door to a situation where different commands might have different sets of return values. If the executor were to the have to handle these then it would become more tightly coupled to the command implementations.

However, I have often given commands state - allowing them to be configured with working values by the client on construction, and then providing getters to allow the client to extract the results of command execution on completion. In this case I may not have strictly followed the command pattern - but the design worked well - and unless there is a definite code smell about this - is this really an issue?

Note: That said, I would be interested to hear thoughts on why this may be a code smell.

teabot
There's another reason to give command a state. When you want to undo them, they have to know how to act. Although this is a matter in itself, full of troubles, when you undo the creation of a line, the command must remember which id it created. Full of landmines as I repeat, since you are not guaranteed to have that id still there (meaning that you could still have the object, but its id changed).
Stefano Borini
I think the second paragraph, above, is the crux of the matter here. The original intention of this pattern is that there is some object that executes the commands but has no knowledge of what they actually do. The question is then: does the executor require knowledge of some non-specific command status (such as pass, fail etc)? If so, add a return type, if not, then don't.
Steg
A: 

In my CAD/CAM Software, the Assemblies containing the Commands references the assemblies containing the interfaces and object hierarchy that holds the various UI elements of my software. It is similar to a Passive View

The Commands can manipulate the UI through the View Interfaces and report any errors themselves.

Basically it goes

Forms implement IFormInterfaces and register themselves with ScreenViews in the EXE

ScreenObjects implement IScreenView and register themselves with the ScreenView assembly as well as grab commands from the command assemblies

Command Assemblies reference the ScreenView Assembly and the Model

ScreenView Assembly little more than a collection of View Interfaces and holds the Application Implementation.

RS Conley
+3  A: 

I'll refer to "Head First Design Patterns". The examples they use for the Command Pattern are:

  1. the diner scenario (customer creates the order, the wait staff invokes it by hollering at the kitchen staff, and the kitchen staff receives the order)
  2. the remote control scenario (person clicks a button, remote control invokes the command and the device receives it)

Obviously in the first case, some kind of state is produced by the receiver: "here's the grub", or "we're out of rye bread". In a fancy restaurant you might do this through exception handling at a higher level (maitre d' comes to the table and apologizes, offers a substitute and comps your dessert), and the wait staff needs to do nothing but properly invoke the commands. But at a diner, maybe the cook goes ahead and substitutes brown bread -- the wait staff (and customer) need to be able to handle that without staring at the counter wondering "where's my tuna on rye?" This isn't addressed directly by the book, but I think it's clearly a valid case.

But in the second scenario, the invoker is deliberately made stupid. It's not going to flash an error at you if something's wrong, it's just going to have no effect at all. All of the smarts are in the client to determine if its command was successful in a timely fashion ("crap, I forgot to plug it in"), or in the receiver to figure out what to do ("play CD: close CD tray first").

I am not an expert, but I would say that returning status to the invoker is totally okay for some applications.

Zac Thompson
+4  A: 

Hi,

As said in your question:

If a command is unsuccessful, the invoking CLIENT must receive a proof of the status, and eventually deploy an appropriate reaction.

In that case, i throw runtime exceptions as status, containing necessary information about it. You could try it.

regards,

Arthur Ronald F D Garcia
+1 for that. Never seen Command Pattern Implementation with return status
Sergey Mirvoda
+1  A: 

There are two questions in the question with multiple answers :) The first question is should a command return an error state?

There is no clear answer for every program every time you apply the pattern you have to think about it again.

One of the things you need to think about is:

  • Am I adding more coupling to many commands and the clients for only some specific error cases?

In the worst case you have many commands that don't care about errors but one or two commands do something that is important for the client to know if it worked. You now add checked Exceptions to the Interface and so every client and every Command are bound to do error handling and are coupled to the Exception. If you have a client that only deals with commands that are not throwing the exceptions you have a big overhead in your code.

This is a thing that you don't want to have. So you can either move the commands that need error handling out of the command structure because they seem to be different from the other commands, or if your language allows it you can add runtime exceptions that are only handled by the clients that care and thrown by the commands that need to throw them.

The other extreme is that every command can fail and the client has a consistent way of handling the errors this means that the errors doesn't depend on the specific command. The client does not have to know what kind of command failed it can handle every error in the same way. Now you could have the interface of the command return an error state and the client can deal with the errors. But dealing with the errors shouldn't depend on the kind of the command for the client.

The second question is: Should a command have a state?

There are architectures where a command needs a state and some where they don't need a state.

Some possibilities to decide this:

  • If you want to have a undo for your command the commands need to have a state.
  • If the commands are only used for hiding a way a function that works on a small set of parameters and the outcomes only depends on the same of the command like the state pattern there is no need for a state and you can use the same object over and over.

  • If you use the command to communicate between threads and you want to transfer data from one thread to another the command needs a state.

  • ... If there is something you think should be in this list leave a comment.
Janusz
+1  A: 

Another compromise is to expose a property "Exception handler" on a concrete command which can fail. This way the creator of the command can handle the exception, and you don't add code overhead to your client. This is very useful when most of your commands shouldn't fail.

Nicolas Dorier