3
\$\begingroup\$

I recently tried to implement the decorator design pattern in C++. Here is the code in full:

#include <algorithm>
#include <iostream>
#include <memory>
#include <numeric>
#include <string>
#include <vector>

//Interface for items of food
class Food {
public:
    virtual float Price() const = 0; //The price of the food in some currency
    virtual std::vector<std::string>  IngredientsList() const = 0;
    virtual ~Food() = default; 
};

//An example of an item of food (concrete component)
class Pizza : public Food {
public:
    float Price() const override {return _base_cost;};
    std::vector<std::string> IngredientsList() const override {return {"Dough"};}
private:
    float _base_cost{3.0}; //The cost of a pizza with no toppings

};

//Interface for decorators
class IngredientsDecorator : public Food {
public:
    explicit IngredientsDecorator(Food* food) : _food{food} {}

    virtual float Price() const override {return _food->Price();}
    virtual std::vector<std::string> IngredientsList() const override {return _food->IngredientsList();}
protected:
    Food* _food;
};
    
//An example of a decorator
class MozzarellaDecorator : public IngredientsDecorator {
public:
    explicit MozzarellaDecorator(std::shared_ptr<Food> food)    
        : IngredientsDecorator{food.get()} {} 

    float Price() const override  {return IngredientsDecorator::Price() + _cost;}; 

    std::vector<std::string> IngredientsList() const override {
        auto AllIngredients = IngredientsDecorator::IngredientsList();
        auto MozzarellaIngredients = Ingredients();
        AllIngredients.insert(AllIngredients.end(), MozzarellaIngredients.begin(), MozzarellaIngredients.end()); 
        return AllIngredients;
        }

private:
    float _cost{0.50}; 
    std::vector<std::string> Ingredients() const {return {"Mozzarella"};};
};

//An order consists of a number of food items

class Order {
public:
    
    void AddToOrder(std::shared_ptr<Food> food) {
        _food.push_back(food.get());
    }

    void ClearOrder() {_food.clear();}

    float Price() const {
        return std::accumulate(_food.begin(),_food.end(),0.f, [](float a, Food* b) -> float {return a + b->Price();});
        }

    std::vector<std::string> IngredientsList() const {
        std::vector<std::string> all_ingredients;
        std::for_each(_food.begin(),
                      _food.end(), 
                      [&all_ingredients](Food* f) {
                        auto curr_ingredients = f->IngredientsList();
                        all_ingredients.insert(all_ingredients.end(), curr_ingredients.begin(), curr_ingredients.end());
                        });
        return all_ingredients;
        }


    void PrintOrderInformation() const {
        std::cout<<Price()<<'\n';
        auto ingredients{IngredientsList()};
        for(const auto& e : ingredients) {
            std::cout<<e<<'\n';
        }
        std::cout<<"\n-------------\n";
    };

private:
    std::vector<Food*> _food; 
};



int main()
{
    
        //Create an empty order
    Order my_order;

    //Add a mozzarella pizza
    auto pizza_base = std::make_shared<Pizza>();
    auto mozzarella_pizza = std::make_shared<MozzarellaDecorator>(pizza_base);
    my_order.AddToOrder(mozzarella_pizza);
    my_order.PrintOrderInformation();
    my_order.ClearOrder();

    //New order for a pizza with double mozzarella
    auto double_mozzarella_pizza = std::make_shared<MozzarellaDecorator>(mozzarella_pizza); 
    my_order.AddToOrder(double_mozzarella_pizza);
    my_order.PrintOrderInformation();
    my_order.ClearOrder();

}

My questions are the following:

  • Have I implemented the decorator pattern correctly?
  • Have I used smart pointers correctly? I'm not 100% sure whether the functions should take a smart pointer or a raw pointer.
  • Is there a way to compose decorators in a constructor? Specifically, is there a way I can write the following
auto base_pizza = std::make_shared<Pizza>();
auto single_mozzarella = std::make_shared<MozzarellaDecorator>(base_pizza);

as something like this instead?

auto single_mozzarella = std::make_shared<MozzarellaDecorator>(std::make_shared<Pizza>());

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. It's best to describe what value this code provides to its user. \$\endgroup\$ Jun 2, 2023 at 19:50

1 Answer 1

4
\$\begingroup\$

The goal of design patterns

The goal of design patterns and principles is so we create more flexible and maintainable code. In particular, I would say they are there so we keep the act of programming itself \$O(1)\$. Consider for example why the decorator pattern was created: without it, if you would want to extend a class with some extra functionality, you might create a derived class:

class Pizza: Food {…};
class PizzaWithExtraMozzarella: Pizza {…};
class PizzaWithExtraAnchovies: Pizza {…};

