10
\$\begingroup\$

Design

A template class that will create a concrete instance of an interface class when given the name (as a string) of the concrete class.

This design is often seen in code, so the purpose of this class is to reduce the boilerplate code needed to write a factory class.

The template class will be used like this:

using MyFactory = factory<MyInterfaceType, MyType_1, MyType_2, MyType_3>;

int main() {
    auto ptr = MyFactory::create("MyType_1");
    ptr->doSomething();
}

There will be a static assert to check that all of the concrete types inherit from the interface type and the create method can accept an optional argument to describe the return type if no class matching the id is found.

Implementation

template<typename interface_type, typename... products>
struct factory {

    template<typename Class>
    struct LabledClass {
        std::string_view label = to_string<Class>();
        Class data;
    };

    using labled_classes = std::tuple<LabledClass<products>...>;

    static std::unique_ptr<interface_type> create(const std::string_view& id, std::unique_ptr<interface_type> default_type = nullptr) {
        std::unique_ptr<interface_type> result = std::move(default_type);

        //checl all products inherit from interface_type
        std::apply([](auto&&... tuple_elem) {
            ((static_check<decltype(tuple_elem.data)>()), ...);
        }, labled_classes{} );

        //if product matches id, return a new instance of that product
        std::apply([&result, &id](auto&&... tuple_elem) {
            (( tuple_elem.label == id ? result = std::make_unique<decltype(tuple_elem.data)>() : result ), ...);
        }, labled_classes{} );

        return result;
    }

private:
    template<typename product>
    static void static_check() {
        static_assert(std::is_base_of<interface_type, product>::value, "all products must inherit from interface_type");
    }
};

This requires the function to_string<Class>() which does not yet exist in the c++ standard, but can be implemented on GCC or Clang like so:

template<typename Class>
constexpr std::string_view to_string() {
    std::string_view str = __PRETTY_FUNCTION__;
    auto first = str.find("= ");
    std::string_view str2 = str.substr(first + 2);
    auto last = str2.find(";");
    str2.remove_suffix(str2.length() - last);
    return str2;
}

Example use

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

struct Animal {
    virtual void makeNoise() const = 0;
};

struct Dog : Animal {
    virtual void makeNoise() const override{
        std::cout << "Woof" << std::endl;
    }
};

struct Cat : Animal {
    virtual void makeNoise() const override{
        std::cout << "Meow" << std::endl;
    }
};

struct Duck : Animal {
    virtual void makeNoise() const override{
        std::cout << "Quack" << std::endl;
    }
};

struct NullAnimal : Animal {
    virtual void makeNoise() const override{
        std::cout << "?" << std::endl;
    }
};


using AnimalFactory = factory<Animal, Dog, Cat, Duck>;

int main() {
    auto animal = AnimalFactory::create("Dog");
    animal->makeNoise();
}

I have tested this with GCC 9.01 and it works

\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

