Do I have to return a pointer from a factory?

Question

Can anyone see any problems with returning an object by value from a factory rather than returning a unique_ptr?

The following compiles and runs correctly for me, but i'm unsure if i've missed something about l-value and r-value reference interactions and lifetimes.

I've the following class hierarchy,

#include <iostream>

struct Reader {
    virtual ~Reader() {}
    virtual void get(int& val) = 0;
};

struct CinReader : public Reader {
    ~CinReader() { std::cout << "~CinReader()\n"; }

    void get(int& val) override
    {
        std::cout << "CinReader::get\n";
        std::cin >> val;
    }
};

struct ConstantReader : public Reader {
    void get(int& val) override
    {
        std::cout << "ConstantReader::get\n";
        val = 120;
    }
};

I am using the following factory to return instances,

enum class ReaderType { cin = 0, constant = 1 };

Reader&& readerFactoryObj(ReaderType type)
{
    switch (type) {
    case ReaderType::cin:
        return std::move(CinReader());
        break;
    case ReaderType::constant:
        return std::move(ConstantReader());
        break;
    default:
        throw "Unknown Reader type";
    }
}

and used as follows,

int main(void)
{
    auto&& reader = readerFactoryObj(ReaderType::cin);
    int val;
    reader.get(val);
}

It works when passed to something that stores a reference to the Reader interface,

class Doubler
{
public:
    Doubler(Reader& reader) : mReader(reader) { }

    void doubleNum() const;

private:
    Reader& mReader;
};

void Doubler::doubleNum() const
{
    int val;
    mReader.get(val);
    std::cout << val * 2 << '\n';
}

int main(void)
{
    auto&& reader = readerFactoryObj(ReaderType::cin);
    Doubler d(reader);
    d.doubleNum();
}

I realise reader isn't of type Reader now, but will be one of its concrete sub-classes.

Are there any problems with passing reader as Reader& and storing it?

Update: I added a destructor to CinReader and it clearly shows me that its lifetime ends before it's used.

Answer 1

You're returning references to temporary objects. Undefined behavior. The compiler would likely warn you if you didn't trick it with a completely redundant move call.