The problem is now: what if I want a pizza with anchovies and double mozzarella? Do I really want to add another class for that, like the following?

class PizzaWithAnchoviesAndDoubleMozzarella: Pizza {…};

Because that way, if you have \$N\$ ingredients to choose from, you'd need at least \$N^2\$ extra classes just so you can get any combination of one extra ingredient, and that doesn't even cover adding the same ingredient twice. The decorator pattern allows you to avoid the \$N^2\$ explosion.

However, if we have to write a separate decorator class for each of the \$N\$ possible ingredients, we still end up having to write \$O(N)\$ classes. That's still not great! This is why it would be much better to just have a mutable list of ingredients, with an AddIngredient() function we can call to add more ingredients. This function could either be added to the base class Food itself, or we can create one decorator that provides that functionality:

class ExtraIngredientsDecorator: public Food {
public:
    explicit ExtraIngredientsDecorator(std::unique_ptr<Food> food):
        _food{std::move(food)} {}

    virtual float Price() const override {
         return _food->Price() + _extra_cost;
    }

    virtual std::vector<std::string> IngredientsList() const override {
        auto AllIngredients = _food->IngredientsList();
        AllIngredients.insert(AllIngredients.end(),
                              _extra_ingredients.begin(), _extra_ingredients.end());
        return AllIngredients;
    }

    void AddIngredient(std::string ingredient, float cost) {
        _extra_ingredients.push_back(ingredient);
        _extra_cost += cost;
    }

private:
    std::unique_ptr<Food> _food;
    float _extra_cost{};
    std::vector<std::string> _extra_ingredients;
};

Because now you can write:

auto pizza_with_extras = ExtraIngredientsDecorator(std::make_unique<Pizza>);
pizza_with_extras.AddIngredient("anchovies", 0.40);
pizza_with_extras.AddIngredient("mozzarella", 0.50);
pizza_with_extras.AddIngredient("mozzarella", 0.50);

Prefer std::unique_ptr if possible

You used both std::shared_ptr and raw pointers in your code. The former does atomic reference counting, which might be unnecessary, and the latter is not very safe. While std::shared_ptr works in your example because every object is immutable, if you have mutable data (like in my ExtraIngredientsDecorator), then with std::shared_ptr being used behind the scenes it becomes hard to understand what happens when copies made of objects. For example, will other copies get modified as well when you call something like AddIngredient() on one of them?

std::unique_ptr has none of these issues. It's clear that there can be only one owner. I've shown it in the example above. The only issue is that you might have to explicitly std::move() objects if they were not temporaries, like for example:

auto pizza_base = std::make_unique<Pizza>;
auto pizza_with_extras = ExtraIngredientsDecorator(std::move(pizza_base));

Also turn your raw pointers into std::unique_ptr; this avoids the possibility of dangling pointers. So:

class Order {
public:
    void AddToOrder(std::unique_ptr<Food> food) {
        _food.push_back(std::move(food));
    }
    …
private:
    std::vector<std::unique_ptr<Food>> _food;
};

And then a full usage example would be:

Order my_order;

// Add a mozzarella pizza
{
    auto pizza_base = std::make_unique<Pizza>;
    auto pizza_with_extras =
        std::make_unique<ExtraIngredientsDecorator>(std::move(pizza_base));
    pizza_with_extras.AddIngredient("mozzarella", 0.50);
    my_order.AddToOrder(std::move(pizza_with_extras));
}

my_order.PrintOrderInformation();

The above code is safe, even though pizza_base and pizza_with_extras went out of scope before PrintOrderInformation() was called.

Composing decorators

You can indeed compose decorators the way you wrote it. It's the same when using unique pointers. You could even write something like:

Order my_order;
my_order.AddToOrder(
    std::make_unique<ExtraIngredientsDecorator>(
        std::make_unique<Pizza>()
    )
);
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for review. Your description of the patterns is very helpful for understanding. I was a bit confused about ownership hence the raw pointers. I do have one question regarding replacing decorators with a single AddIngredient() function: If I wanted to add more data to each Ingredient decorator, the AddIngredient() function would become large as you essentially need to pass all the associated data into it (cost in this case). It also seems like you would be losing some encapsulation for a similar reason. Would it still be better this way if ingredients had more data attached to them? \$\endgroup\$
    – Sc2046
    Jun 6, 2023 at 10:16
  • 1
    \$\begingroup\$ I would create an Ingredient class then that encapsulates all the data of an ingredient. You could make that a base class so you can extend Ingredients with decorators as well. \$\endgroup\$
    – G. Sliepen
    Jun 6, 2023 at 10:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.