Simple factory pattern

Hello,

how can pass a value to the constructor of the derived structs in this simple factory pattern? thx

Code:

---

#include <iostream>
#include <memory>
#include <vector>

struct B {
        virtual ~B() = default;
};

struct D1 : B {
        int Val;
        D1(int val) : Val(val) {}
};

struct D2 : B {
        int Val;
        D2(int val) : Val(val) {}
};

template<typename T> std::unique_ptr<B> factory(int val) {
        return std::make_unique<T>(val);
}

int main() {
        std::vector<std::shared_ptr<B>> vec{ factory<D1>(1), factory<D2>(2) };
        for(const auto& v : vec ) std::cout << v->Val << '\n';
}

---

Code:

---

#include <iostream>
#include <memory>
#include <vector>
 
struct B {
    int Val;
    B(int val = 0) : Val(val) { }
    virtual ~B() = default;
};
 
struct D1 : B {
    D1(int val) : B(val) { }
};
 
struct D2 : B {
    D2(int val) : B(val) { }
};
 
template<typename T>
std::unique_ptr<B> makeB(int val) {
    return std::make_unique<T>(T(val));
}
 
int main() {
    for (const auto& b : { makeB<D1>(1), makeB<D2>(2) })
        std::cout << b->Val << '\n';
}

---

Quote:

---

Originally Posted by serge

how can pass a value to the constructor of the derived structs in this simple factory pattern? thx

---

You did so successfully.

What you didn't do successfully was trying to access a member specific to derived classes on the elements of a vector of base class smart pointers:

Code:

---

for(const auto& v : vec ) std::cout << v->Val << '\n';

---

Since vec is a vector of B shared pointers, v is a const reference to a B shared pointer, so you cannot write v->Val because B doesn't have a Val member, not even a private one.

This should have been more obvious to you if you paid attention to error messages: next time, force yourself to read and post the error message, not just post a question and code without any indication as to why you think it doesn't work.

If you look at john.c's post #2, you can see that one simple solution to this is to move the declaration of Val to the base class, if that fits with your overall design.

Quote:

---

Originally Posted by john.c

Code:

---

return std::make_unique<T>(T(val));

---

---

That explicit construction of a T temporary is not necessary: make_unique has perfect forwarding, so serge's version of the make_unique call is better.

Thanks to both of you. I realized that I don't need y base class at all, so the problem became simpler. Yes, checking error messages does help in most cases.

I have a follow up issue that I cannot solve even after checking a few examples on the net. How do I declare the two entities in a header and a cpp file so that function can reach the destructor? This is to avoid the std::unique_ptr with an incomplete type won't compile error. The solution apparently involves moving the constructors and destructors to the cpp file. But this I do not understand, since both are contained in this file already.

Code:

---

struct Engine
{
        const int Id;
        const int Mounts;
        constexpr Engine(int id, int mounts) : Id(id), Mounts(mounts) {}
        ~Engine() = default;
        Engine(const Engine&) = delete;
        Engine(Engine&&) = delete;
};

auto ForgeEngine(int id, int mounts)
{
        return std::make_unique<Engine>(id, mounts);
}

---

Quote:

---

Originally Posted by serge

This is to avoid the std::unique_ptr with an incomplete type won't compile error.

---

Post the corresponding error message, don't just refer to it.

This compiles for me without even a warning on g++ 7.4.0:

Code:

---

#include <iostream>
#include <memory>
 
struct Engine
{
    const int Id;
    const int Mounts;
    constexpr Engine(int id, int mounts) : Id(id), Mounts(mounts) {}
    ~Engine() = default;
    Engine(const Engine&) = delete;
    Engine(Engine&&) = delete;
};
 
auto ForgeEngine(int id, int mounts)
{
    return std::make_unique<Engine>(id, mounts);
}

int main()
{
    auto engine = ForgeEngine(1, 2);
    std::cout << engine->Id << ", " << engine->Mounts << std::endl;
}

---

I note that if you want to disable the copy and move constructors, then you probably should disable the copy and move assignment operators too.

Quote:

---

Originally Posted by laserlight

I note that if you want to disable the copy and move constructors, then you probably should disable the copy and move assignment operators too.

---

You mean like this?

Code:

---

Engine& operator=(const Engine&) = delete;
Engine& operator=(Engine&&) = delete;

---

The problem I had is if I put this in engine.h

Code:

---

#pragma once
#include<memory>

struct Engine;

std::unique_ptr<Engine> ForgeEngine(int id, int mounts);

---

and this in engine.cpp

Code:

---

#include "engine.h"

struct Engine
{
        const int Id;
        const int Mounts;
        constexpr Engine(int id, int mounts) : Id(id), Mounts(mounts) {}
        ~Engine() = default;
        Engine(const Engine&) = delete;
        Engine(Engine&&) = delete;
};

std::unique_ptr<Engine> ForgeEngine(int id, int mounts)
{
        return std::make_unique<Engine>(id, mounts);
}

---

I get these errors:

Error C2338 can't delete an incomplete type

Warning C4150 deletion of pointer to incomplete type 'Engine'; no destructor called

follows by
Error C2027 use of undefined type 'Engine'

Then don't do that :)

The class definition should go in the header unless you're trying for the pimpl (opaque pointer) idiom... but in that case in C++ you would still have a class wrapper; it's just the implementation that's hidden in the source file.

You could even put ForgeEngine's implementation in the header by declaring it as inline.