Design pattern for factory of shared objects

Question

I have a system where a singleton object is responsible for creating and sharing objects between other non-singleton classes. For example:

// One instance is shared between multiple instances
// of Widget that have the same ID.
class Lease {};

// For each unique Widget instance that has the same
// value for `m_id`, the Controller will either
// create a new Lease (if one has not already been
// created) or it will return an existing one.
class Widget
{
private:
    unsigned m_id{};
    std::shared_ptr<Lease> m_lease;
};

// Singleton object that is used by all instances of
// Widget to obtain shared instances to Lease
class Controller
{
public:
    std::shared_ptr<Lease> CreateLease(unsigned id);

private:
    std::map<unsigned, std::weak_ptr<Lease>> m_leases;
};

This is the gist of what I have at the moment. What Controller does is create a new Lease if one does not already exist in the map for the given ID. If it does exist, it returns the existing shared object.

I have a timer that regularly checks for "expired" weak pointers in the m_leases map and will remove them if so. This means that at some point, multiple Widgets were destroyed that subsequently released their Leases as well.

Now that you have some background, what I think I have here is a factory (Controller) that is a little bit smarter than a normal factory: It keeps track of instances is creates and only creates new ones based on certain business rules (specifically, if a matching ID is found in the map). I'm not sure if this is the best design for what I'm trying to do (and that is: Some mechanism to share Lease instances across unique instances of Widget). A few things I don't like about this solution:

1. A singleton is required as the point of contact for Widget instances to acquire a Lease
2. A timer is used to manage the map: There's no event-based approach to managing removing expired leases from the map that I could think of.
3. Follow on to #2: Because I'm using a timer to manage lease expiry in the map, there's always a small window where a lease remains in the map even after it expires, which means CreateLease() also has to check if a weak_ptr is expired, if an existing ID mapping is found, prior to returning it.

This logic just feels wrong to me. I need extra sets of eyes on this idea and ideally recommendations for better patterns to solve this problem.

Answer 1

I would have the lease broadcast in the destructor - the observer pattern.

This would allow the Controller to register an observer for the deletion of the lease and remove it. This removes your need for the timer, and introduces the concept of an event; which satisfies 2 & 3.

As for the controller being a singleton, from what you've posted, it could be nested into the Widget and made private (or moved into the cpp file of Widget). While this still will end up being a singleton, it's one that's much more controlled and can be easily replaced later because of the limited scope of it.

Example of how it could be thrown together:

class Lease {
public:
    struct listener {
        virtual void leaseGone(int id) = 0;
    }

    void addListener(listener* l) {
        listeners.push_back(l);
    }

    ~Lease() {
        for (auto x&: listeners)
            x->leaseGone(myId);
    }
 }


class Controller : Lease::listener {
    void leaseGone(int id) {
        m_leases.erase(id);
    }
}