Factory Method Design Pattern Implementation as a Coffee maker program

Question

File: InfoFactory.h contains four classes.

1. CaffienatedBeverage is an Abstract class with one pure virtual function Name. It also contains some data members that are used to make a coffee based drink and name of the drink.
2. Latte is a class that inherits CaffienatedBeverage class. Latte contains milk and coffee.
3. Expresso is another class that inherits CaffienatedBeverage class and is made of water and coffee.
4. BeverageFactory class that contains a static method createBeverage which takes input and creates a Caffienated Beverage.

#include <iostream>


class CaffeinatedBeverage
{
private:
    bool _water;
    bool _milk;
    bool _coffee;
    std::string _name;
public:
    CaffeinatedBeverage() : _water{ false }, _milk{ false }, _coffee{ false }, _name{nullptr}{};
    CaffeinatedBeverage(bool water, bool milk, bool coffee, std::string str)
    {
        _water = water;
        _milk = milk;
        _coffee = coffee;
        _name = str;
    }
    ~CaffeinatedBeverage(){}
    virtual std::string Name() = 0;
};

class Latte : public CaffeinatedBeverage
{
public:
    Latte(): CaffeinatedBeverage(false, true, true, std::string("Latte"))
    {
        std::cout << "Latte is a Coffee based drink that contains milk\n";
    }
    ~Latte() {};

    std::string Name() override;

};

class Espresso : public CaffeinatedBeverage
{
public:
    Espresso(): CaffeinatedBeverage(true, false, true, std::string("Espresso"))
    {
        std::cout << "Creates \n";
    }
    ~Espresso() {};
    std::string Name() override;
};


class BeverageFactory
{
public:
    static CaffeinatedBeverage* createBeverage();
};

File InfoFactory.cxx

#include <iostream>
#include "InfoFactory.h"


std::string Latte::Name()
{
    return "\n\n\n\nLatte\n\n\n\n";
}

std::string Espresso::Name()
{
    return "\n\n\nEspresso\n\n\n";
}


CaffeinatedBeverage* BeverageFactory::createBeverage()
{
    int option = -1;
    std::cout << "Please enter 1 for Latte and 2 for Espresso. To exit please enter 0. ";
    std::cin >> option;
    while (option)
    {
        switch (option)
        {
        case 1:
            return new Latte();
            break;
        case 2:
            return new Espresso();
            break;
        case 0:
            break;
        default:
            std::cout << "Please try again: ";
            std::cin >> option;
        }
    }
}

File main.cxx

#include <iostream>
#include "InfoFactory.h"

int main()
{

    auto beverage = BeverageFactory::createBeverage();

    std::cout << beverage->Name() << std::endl;
    return 0;
}

Does this code implements Factory Method desing pattern correctly?

Answer 1

Memory management and cleanup with inheritance

Objects allocated with new must be cleaned up by calling delete. In this case the beverage object is never cleaned up. In modern (and not so modern) C++ we should use a std::unique_ptr instead of doing manual memory management.

When we call member functions on a pointer to the base-class (CaffeinatedBeverage), it will call the base-class member function if that function is not marked virtual. The destructor behaves in a similar way. So when destroying an object from a base class pointer, we must make the destructor virtual to ensure the derived destructor is called.

(Note that virtual destructors are not quite the same as virtual functions - the base class destructor will be called automatically after the virtual derived class destructor, so we don't need to that manually.)

---

Undefined behavior

CaffeinatedBeverage() : _water{ false }, _milk{ false }, _coffee{ false }, _name{nullptr}{};

Passing a nullptr to a std::string constructor is actually undefined behavior (and will be explicitly prevented in C++23). The relevant constructor requires a null-terminated character string.

We can create an empty std::string by using the default constructor (_name{}) or simply omitting it from the initializer list.

---

Constructor initializer list and std::move

CaffeinatedBeverage(bool water, bool milk, bool coffee, std::string str)
{
    _water = water;
    _milk = milk;
    _coffee = coffee;
    _name = str;
}

We should also use the constructor initializer list here. Since we make a copy of the name when taking it as an argument, we can move that copy into place instead of copying it again: _name{ std::move(str) }.

---

Prefer free functions when we have no class state

class BeverageFactory
{
public:
    static CaffeinatedBeverage* createBeverage();
};

This class has no state (member variables), and only a single static function. This implies that it should not be a class. It can be a simple function instead.

---

Simplify

Note that currently there is no need to use inheritance here. The name is stored in the base class, and there is no significant difference in behavior between the classes. We can use a simple struct to store the data, such as:

struct CoffeeDrink
{
    std::string name;
    bool water;
    bool milk;
    bool coffee;
};

Answer 2

class CaffeinatedBeverage
{
private:
    bool _water;
    bool _milk;
    bool _coffee;
    std::string _name;
public:
    CaffeinatedBeverage() : _water{ false }, _milk{ false }, _coffee{ false }, _name{nullptr}{};
    ⋮

You should use inline initializers and then you don't need the default constructor to be spelled out. Note that string has a constructor and defaults to being empty, so you don't need to specify an initializer for that.

Write:

    bool _water = false;
    bool _milk = false;
    bool _coffee = false;
    std::string _name;
public:
    CaffeinatedBeverage() = default;

Continuing...

        ⋮
    CaffeinatedBeverage(bool water, bool milk, bool coffee, std::string str)
    {
        _water = water;
        _milk = milk;
        _coffee = coffee;
        _name = str;
    }
    ~CaffeinatedBeverage(){}
    virtual std::string Name() = 0;
};

This constructor should use the member init list, not assignments in the constructor function body. It is strange that str is being passed by value. In this case you could use the "sink" idiom to avoid copying, but I don't think you were aware of that. More generally, pass string values using const & parameters, or use string_view where you can.

The destructor should also be declared virtual. And don't define trivial destructors (or other special members) like that, as it makes the compiler not understand that it really is the trivial or same-as-automatic stuff. Use:

virtual ~CaffeinatedBeverage() =default;

---

Latte(): CaffeinatedBeverage(false, true, true, std::string("Latte"))

Why are you explicitly constructing string here? Just passing "Latte" by itself to the std::string parameter would work, and is idiomatic.

Answer 3

I see the other answers have focused on the use of C++, so I'll briefly comment about the actual use of the design pattern.

You are indeed using the factory method pattern.

After all, this pattern just needs for a method to create an instance of a certain class. The actual instace created (one of its children) will be chosen depending on a certain condition.

In your case, this was a simple stdin prompt. In a real-world application, the decision could have been made by the user iteracting with the UI.