tidy up Blob and UTs

Author: 5e3

Rfx/Blob.h

@@ -19,9 +19,9 @@ namespace Rfx
 /// that avoids the usual value initialization (see std::make_unique_for_overwrite())
 class Blob
 {
-    std::unique_ptr<std::byte[]> m_storage;
     size_t m_size = 0;
     size_t m_capacity = 0;
+    std::unique_ptr<std::byte[]> m_storage;
 
 public:
     using iterator = std::byte*;
@@ -48,11 +48,10 @@ class Blob
     template <std::input_iterator It> Blob(It begin, It end) requires std::is_same_v<std::iter_value_t<It>, std::byte>;
     template <std::convertible_to<std::string_view> TString> Blob(TString&& str);
     Blob(const Blob& other);
-    Blob(Blob&& other) noexcept;
-    ~Blob() = default;
+    Blob(Blob&& other) noexcept = default;
 
     Blob& operator=(const Blob&);
-    Blob& operator=(Blob&&) noexcept;
+    Blob& operator=(Blob&&) noexcept = default;
 
     auto operator<=>(const Blob& rhs) const noexcept;
     bool operator==(const Blob& rhs) const noexcept;
@@ -71,17 +70,18 @@ class Blob
 ///////////////////////////////////////////////////////////////////////////////
 
 inline Blob::Blob(size_t size)
-    : Blob()
+    : m_size(size)
+    , m_capacity(std::max(std::bit_ceil(size), size_t(1024)))
+    , m_storage(std::make_unique_for_overwrite<std::byte[]>(m_capacity))
 {
-    grow(size);
 }
 
 template <std::input_iterator It>
 inline Blob::Blob(It begin, It end)
     requires std::is_same_v<std::iter_value_t<It>, std::byte>
 {
     for (; begin != end; ++begin)
-        *grow(1) = *begin;
+        push_back(*begin);
 }
 
 template <std::convertible_to<std::string_view> TString>
@@ -98,37 +98,20 @@ inline Blob::Blob(const Blob& other)
     std::copy(other.begin(), other.end(), begin());
 }
 
-inline Blob::Blob(Blob&& other) noexcept
-    : m_storage(std::move(other.m_storage))
-    , m_size(other.m_size)
-    , m_capacity(other.m_capacity)
+inline Blob& Blob::operator=(const Blob& other)
 {
-    other.m_size = 0;
-    other.m_capacity = 0;
-}
-
-inline Blob& Blob::operator= (const Blob& other)
-{
-    if (&other != this)
+    if (!m_storage || other.m_size > m_capacity)
     {
-        if (other.size() > m_size)
-            grow(other.size() - m_size);
-
-        std::copy(other.begin(), other.end(), begin());
+        Blob cpy { other };
+        return operator=(std::move(cpy));
     }
-    return *this;
-}
 
-inline Blob& Blob::operator=(Blob&& other) noexcept
-{
     if (&other != this)
     {
-        m_storage = std::move(other.m_storage);
+        std::copy(other.begin(), other.end(), begin());
         m_size = other.m_size;
-        m_capacity = other.m_capacity;
-        other.m_size = 0;
-        other.m_capacity = 0;
     }
+
     return *this;
 }
 
@@ -148,28 +131,25 @@ inline bool Blob::operator==(const Blob& rhs) const noexcept
 
 inline Blob::iterator Blob::grow(size_t size)
 {
-    auto newSize = size + m_size; 
-    auto start = m_size;
-    reserve(newSize);
-    m_size = newSize;
-    return m_storage.get() + start;
+    auto s = m_size;
+    resize(m_size + size);
+    return begin() + s;
 }
 
 inline void Blob::resize(size_t size)
 {
-    if (m_size < size)
-        reserve(size);
+    reserve(size);
     m_size = size;
 }
 
 inline void Blob::reserve(size_t capacity)
 {
     if (m_capacity < capacity)
     {
-        m_capacity = std::max(std::bit_ceil(capacity), size_t(1024));
-        auto storage = std::move(m_storage);
-        m_storage = std::make_unique_for_overwrite<std::byte[]>(m_capacity);
-        std::copy(storage.get(), storage.get() + m_size, begin());
+        Blob cpy(capacity);
+        cpy.m_size = m_size;
+        std::copy(begin(), end(), cpy.begin());
+        operator=(std::move(cpy));
     }
 }
 

