patterncppMinor
Associative container that produces a unique, instance specific handle for each inserted object
Viewed 0 times
uniquecontainereachhandleinsertedinstancethatforproducesobject
Problem
It is not always possible to simplify program design by strictly managing the lifetimes of objects. If two objects have unpredictable lifetimes, but one of them needs to refer to the other, a simple pointer or reference is a segmentation fault waiting to happen.
One solution would be to allocate the object being tracked with
This led me to experiment with different approaches, ultimately resulting in a system implemented as a generic container:
One solution would be to allocate the object being tracked with
std::shared_ptr, and to distribute std::weak_ptr:s instead of plain pointers to any classes needing to refer to the object. This way the interested parties can simply check if the tracked object still exists by calling std::weak_ptr::expired() or by checking if the shared_ptr returned by std::weak_ptr::lock() is equal to nullptr. However this approach is not without drawbacks:- The objects being tracked are forced to be dynamically allocated with
std::shared_ptr, even when shared ownership is not really required.
- Calling
std::weak_ptr::lock()->/class/./member/everywhere is cumbersome and causes sizeable performance overhead.
- The objects being tracked are not stored contiguously in memory which hurts cache performance.
This led me to experiment with different approaches, ultimately resulting in a system implemented as a generic container:
- For each inserted object, the container returns a handle. The handle can be used as a key for accessing or removing the inserted object, or it can be tested for validity.
- The returned handle is permanently bound to the inserted object, and while removing the object results in an expired handle the handle can still be safely tested for validity, even after millions of other objects have been inserted and removed.
- The elements are stored contiguously in memory, and the container acts like a memory pool, only allocating more storage if no destroyed elements remain to be reused.
- Dereferring a handle involves just a pointer addition and optionally a comparison as a validity check.
- The container supports assignment. The assignment causes any handles to the container being
Solution
-
To "answer" this comment:
No, that library isn't required for basic uses of
-
These aren't too readable:
With such long lines, they should be spaced out:
-
This seems unnecessary:
The default copy constructor already does this, so just let the compiler provide it.
-
You have a data member (
-
This member function in
No assignments or such are done, so it only displays a statement. Either have something like this added or remove this function.
-
These statements lack curly braces and could be bad for maintenance:
It should become this:
The same should be applied elsewhere as well.
To "answer" this comment:
#include //required for placement new?No, that library isn't required for basic uses of
new (it's a keyword). However, a feature such as std::nothrow would require it. Check the documentation for more information.-
These aren't too readable:
void hire(Container::Handle employee){employees.insert(employee);}
void fire(Container::Handle employee){employees.erase(employee);}With such long lines, they should be spaced out:
void hire(Container::Handle employee)
{
employees.insert(employee);
}
void fire(Container::Handle employee)
{
employees.erase(employee);
}-
This seems unnecessary:
Container(const Container &other)
{
*this = other;
}The default copy constructor already does this, so just let the compiler provide it.
-
You have a data member (
std::string name) in Person. As data members shouldn't be part of the interface, Person should become a class so that name can be private.-
This member function in
Person doesn't really do anything useful:void receivePayment(int amount)
{
std::cout << name << " was paid $" << amount << "\n";
}No assignments or such are done, so it only displays a statement. Either have something like this added or remove this function.
-
These statements lack curly braces and could be bad for maintenance:
if(temp.empty())
new(static_cast(&dataByIndex_[n]))
T(other[Handle{n, 0}]);
else
{
if(n == temp.back())
temp.pop_back();
else
new(static_cast(&dataByIndex_[n]))
T(other[Handle{n, 0}]);
}It should become this:
if (temp.empty())
{
new(static_cast(&dataByIndex_[n]))
T(other[Handle{n, 0}]);
}
else
{
if (n == temp.back())
{
temp.pop_back();
}
else
{
new(static_cast(&dataByIndex_[n]))
T(other[Handle{n, 0}]);
}
}The same should be applied elsewhere as well.
Code Snippets
#include <memory> //required for placement new?void hire(Container<Person>::Handle employee){employees.insert(employee);}
void fire(Container<Person>::Handle employee){employees.erase(employee);}void hire(Container<Person>::Handle employee)
{
employees.insert(employee);
}
void fire(Container<Person>::Handle employee)
{
employees.erase(employee);
}Container(const Container &other)
{
*this = other;
}void receivePayment(int amount)
{
std::cout << name << " was paid $" << amount << "\n";
}Context
StackExchange Code Review Q#49888, answer score: 8
Revisions (0)
No revisions yet.