Flyweight::get() takes emplace params and returns shared_ptr
Instead of creating an object, passing it by reference, and then copying
that object again, the Flyweight::get() function was changed to take the
parameters for the template object's constructor so that the object is
created emplace. This avoids the extra copy.
Also, using shared_ptr to make things a little more safe with memory
management.
Change-Id: Ib23010d5ee9d19b0be257eafc5297c8a158eebb0
Signed-off-by: Zane Shelley <zshelle@us.ibm.com>
diff --git a/src/util/hei_flyweight.hpp b/src/util/hei_flyweight.hpp
index 437dcad..6228759 100644
--- a/src/util/hei_flyweight.hpp
+++ b/src/util/hei_flyweight.hpp
@@ -3,6 +3,7 @@
#include <stdint.h>
#include <algorithm>
+#include <memory>
#include <vector>
namespace libhei
@@ -17,10 +18,7 @@
Flyweight() = default;
/** @brief Destructor. */
- ~Flyweight()
- {
- clear();
- }
+ ~Flyweight() = default;
/** @brief Default copy constructor. */
Flyweight(const Flyweight&) = delete;
@@ -37,30 +35,42 @@
}
/**
- * @brief Adds the given entry to the factory, if it does not already
- * exist. Then returns a reference to that entry in the factory.
- * @param The target entry.
- * @return A reference to this entry in the factory.
+ * @brief Does an emplace add of a new entry to the factory, if the entry
+ * does not already exist, and returns a pointer to the entry in the
+ * factory.
+ * @param An variable argument list that would be passed to a contructor of
+ * class T.
+ * @return A pointer to this entry in the factory.
*/
- T& get(const T& i_entry)
+ template <class... Args>
+ std::shared_ptr<T> get(Args&&... i_args)
{
+ // Create a new instance with the given arguments.
+ std::shared_ptr<T> newEntry = std::make_shared<T>(i_args...);
+
// The index is sorted by the value of the T objects. Check to see if
- // this entry already exists in the factory.
- auto itr =
- std::lower_bound(iv_index.begin(), iv_index.end(), &i_entry,
- [](const T* a, const T* b) { return *a < *b; });
+ // newEntry already exists in the factory.
+ auto itr = std::lower_bound(
+ iv_index.begin(), iv_index.end(), newEntry,
+ [](const std::shared_ptr<T> a, const std::shared_ptr<T> b) {
+ return *a < *b;
+ });
// std::lower_bound() will return the first element that does not
- // compare less than i_entry. So if an entry is found, we must make sure
- // it does not have the same value as i_entry.
- if (iv_index.end() == itr || !(i_entry == *(*itr)))
+ // compare less than newEntry. So if an element is found, we must make
+ // sure it does not have the same value as newEntry.
+ if (iv_index.end() == itr || !(*newEntry == *(*itr)))
{
- // Create a new entry and store the pointer in the sorted index.
- itr = iv_index.insert(itr, new T{i_entry});
+ // Store the new enty in the sorted index.
+ itr = iv_index.insert(itr, newEntry);
}
- // Return a reference to this entry in the factory.
- return *(*itr);
+ // It is important to note that if newEntry was not inserted into the
+ // index above because a duplicate already existed, it will be deleted
+ // as soon as it goes out of scope.
+
+ // Return a pointer to the this entry in the factory.
+ return *itr;
}
/**
@@ -70,10 +80,6 @@
*/
void clear()
{
- for (auto i : iv_index)
- {
- delete i;
- }
iv_index.clear();
}
@@ -92,6 +98,12 @@
iv_index.shrink_to_fit();
}
+ /** @return The number of entries in this flyweight factory. */
+ size_t size()
+ {
+ return iv_index.size();
+ }
+
private:
/**
* Each new T is allocated on the memory heap and a pointer to each of those
@@ -109,7 +121,7 @@
* the structure. Also, the Hostboot user application does not support
* std::set at this time.
*/
- std::vector<T*> iv_index;
+ std::vector<std::shared_ptr<T>> iv_index;
};
} // end namespace libhei
diff --git a/test/flyweight_test.cpp b/test/flyweight_test.cpp
index 139d7c1..fb982e1 100644
--- a/test/flyweight_test.cpp
+++ b/test/flyweight_test.cpp
@@ -7,14 +7,8 @@
class Foo
{
public:
- Foo() = default;
explicit Foo(int i) : iv_i(i) {}
- int get() const
- {
- return iv_i;
- }
-
bool operator==(const Foo& i_r) const
{
return iv_i == i_r.iv_i;
@@ -29,31 +23,16 @@
int iv_i = 0;
};
-Foo& addFoo(int i)
-{
- return Flyweight<Foo>::getSingleton().get(Foo{i});
-}
-
TEST(FlyweightTest, TestSet1)
{
- // Add some unique entries in a random order and keep track of where those
- // enties exist in memory.
- Foo* a[5];
- a[1] = &(addFoo(1));
- a[2] = &(addFoo(2));
- a[0] = &(addFoo(0));
- a[4] = &(addFoo(4));
- a[3] = &(addFoo(3));
+ auto& foo_factory = Flyweight<Foo>::getSingleton();
- // Now add more entries and verify the 'new' entries match the same
- // addresses as the previously added entries.
- for (int i = 4; i >= 0; i--)
- {
- ASSERT_EQ(a[i], &(addFoo(i)));
- }
+ auto f1 = foo_factory.get(1);
+ auto f2 = foo_factory.get(2);
+ auto f3 = foo_factory.get(1); // same as f1
- // At this point, we have proven that duplicate entries will return
- // references to the original unique entries. There is probably more we can
- // do here, but this is enough to prove the Flyweight class follows the
- // flyweight design pattern.
+ ASSERT_NE(f1, f2); // Pointing to different objects
+ ASSERT_EQ(f1, f3); // Pointing to the same object
+
+ ASSERT_EQ(2, foo_factory.size()); // Only two entries in the flyweight
}