TestRfx/TestBlob.cpp

@@ -31,6 +31,37 @@ TEST(Blob, initialize_list)
     ASSERT_EQ(ar[3], std::byte { 't' });
 }
 
+TEST(Blob, CopyCtor)
+{
+    Blob b1 = "dasisntest";
+    auto b2 = b1;
+    ASSERT_EQ(b1.size(), b2.size());
+    ASSERT_TRUE(std::equal(b1.begin(), b1.end(), b2.begin()));
+    ASSERT_NE(b1.begin(), b2.begin());
+}
+
+TEST(Blob, Assignment)
+{
+    Blob b1 = "dasisntest";
+    Blob b2;
+    b2 = b1;
+    ASSERT_EQ(b1.size(), b2.size());
+    ASSERT_TRUE(std::equal(b1.begin(), b1.end(), b2.begin()));
+    ASSERT_NE(b1.begin(), b2.begin());
+
+    Blob b3 = "test";
+    auto it = b3.begin();
+    b3 = b1;
+    ASSERT_EQ(b1.size(), b3.size());
+    ASSERT_TRUE(std::equal(b1.begin(), b1.end(), b3.begin()));
+    ASSERT_EQ(it, b3.begin());
+
+    Blob b4 = b1;
+    b4 = b4;
+    ASSERT_EQ(b1.size(), b4.size());
+    ASSERT_TRUE(std::equal(b1.begin(), b1.end(), b4.begin()));
+}
+
 TEST(Blob, comparison)
 {
     Blob b1 = "testlang";
@@ -51,7 +82,30 @@ TEST(Blob, comparison)
     ASSERT_NE(b4.capacity(), b2.capacity());
 }
 
-TEST(Blob, Ctors)
+TEST(Blob, moveCtorAssignment)
+{
+    Blob b1 = "dasisntest";
+    {
+        auto b2 = b1;
+        auto it = b2.begin();
+        auto b3 = std::move(b2);
+        ASSERT_EQ(b3, b1);
+        ASSERT_EQ(it, b3.begin());
+    } // call moved-from d'tor
+
+    auto b2 = b1;
+    auto b3 = std::move(b2);
+    auto it = b1.begin();
+    b2 = std::move(b1); // move assign to a moved-from object
+    ASSERT_EQ(b2, b3);
+    ASSERT_EQ(it, b2.begin());
+
+    b1 = b2; // assign to moved-from object
+    ASSERT_EQ(b1, b2);
+    ASSERT_NE(b1.begin(), b2.begin());
+}
+
+TEST(Blob, OtherCtorVariants)
 {
     std::vector<std::byte> bv = { std::byte { 1 }, std::byte { 2 }, std::byte { 3 } };
     Blob b1(bv.begin(), bv.end());
@@ -60,17 +114,16 @@ TEST(Blob, Ctors)
     std::ranges::reverse(b1);
     ASSERT_EQ(b1, b2);
 
-    auto it = b2.begin();
-    auto b3 = std::move(b2);
-    ASSERT_EQ(it, b3.begin());
-    ASSERT_TRUE(b2.empty());
-    ASSERT_TRUE(b2.capacity() == 0);
-
-    b2 = b1;
-    ASSERT_EQ(b2.size(), 3);
+    auto b3 = b2;
+    b2.push_back(std::byte { 4 });
+    ASSERT_NE(b2, b3);
+}
 
-    b2 = std::move(b3);
-    b3 = b1;
+TEST(Blob, clearResultsInZeroSize)
+{
+    Blob b = "DasIsnTest";
+    b.clear();
+    ASSERT_EQ(b.size(), 0);
 }
 
 TEST(Blob, capacityGrowsWhenNeeded)
@@ -96,10 +149,14 @@ TEST(Blob, clearDoesntChangeCapacity)
     ASSERT_EQ(oldCapacity, b.capacity());
 }
 
-TEST(Blob, reserveDoesntChangeContents)
+TEST(Blob, reserveDoesntChangeContentsButChangesBuffer)
 {
     Blob b = "test";
     Blob b2 = b;
+    auto it = b.begin();
     b.reserve(5000);
     ASSERT_EQ(b, b2);
+    ASSERT_NE(it, b.begin());
 }
+
+