reduce code duplication - factory design pattern

Question

I have created a small chess game and implemented a factory class to produce game pieces. However, I realized that my implementation is not ideal, as every game piece is responsible for registering itself with the factory. For instance, the bishop registers itself with the following code:

// white bishop registration
bool Bishop::m_registerit_white =
Factory<GamePiece>::registerit
(
    'B', []()->std::unique_ptr<GamePiece>
    { return std::make_unique<Bishop>(WHITE); }
);

// black bishop registration
bool Bishop::m_registerit_black =
Factory<GamePiece>::registerit
(
    'b', []()->std::unique_ptr<GamePiece>
    { return std::make_unique<Bishop>(BLACK); }
);

My teacher pointed out that these registration calls result in A LOT of code duplication for each piece (I agree). He suggested that I reduce the duplication by creating a map between characters and their corresponding classes, so that I could call the class constructor when needed, instead of having each piece register itself with the factory.

I am not entirely sure how to implement this suggestion and would appreciate some guidance.

This is my factory class:

template<typename T>
class Factory {

public:
    //the pointer to the function that creates the object
    using creationFunc = unique_ptr<T>(*)();

    //register a pair of char+creator function into the map:
    static bool registerit(const char& name, creationFunc f) {
        getMap().emplace(name, f);
        return true;
    }

    //creates the objects according to the char that represents it
    //(searches this char in the map):
    static unique_ptr<T> create(const char& name) {
        const auto& map = getMap();
        auto it = map.find(name);
        if (it == map.end())
            return nullptr;
        return it->second();
    }

private:
    static auto& getMap() {
        static map<char, creationFunc> map;
        return map;
    }
};

Unfortunately, Stackoverflow is not a C++ help site or a tutorial site, we only answer ***specific*** questions. It's also not helpful that your instructor is apparently incompetent. There is no such thing as a " a map between characters and their corresponding classes" in C++. A map is a map of objects. A class is not an object. There's no such thing as a map of classes in C++, that's nonsensical. You need to go back to your instructor and ask for a clarification. This is what instructors are for, to help their students learn, that's what they're paid to do. — Sam Varshavchik, Apr 28 '23 at 11:42
Do you really need to register black and white separately? Can't you pass the colour as a parameter to your registration function? — Alan Birtles, Apr 28 '23 at 11:44
I dont see the issue with duplication. the code you post is populating a map with char key and a function to create object of the right type. Thats what the code does and not more. And thats about as close as you can get to a "map between char and type" — 463035818_is_not_an_ai, Apr 28 '23 at 11:46
Viewing this from a different angle - do we really *need* a factory function to create the two white bishops on a chess board? — BoP, Apr 28 '23 at 11:55

score 2 · Accepted Answer · answered Apr 28 '23 at 11:56

If all the pieces are constructed the same way you could create a templated utility function or class to perform the registration.

For example:

template <typename Piece>
struct PieceRegister
{
    PieceRegister(char white, char black)
    {
        Factory<GamePiece>::registerit
        (
            white, []()->std::unique_ptr<GamePiece>
            { return std::make_unique<Piece>(WHITE); }
        );

        Factory<GamePiece>::registerit
        (
            black, []()->std::unique_ptr<GamePiece>
            { return std::make_unique<Piece>(BLACK); }
        );
    }
};

PieceRegister<Bishop> bishopRegister('B', 'b');

reduce code duplication - factory design pattern

1 Answers1