Seems pretty reasonable. I mean, I definitely wouldn't put this in production code because it relies on parsing a class name out of __PRETTY_FUNCTION__, and that's not necessarily guaranteed to keep working in future versions of GCC let alone Clang (and __PRETTY_FUNCTION__ isn't even supported at all on MSVC; they have __FUNC_SIG instead).

In fact, I just tested on Godbolt, and your to_string<T>() function doesn't work at all on Clang. Furthermore, even on GCC, it has trouble with corner cases like A<';'>https://godbolt.org/z/UNVRQL


Nitpicks on your test code: The rule of thumb I follow is that every polymorphic method should have exactly one of virtual, override, or final (and really nothing should ever have final). So your repetition of virtual is just clutter, to me.

Ditto your use of std::endl (which flushes) versus plain old "\n" (which also flushes if you're outputting to a line-buffered stream such as std::cout). You could save some typing there.


You misspell "labeled" in at least two places: LabledClass and labled_classes. These are implementation details, but it's still important to spell things right so that you can grep for them later.

You pass const std::string_view& id by reference. This is unidiomatic. string_view is already a trivially copyable type, the size of two pointers. It doesn't make sense to force one of those onto the stack just so you can take its address and pass it by reference. Pass string_view by value.


    //checl all products inherit from interface_type
    std::apply([](auto&&... tuple_elem) {
        ((static_check<decltype(tuple_elem.data)>()), ...);
    }, labled_classes{} );

Typo: checl for check. And this is waaay more complicated than it needs to be. Just static_assert the thing you want to assert:

static_assert(std::is_base_of_v<interface_type, products> && ...);

In fact, let's use the idiomatic CamelCase for template arguments, and keep them short:

template<class Base, class... Ps>
[...]
    static_assert(std::is_base_of_v<Base, Ps> && ...);

std::apply([&result, &id](auto&&... tuple_elem) {
    (( tuple_elem.label == id ?
       result = std::make_unique<decltype(tuple_elem.data)>() :
       result ), ...);
}, labled_classes{} );

This complexity is a little more irreducible, but still, doing it all with a tuple and std::apply seems like way more template instantiations than you really ought to have here. What's wrong with a good old-fashioned chain of ifs?

Also, nit: if you're capturing everything by reference, just write [&]. It saves brain cells for the reader of your code.

int dummy[] = {
    ([&]() { if (id == to_string<Ps>()) result = std::make_unique<Ps>(); }(), 0) ...
};

We could even short-circuit as soon as we find the match. That's easy if we leave result null at first; then result will be null if and only if we should still be doing string comparisons.

static std::unique_ptr<Base>
create(std::string_view id, std::unique_ptr<Base> default_type = nullptr)
{
    static_assert(std::is_base_of_v<Base, Ps> && ...);

    std::unique_ptr<Base> result = nullptr;
    int dummy[] = {
        ([&]() {
            if (result == nullptr && id == to_string<Ps>()) {
                result = std::make_unique<Ps>();
            }
        }(), 0) ...
    };
    if (result == nullptr) {
        result = std::move(default_type);
    }
    return result;
}

At this point it's no longer clear why you need struct factory at all. So personally I'd get rid of it, and rename the now-free function create to makeUniqueByName or something.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ “really nothing should ever have finalis a bad rule (the link concerns final on classes but the same is true, to an even larger extent, for member functions). \$\endgroup\$ Apr 9, 2020 at 9:16
  • \$\begingroup\$ @Quuxplusone Thanks! This is really helpful advice. I'm still learning fold expressions so that is why there was the convoluted use of tuples and std::apply. Just a note, I have applied your changes but static_assert(std::is_base_of_v<Base, Ps> && ..., ""); does not compile. I instead need to use static_assert((std::is_base_of_v<Base, Ps> && ...), "");. \$\endgroup\$
    – Blue7
    Apr 9, 2020 at 9:49
  • \$\begingroup\$ @KonradRudolph: If "final" were the default and you had to type heritable to get a base class, then I'd say that "really nothing should ever have heritable." I feel the same way about const local variables (although I was on the other side circa 2005). C++ definitely has bad defaults, but still, there's no point going out of your way just to reiterate something that should be obvious by convention and/or to remove freedom from the maintainer. \$\endgroup\$ Apr 9, 2020 at 14:28
  • \$\begingroup\$ @Blue7 re parens around the fold-expression: Oops, yes, you're right, the parens are needed grammatically. \$\endgroup\$ Apr 9, 2020 at 14:29
  • \$\begingroup\$ @Quuxplusone The whole point of a type system is to remove freedoms from the maintainer. That’s a good thing. That said, I agree otherwise with your comment. In this spirit I rarely use final in C++ purely because I rarely use inheritance, and thus most (> 90%) of my classes are obviously not heritable. I only use it in cases where I work in an OOP hierarchy). \$\endgroup\$ Apr 9, 2020 at 17:03
1
\$\begingroup\$

I like explicit code even more verbose it pay back in readability, so no special reason for using extended form for a condition, in my opinion it makes the code more readable especially for beginners. btw I do the same also for other conditions such as == true, == nullprt and so on, you are free to use the simple condition and the result will be exactly the same one compiled, no extra code will be produced by any compiler :D.

Yes, indeed also for namespace I use the explicit form even more verbose, and in the above, implicit use of 'std' is only for the example, so after the 3 line of slashs "////.... "

Disclaimer: do not use namespace, all the time try to use explicit form such us std::string

\$\endgroup\$
0
0
\$\begingroup\$

@Quuxplusone thanks for your sharing, I loved your design and I did some changes to generalise it (and provided a working copy on godbolt so everyone can play with it ).

This review is intended to go over different limitations such as:

  • retrieving a name to the inner template class, originally named LabledClass and renamed in concrete_factory.
  • avoiding instantiating classes when not necessary, in fact a data member in LabeledClass was instantiating an object for each derived class.
  • simply dependencies check removing all static functions and replacing it with concepts.
  • allow to have different constructor for each object and to forward all arguments.
  • updating default, allowing the user to choice a default type to be instantiated or to return nullptr.

Enjoy :D

#include <iostream>     
#include <functional>   
#include <string>
#include <memory>
#include <type_traits>

template<std::size_t size>
struct tstring_t {
    /***/
    constexpr tstring_t(const char (&str)[size]) 
    { std::copy_n(str, size, _in_string); }

    /** Return string length. */
    constexpr td::size_t      length()  const 
    { return size-1; }
    /** Return a pointer to the null terminated string. */
    constexpr const char* c_str() const
    { return &_in_string[0]; }

    char _in_string[size];
};

template<tstring_t str>
struct plug_name
{ 
  constexpr static const bool             has_name = (str.length()>1)?true:false;
  constexpr static const std::string_view name     = str.c_str();
};

/** @brief Define concept for plug_name interface requirements. */
template <typename P>
concept plug_name_interface = requires( ) 
{
  { P::has_name };
  { P::name     };
};

/** @brief std::is_base_of extended for multiple types */
template<typename base_t, typename... others_t>
struct are_base_of
{ constexpr static const bool value = std::conjunction_v<std::is_base_of<base_t,others_t>... >; };

/** @brief Define concept checking that all @tparamother_t types are derived from @base_t. */
template<typename base_t, typename... others_t>
concept derived_types = requires
{
  requires are_base_of<base_t,others_t...>::value;
};

/***
 * Original design and for this class and all credits for that rely on @quuxplusone, get the link to 
 * the question on Code Review for that: https://codereview.stackexchange.com/a/240186 
 * The review for such implementation is intended to go over different limitations such as:
 *   - retrieving a name to the inner template class, originally named "LabledClass" and renamed in "concrete_factory"
 *   - avoiding instantiating classes when not necessary, in fact a data member in LabeledClass was instantiating an
 *     object for each derived class.
 *   - simply dependencies check removing all static functions and replacing it with concepts
 *   - allow to have different constructor for each object and to forward all arguments
 *   - updating default, allowing the user to choice a default type to be instantiated or to return nullptr
 */
template<typename base_t, typename default_t, typename... others_t>
  requires derived_types<base_t,others_t...> && 
           ( std::is_base_of_v<base_t,default_t> || std::is_same_v<default_t,std::nullptr_t> )
class abstract_factory 
{
public:
    template<typename derived_t>
    requires plug_name_interface<derived_t>
    struct concrete_factory {
      std::string_view name = derived_t::name;

      /** Create derived class instance forwarding arguments. */
      template<typename... args_t>
      std::unique_ptr<base_t> create( args_t&&... args )
      {  return std::make_unique<derived_t>( std::forward<args_t&&>(args)... ); }
    };

    using concrete_factories = std::tuple<concrete_factory<others_t>...>;

    template<typename... args_t>
    static std::unique_ptr<base_t> create(const std::string_view& id, args_t&&... args ) 
    {
        std::unique_ptr<base_t> result = nullptr;

        // if concrete_factory matches with the name, use the concrete factory to create the new instance.
        std::apply( [&result, &id, &args...](auto&&... tuple_item ) {
                        (( tuple_item.name == id ? result = tuple_item.create( std::forward<args_t>(args)... ) : result ), ...);
                    }, concrete_factories{}
                  );

        if ( result == nullptr )
        {
          if constexpr ( std::is_same_v<std::nullptr_t,default_t> == false )
          { result = std::make_unique<default_t>( std::forward<args_t&&>(args)... ); }
        }

        return result;
    }
};

/////////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////////

using namespace std;

class base
{
public:
    base()
    {  cout << "empty constructor base()" << endl; }

    base( int i, string s )
    {  cout << "empty constructor base( int, string ) i=" << i << " s=" << s << endl; }

    virtual const char* print() const noexcept
    {  return typeid(base).name(); }    
};

class derived_0 : public base, public plug_name<"derived 0">
{
public:
    derived_0()
    { cout << "empty constructor derived_0()" << endl; }
    
    derived_0( int i, string s )
    { cout << "empty constructor derived_0( int, string ) i=" << i << " s=" << s << endl; }

    virtual const char* print() const noexcept override
    {  return typeid(derived_0).name(); }
};

class derived_1 : public base, public plug_name<"derived 1">
{
public:
    derived_1()
    { cout << "empty constructor derived_1()" << endl; }
        
    derived_1( int i, string s )
    { cout << "empty constructor derived_1( int, string ) i=" << i << " s=" << s << endl; }


    virtual const char* print() const noexcept override
    {  return typeid(derived_1).name(); }    
};

using Factory = abstract_factory<base, std::nullptr_t, derived_0, derived_1 >;

int main () {

    auto db = Factory::create("undefined" );
    auto d0 = Factory::create("derived 0" );
    auto d1 = Factory::create("derived 1", 10, "hi" );

    cout << "output :" << endl; 
    cout << "  " << db->print() << endl;
    cout << "  " << d0->print() << endl;
    cout << "  " << d1->print() << endl;

    return 0;
}

\$\endgroup\$
1
  • \$\begingroup\$ Why write (str.length()>1)?true:false instead of simply str.length()>1? And please don't promote using namespace std; - consider that a beginner might happen across this and think that's acceptable. \$\endgroup\$ Aug 30, 2022 at 13:40

Your Answer

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

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