Questions about Command Pattern (PHP)

Question

i did up a minimalistic Command Pattern example in PHP after reading up about it. i have a few questions ...

i'll like to know if what i did is right? or maybe too minimal, thus reducing the point of the command pattern

interface ICommand {
  function execute($params);
}
class LoginCommand implements ICommand {
  function execute($params) {
    echo "Logging in : $params[user] / $params[pass] <br />";
    $user = array($params["user"], $params["pass"]);
    // faked users data
    $users = array(
      array("user1", "pass1"),
      array("user2", "pass2")
    );

    if (in_array($user, $users)) {
      return true;
    } else {
      return false;
    }
  }
}

$loginCommand = new LoginCommand();
// $tries simulate multiple user postbacks with various inputs
$tries = array(
  array("user" => "user1", "pass" => "pass1"),
  array("user" => "user2", "pass" => "pass1"),
  array("user" => "user2", "pass" => "PaSs2")
);

foreach ($tries as $params) {
  echo $loginCommand->execute($params) ? " - Login succeeded!" : " - Login FAILED!";
  echo " <br />";
}

i am wondering if there is any difference from simply putting this LoginCommand into a simple function say in the Users class?

if LoginCommand is better fit for a class, won't it be better if it were a static class so i can simply call LoginCommand::execute() vs needing to instanciate an object 1st?

Answer 1

The point of the Command Pattern is being able to isolate distinct functionality into an object (the command), so it can be reused across multiple other objects (the commanders). Usually, the Commander also passes a Receiver to the Command, e.g. an object that the command is targeted at. For instance:

$car = new Car;
echo $car->getStatus(); // Dirty as Hell
$carWash = new CarWash;
$carWash->addProgramme('standard',
                        new CarSimpleWashCommand, 
                        new CarDryCommand, 
                        new CarWaxCommand);
$carWash->wash();
echo $car->getStatus(); // Washed, Dry and Waxed

In the above example, CarWash is the Commander. The Car is the Receiver and the programme are the actual Commands. Of course I could have had a method doStandardWash() in CarWash and made each command a method in CarWash, but that is less extensible. I would have to add a new method and command whenever I wanted to add new programmes. With the command pattern, I can simply pass in new Commands (think Callback) and create new combinations easily:

$carWash->addProgramme('motorwash',
                        new CarSimpleWashCommand, 
                        new CarMotorWashCommand,
                        new CarDryCommand, 
                        new CarWaxCommand);

Of course, you could use PHP's closures or functors for this too, but let's stick to OOP for this example. Another thing where the Commands come in handy, is when you have more than one Commander that needs the Command functionality, e.g.

$dude = new Dude;
$dude->assignTask('washMyCarPlease', new CarSimpleWashCommand);
$dude->do('washMyCarPlease', new Car);

If we had hardcoded the washing logic into the CarWash, we would now have to duplicate all code in the Dude. And since a Dude can do many things (because he is human), the list of tasks he can do, will result in a terrible long class.

Often, the Commander itself is also a Command, so you can create a Composite of Commands and stack them into a tree. Commands often provide an Undo method as well.

Now, looking back at your LoginCommand, I'd say it doesn't make much sense to do it this way. You have no Command object (it's the global scope) and your Command has no Receiver. Instead it returns to the Commander (which makes the global scope the Receiver). So your Command does not really operate on the Receiver. It is also unlikely, that you will need the abstraction into an Command, when doing the login is only ever done in one place. In this case, I'd agree the LoginCommand is better placed into an Authentication adapter, maybe with a Strategy pattern:

interface IAuthAdapter { public function authenticate($username, $password); } 
class DbAuth implements IAuthAdapter { /* authenticate against database */ }
class MockAuth implements IAuthAdapter { /* for UnitTesting */ }

$service = new AuthService();
$service->setAdapter(new DbAuth);
if( $service->authenticate('JohnDoe', 'thx1183') ) {
    echo 'Successfully Logged in';
};

You could do it somewhat more Command-like:

$service = new LoginCommander;
$service->setAdapter(new DbAuth);
$service->authenticate(new User('JohnDoe', 'thx1138'));
if($user->isAuthenticated()) { /* ... */}

You could add the authenticate method to the User of course, but then you would have to set the Database adapter to the User in order to do the authentication, e.g.

$user = new User('JohnDoe', 'thx1138', new DbAuth);
if ( $user->authenticate() ) { /* ... */ }

That would be possible too, but personally, I don't see why a User should have an Authentication adapter. It doesn't sound like something a user should have. A user has the Credentials required by an Authentication adapter, but the not the adapter itself. Passing the adapter to the user's authenticate method would be an option though:

$user = new User('JohnDoe', 'thx1138');
if ( $user->authenticateAgainst($someAuthAdapter) ) { /* ... */ }

Then again, if you are using ActiveRecord, then your user will know about the database anyway and then you could simply dump all the above and write the entire authenticatation code into the user.

As you can see, it boils down to how you are setting up your application. And that brings us to to the most important point: Design Patterns offer solutions to common problems and they let us allow to speak about these without having to define tons of terms first. That's cool, but often, you will have to modify the patterns to make them solve your concrete problem. You can spend hours theorizing about architecture and which patterns to use and you wont have written a single code. Don't think too much about if a pattern is 100% true to the suggested definition. Make sure your problem is solved.