Why Decorator pattern works with pointers but not references?

Viewed 104

I just learned about the decorator pattern and tried to write an example that uses the code. The example is about beverages and some condiments. Inside the Decorator I have a reference variable to a beverage. The beverages available are Decaf and Espresso. The condiments available are Soy and Caramel. If I define a Decaf with more than one Caramel for example, the result I get is just a Decaf with one decorator. So define Caramel->Caramel->Decaf gives me Caramel->Decaf. Defining Caramel->Soy->Caramel->Decaf works fine. Defining Caramel->Soy->Caramel->Caramel->Decaf gives me Caramel->Soy->Caramel->Decaf. Long story short, I can't have two or more condiments of the same type right one after the other. They become only one condiment. If I use pointers it works fine.

The code:

#include <iostream>
//#include "Decaf.h"
//#include "Espresso.h"
//#include "SoyDecorator.h"
//#include "CaramelDecorator.h"

class Beverage
{
public:

    virtual std::string GetDescription() const = 0;
    virtual int GetCost() const = 0;
};

class CondimentDecorator : public Beverage
{
public:

    Beverage& beverage;
    CondimentDecorator(Beverage& beverage) : beverage(beverage) {}
};

class Espresso : public Beverage
{
    virtual std::string GetDescription() const override
    {
        return "Espresso";
    }

    virtual int GetCost() const override
    {
        return 5;
    }
};

class Decaf : public Beverage
{
    virtual std::string GetDescription() const override
    {
        return "Decaf";
    }

    virtual int GetCost() const override
    {
        return 4;
    }
};

class CaramelDecorator : public CondimentDecorator
{
public:

    CaramelDecorator(Beverage& beverage) : CondimentDecorator(beverage) {}

    virtual std::string GetDescription() const override
    {
        return this->beverage.GetDescription() + " with Caramel";
    }

    virtual int GetCost() const override
    {
        return this->beverage.GetCost() + 2;
    }
};

class SoyDecorator : public CondimentDecorator
{
public:

    SoyDecorator(Beverage& beverage) : CondimentDecorator(beverage) {}

    virtual std::string GetDescription() const override
    {
        return this->beverage.GetDescription() + " with Soy";
    }

    virtual int GetCost() const override
    {
        return this->beverage.GetCost() + 1;
    }
};

int main()
{
    Decaf d;
    SoyDecorator s(d);
    CaramelDecorator c(s);
    CaramelDecorator cc(c);

    std::cout << cc.GetDescription() << std::endl;
    std::cout << cc.GetCost() << std::endl;
}

output:

Decaf with Soy with Caramel
7

// Expected:
// Decaf with Soy with Caramel with Caramel
// 9

Here is the same code but using pointers and works just fine: https://ideone.com/7fpGSp

1 Answers

With switching from pointers to references, OPs constructor signature becomes very similar to the (default) copy constructor.

    CondimentDecorator(Beverage &beverage) : beverage(beverage) {}

vs.

    CondimentDecorator(const Beverage&); // generated by compiler

First, I assumed to delete the copy constructor would be sufficient but the compiler still tries to use the deleted constructor with a respective complaint as it cannot anymore.

Finally, I was able to fix OP's issue with providing the resp. candidates which prevent using the copy constructor.

(Deleting of copy constructor wasn't actually anymore needed but I left it in.)

class CondimentDecorator : public Beverage
{
public:

    Beverage& beverage;
    CondimentDecorator(Beverage &beverage) : beverage(beverage) {}
    CondimentDecorator(CondimentDecorator &beverage) : beverage(beverage) {}
    CondimentDecorator(const CondimentDecorator&) = delete;
};

The same has to be done for derived classes:

class CaramelDecorator : public CondimentDecorator
{
public:

    CaramelDecorator(Beverage &beverage) : CondimentDecorator(beverage) {}
    CaramelDecorator(CaramelDecorator &beverage) : CondimentDecorator(beverage) {}
    //CaramelDecorator(const CaramelDecorator&) = delete;

    virtual std::string GetDescription() const override
    {
        return this->beverage.GetDescription() + " with Caramel";
    }

    virtual int GetCost() const override
    {
        return this->beverage.GetCost() + 2;
    }
};

I fixed only the CaramelDecorator for demo but, actually, this has to be done for all derived classes of class CondimentDecorator.

The fixed MCVE of OP:

#include <iostream>
//#include "Decaf.h"
//#include "Espresso.h"
//#include "SoyDecorator.h"
//#include "CaramelDecorator.h"

class Beverage
{
public:
    virtual std::string GetDescription() const = 0;
    virtual int GetCost() const = 0;
};

class CondimentDecorator : public Beverage
{
public:

    Beverage& beverage;
    CondimentDecorator(Beverage &beverage) : beverage(beverage) {}
    CondimentDecorator(CondimentDecorator &beverage) : beverage(beverage) {}
    CondimentDecorator(const CondimentDecorator&) = delete;
};

class Espresso : public Beverage
{
    virtual std::string GetDescription() const override
    {
        return "Espresso";
    }

    virtual int GetCost() const override
    {
        return 5;
    }
};

class Decaf : public Beverage
{
    virtual std::string GetDescription() const override
    {
        return "Decaf";
    }

    virtual int GetCost() const override
    {
        return 4;
    }
};

class CaramelDecorator : public CondimentDecorator
{
public:

    CaramelDecorator(Beverage &beverage) : CondimentDecorator(beverage) {}
    CaramelDecorator(CaramelDecorator &beverage) : CondimentDecorator(beverage) {}
    //CaramelDecorator(const CaramelDecorator&) = delete;

    virtual std::string GetDescription() const override
    {
        return this->beverage.GetDescription() + " with Caramel";
    }

    virtual int GetCost() const override
    {
        return this->beverage.GetCost() + 2;
    }
};

class SoyDecorator : public CondimentDecorator
{
public:

    SoyDecorator(Beverage &beverage) : CondimentDecorator(beverage) {}

    virtual std::string GetDescription() const override
    {
        return this->beverage.GetDescription() + " with Soy";
    }

    virtual int GetCost() const override
    {
        return this->beverage.GetCost() + 1;
    }
};

int main()
{
    Decaf d;
    SoyDecorator s(d);
    CaramelDecorator c(s);
    CaramelDecorator cc(c);

    std::cout << cc.GetDescription() << std::endl;
    std::cout << cc.GetCost() << std::endl;
}

Output:

Decaf with Soy with Caramel with Caramel
9

Live Demo on coliru


Why the additional candidates are needed?

CondimentDecorator is derived from Beverage.

So, for:

CondimentDecorator d;
CondimentDecorator d2(d);

the compiler has two choices to construct d2:

  1. the custom constructor CondimentDecorator::CondimentDecorator(Beverage &beverage)
  2. the (default) copy constructor CondimentDecorator::CondimentDecorator(const CondimentDecorator&).

For the first, an implicit cast has to be applied but for the copy constructor, no cast is necessary (or at most, a const-cast).

Hence, the compiler prefers the copy constructor (unfortunately even, although it is deleted).

So, another candidate has to be provided which requires as less as implicit casts like the copy constructor:

  1. another custom constructor CondimentDecorator::CondimentDecorator(CondimentDecorator&).

Further reading: Overload Resolution

Related