Using std::unique_ptr in object cache

I am trying to convert some legacy code to C++17(ish).

I have some objects which have multiple versions – each version has the same members, but different sizes.
I have a factory class which caches objects as they are created and continues to return the same object on future requests.

This all worked great with C pointers (albeit not ‘safe’). Using the unique_ptr template is giving me grief due to the Ownership.

I want the factory to have full ownership and control of the pointers and the caller to just access the object it is given (originally that was through an Object*)

I have tried making the factory function return a unique_ptr and also a reference. Each has its own issues.

Getting the unique_ptr returned fails for several reasons and my understanding is the caller then posses ownership so the cache no longer can get to the object.

I made the factory function work with a ref return, but the caller has issues getting the ref into a variable (see code).

Here is the code on Compiler Explorer: https://godbolt.org/z/M9Gr9j

I am sorry the code is long – I did not know how to make it shorter without loosing contexts. The real code is a lot more involved.

Comments are in places of trouble.

The common parts:

#include <map>
#include <string>
#include <memory>
#include <iostream>


using ll = long long;

class Object    // Base class for an object that supports different sizes of members
{
public:
    Object() = default;
    virtual ~Object() = default;

    virtual ll GetId() = 0;
};

class ObjectV1 : public Object              // For this usage, this object will create ids 1 to 32767
{
public:
    ObjectV1() : id(++counter) {  }
    ~ObjectV1() override = default;

    ll GetId() override { return id; };
    /* v1 members . . . */
private:
    short id;   // 2 byte int
    static inline short counter{0};
};

class ObjectV2 : public Object              // For this usage, this object will create ids 200000 up
{
public:
    ObjectV2() : id(200000 + ++counter) {}
    ~ObjectV2() override = default;

    ll GetId() override { return id; }
private:
    ll id;  // 8 byte int
    static inline ll counter{0};
    /* v2 members . . . */
};

Original code using C pointers:


// Factory of Objects based on a key lookup
// Once the object for a given key is created, just return the cashed item
// In real life, the key determines which version of record to get - I translated to an int to simplify
class ObjectFactory_p
{
public:
    ObjectFactory_p() = default;
    ~ObjectFactory_p() noexcept { EmptyObjectCache(); };

    Object* GetObject(const std::string& key, int ver)
    {
        Object* pO{nullptr};
        auto it = objectCache.find(key);
        if (it == objectCache.end())
        {
            if (ver == 1)
                pO = new ObjectV1();
            else
                pO = new ObjectV2();

            if (pO)
            {
                //pO->Load();
                objectCache.emplace(key, pO);
            }
        }
        else pO = it->second;

        return pO;
    }

    void RemoveObjectFromCache(const std::string& key)
    {
        auto it = objectCache.find(key);
        if (it != objectCache.end())
        {
            delete it->second;
            objectCache.erase(it);
        }
    }

    inline void EmptyObjectCache()
    {
        for (const auto& [key, pO] : objectCache)
            delete pO;
        objectCache.clear();
    }

private:
    using ObjectMap = std::map<const std::string, Object*>;

    ObjectMap objectCache;
};  // ObjectFactory_p



void testing_p()
{
    ObjectFactory_p objectFactory;

    auto ov1 = objectFactory.GetObject("a-key-value-1", 1); // cacheSize = 1
    ll i1 = ov1->GetId();  std::cout << i1 << std::endl;    // = 1

    auto ov2 = objectFactory.GetObject("a-key-value-2", 2); // cacheSize = 2
    ll i2 = ov2->GetId();  std::cout << i2 << std::endl;    // = 200001

    auto ov3 = objectFactory.GetObject("a-key-value-1", 1); // cacheSize = 2
    ll i3 = ov3->GetId();  std::cout << i3 << std::endl;    // = 1

    auto ov4 = objectFactory.GetObject("a-key-value-3", 2); // cacheSize = 3
    ll i4 = ov4->GetId();  std::cout << i4 << std::endl;    // = 200002
}

