C++ implementation of the Command Pattern

Question

Below is a C++ implementation of the Command Pattern. I wrote three receivers that carry out the intended action (making a sound) differently. I also wrote command and invoking classes. I saw no need to write a client class, so main() is the client.

#include <iostream>
#include <string>

// receivers

class Cow {
public:
    void moo() {
        std::cout << "Cow says moo!" << std::endl;
    }
};

class Dog {
    std::string name;
public:
    Dog(const std::string& name) : name(name) {}

    void makeSound() {
        std::cout << name + " barks!" << std::endl;
    }
};

class Car {
    bool engineOn;
public:
    Car() : engineOn(false) {};

    void turnCarOn() {
        engineOn = true;
    }

    void revEngine() {
        if (engineOn) {
            std::cout << "Vroom, vroom, vroom!" << std::endl;
        }
        else {
            std::cout << "Silence!" << std::endl;
        }
    }
};

// commands

class Command {
public:
    virtual void execute() = 0;
};

class CowCommand : public Command {
    Cow cow;
public:
    CowCommand(Cow cow) : cow(cow) {}

    void execute() {
        cow.moo();
    }
};

class DogCommand : public Command {
    Dog dog;
public:
    DogCommand(Dog dog) : dog(dog) {}

    void execute() {
        dog.makeSound();
    }
};

class CarCommand : public Command {
    Car car;
public:
    CarCommand(Car car) : car(car) {}

    void execute() {
        car.turnCarOn();
        car.revEngine();
    }
};

// invoker

class PlatoSays {
    Command* ptr_command;
public:
    PlatoSays() : ptr_command(nullptr) {}

    void setCommand(Command* ptr_cmd) {
        ptr_command = ptr_cmd;
    }

    void invokeCommand() {
        if (ptr_command) {
            std::cout << "Plato says: ";
            ptr_command->execute();
        }
        else {
            std::cout << "Plato has nothing to say!" << std::endl;
        }
    }
};

// client

int main() {
    PlatoSays plato;

    plato.setCommand(new CowCommand(Cow()));
    plato.invokeCommand();

    plato.setCommand(new DogCommand(Dog("Lucky")));
    plato.invokeCommand();

    plato.setCommand(new CarCommand(Car()));
    plato.invokeCommand();

    getchar();
}

Answer 1

Virtual Destructor

If a class has virtual methods it should also have a virtual destructor. Now this is only important if you destroy a class from a pointer to the base class. Since your code leaks all the objects it will not hurt you (Note: I am being facetious). Both leaking and incorrectly destroying objects is a big deal. This is after all not C and thus we like to actually write working programs.

class Command {
public:
    virtual ~Command() {}      // Declare a virtual destuctor in
                               // the base class. All derived types
                               // will inherit a virtual destructor.
    virtual void execute() = 0;
};

Override Specifier

In C++11 we added the override specifier. You are supposed to use this to make virtual functions you are overriding. It provides a nice safety blanket, if somebody in the future changes the virtual function definition then the override specifier in your class will cause an error if you have not changed the definition in your class to match.

class CowCommand : public Command {
    Cow cow;
public:
    CowCommand(Cow cow) : cow(cow) {}

    void execute() override {   // Add override here.
        cow.moo(); 
    }
};

Don't pass by value (Pass by const ref as a first default).

If you pass by value you are causing a copy of the object. This may be unnecessarily expensive. Pass by const reference. If you need to then you can make a copy before using.

Alternatively you can pass by R-Value reference and move the object in the methods.

    Cow cow;
public:
    // You make two copies of cow here.
    CowCommand(Cow cow)       // A copy is made to pass it to the method.
        : cow(cow)            // A second copy is made passing while
                              // creating the local `cow` object.
    {}


    // I would have done this:
    CowCommand(Cow const& cow)   // Pass a normal reference
        : cow(cow)               // Now make a copy (only 1 copy)
    {}
    CowCommand(Cow&& cow)        // Pass an r-value reference
        : cow(std::move(cow))    // Move the object into cow.
    {}

Don't pass pointers

A pointer has no concept of ownership. Ownership decides who is supposed to delete the object. If I don't know who is supposed to delete I may accidentally delete something that is not dynamic or if I don't delete something I am supposed to then I leak an object.

The idea is that public interfaces should pass things around in a way that ownership of the object is clear (private and internal methods can use pointers for efficiency).

This public function passes a pointer.

void setCommand(Command* ptr_cmd) {
    ptr_command = ptr_cmd;
}

Internally you store it as a pointer. But you don't take ownership (i.e. you don't call delete on the pointer in the destructor). So this object can easily be used incorrectly.

{
    PlatoSays  plato;
    plato.setCommand(new LokiCommand);
}

Here I have created a dynamic object and passed in it setCommand() (which has the only copy of this pointer. At the end of scope the LokiCommand object is leaked.

If you are not going to take ownership then pass by reference. This indicates to the method you can have a link to my object but I will delete it (not you).

void setCommand(Command& ptr_cmd) {
    ptr_command = &ptr_cmd;   // Its OK to store a pointer to the object
                              // Just mean Plato has to be created first
                              // and destroyed last.
}

Note: If you do want to pass ownership. Then wrap your pointer in a smart pointer. This will take care of the ownership.

class PlatoSays {
    std::unique_ptr<Command> ptr_command;
public:
    void setCommand(std::unique_ptr<Command>&& ptr_cmd) {
        ptr_command = std::move(ptr_cmd);
    }

    // Notice the usage of a unique_ptr is exactly the same as the
    // usage for a normal pointer. The only difference is that it
    // Correctly initializes itself (so you don't need a constructor)
    // And when updating it you need to use `std::move()`
    void invokeCommand() {
        if (ptr_command) {
            std::cout << "Plato says: ";
            ptr_command->execute();
        }
        else {
            std::cout << "Plato has nothing to say!" << std::endl;
        }
    }
};

Leaks in your program.

And here we see the exact problem with pointers.

plato.setCommand(new CowCommand(Cow()));
plato.invokeCommand();

plato.setCommand(new DogCommand(Dog("Lucky")));
plato.invokeCommand();

plato.setCommand(new CarCommand(Car()));
plato.invokeCommand();

All three of these objects are leaked.

Use when ownership is not transferred:

PlatoSays plato;
CowCommand   cow(Cow());
DogCommand   dog(Dog("Lucky");
CarCommand   car(Car());

plato.setCommand(cow);
plato.invokeCommand();

plato.setCommand(dog);
plato.invokeCommand();

plato.setCommand(car);
plato.invokeCommand();

Use when ownership is transferred:

PlatoSays plato;
plato.setCommand(std::unique_ptr<Command>(new CowCommand(Cow()));
plato.invokeCommand();

plato.setCommand(std::unique_ptr<Command>(new DogCommand(Dog("Lucky"))));
plato.invokeCommand();

plato.setCommand(std::unique_ptr<Command>(new CarCommand(Car())));
plato.invokeCommand();

Prefer "\n" over std::endl

The only difference is that std::endl flushes the stream. Manual programmer flushing is the usually non optimal and the cause of most slow downs when comparing C++ code to C code speed.