Factory reworked with unique_ptr:

class ObjectFactory
{
public:
    ObjectFactory() = default;
    ~ObjectFactory() noexcept { EmptyObjectCache(); };

    std::unique_ptr<Object> GetObjectPtr(const std::string& key, int ver)
    {
        std::unique_ptr<Object> upO;
        auto it = objectCache.find(key);
        if (it == objectCache.end())
        {
            if (ver == 1)
                upO = std::make_unique<ObjectV1>();
            else
                upO = std::make_unique<ObjectV2>();
            
            if (upO)
            {
                //upO->Load();
                objectCache.emplace(key, std::move(upO));
                // upO is now 'gone' so it can't be returned
            }
        }
        //else upO = it->second; // error C2280: 'std::unique_ptr<Object> &std::unique_ptr<Object>::operator =(const std::unique_ptr<Object> &)': attempting to reference a deleted function

        return upO;
    }

    Object& GetObjectRef(const std::string& key, int ver)
    {
        std::unique_ptr<Object> upO;
        auto it = objectCache.find(key);
        if (it == objectCache.end())
        {
            if (ver == 1)
            {
                auto cit = objectCache.emplace(key, std::make_unique<ObjectV1>());
                //cit.first->second->Load();
                return *cit.first->second;
            }
            else
            {
                auto cit = objectCache.emplace(key, std::make_unique<ObjectV2>());
                //cit.first->second->Load();
                return *cit.first->second;
            }
        }
        else return *it->second;
    }

    void RemoveObjectFromCache(const std::string& key)
    {
        auto it = objectCache.find(key);
        if (it != objectCache.end())
        {
            it->second.reset();
            objectCache.erase(it);
        }
    }

    inline void EmptyObjectCache()
    {
        for (auto& [key, upO] : objectCache)
            upO.reset();
        objectCache.clear();
    }

private:
    using ObjectMap = std::map<const std::string, std::unique_ptr<Object>>;

    ObjectMap objectCache;
};  // ObjectFactory

Calling the objects:

// Same code as above but using std::unique_ptr instead of *
// It won't run because it gets back an empty object
void testing_SUP()
{
    ObjectFactory objectFactory;

    auto ov1 = objectFactory.GetObjectPtr("a-key-value-1", 1);  // shouldbe: // cacheSize = 1
    ll i1 = ov1->GetId();  std::cout << i1 << std::endl;        // shouldbe: // = 1

    auto ov2 = objectFactory.GetObjectPtr("a-key-value-2", 2);  // shouldbe: // cacheSize = 2
    ll i2 = ov2->GetId();  std::cout << i2 << std::endl;        // shouldbe: // = 200001

    auto ov3 = objectFactory.GetObjectPtr("a-key-value-1", 1);  // shouldbe: // cacheSize = 2
    ll i3 = ov3->GetId();  std::cout << i3 << std::endl;        // shouldbe: // = 1

    auto ov4 = objectFactory.GetObjectPtr("a-key-value-3", 2);  // shouldbe: // cacheSize = 3
    ll i4 = ov4->GetId();  std::cout << i4 << std::endl;        // shouldbe: // = 200002
}






// This one is using the return-by-ref but it then must revert to bad habits to make it into a C pointer again.
ll closer_to_the_real_code(const std::string& key)
{
    ObjectFactory objectFactory;

    Object* o;          // Since I need to try multiple times, this cannot be a ref - compiler wants it Init here

    o = &objectFactory.GetObjectRef(key, 2);            // Try to access v2 data
    if (!o)
        o = &objectFactory.GetObjectRef(key, 1);        // If fails, fall back to v1 data
    if (o)
        return o->GetId();
    return -1;
}





int main()
{
    testing_p();

    //testing_SUP();


    ll id = closer_to_the_real_code("a-key-value");
    std::cout << std::endl << id << std::endl;       // 200003

    return 1;
}

Source: Windows Questions C++

LEAVE A COMMENT