Addressing review comments

Author: MaheshGPai

tdigest/include/tdigest.hpp

@@ -106,6 +106,12 @@ class tdigest {
    */
   explicit tdigest(uint16_t k = DEFAULT_K, const Allocator& allocator = Allocator());
 
+  /**
+ * Copy constructor
+ * @param other sketch to be copied
+ */
+  tdigest(const tdigest& other);
+
   /**
    * Update this t-Digest with the given value
    * @param value to update the t-Digest with
@@ -275,13 +281,11 @@ class tdigest {
 private:
   bool reverse_merge_;
   uint16_t k_;
-  uint16_t internal_k_;
   T min_;
   T max_;
   size_t centroids_capacity_;
   vector_centroid centroids_;
   uint64_t centroids_weight_;
-  size_t buffer_capacity_;
   vector_t buffer_;
 
   static const size_t BUFFER_MULTIPLIER = 4;

tdigest/include/tdigest_impl.hpp

@@ -597,6 +597,24 @@ bool tdigest<T, A>::is_single_value() const {
   return get_total_weight() == 1;
 }
 
+template<typename T, typename A>
+tdigest<T, A>::tdigest(const tdigest<T, A>& other):
+  reverse_merge_(other.reverse_merge_),
+  k_(other.k_),
+  min_(other.min_),
+  max_(other.max_),
+  centroids_capacity_(other.centroids_capacity_),
+  centroids_(other.centroids_, other.get_allocator()),
+  centroids_weight_(other.centroids_weight_),
+  buffer_(other.buffer_, other.get_allocator())
+{
+  if (other.k_ < 10) throw std::invalid_argument("k must be at least 10");
+  const size_t fudge = other.k_ < 30 ? 30 : 10;
+  centroids_capacity_ = 2 * k_ + fudge;
+  centroids_.reserve(centroids_capacity_);
+  buffer_.reserve(centroids_capacity_ * BUFFER_MULTIPLIER);
+}
+
 template<typename T, typename A>
 tdigest<T, A>::tdigest(bool reverse_merge, uint16_t k, T min, T max, vector_centroid&& centroids, uint64_t weight, vector_t&& buffer):
 reverse_merge_(reverse_merge),
@@ -638,11 +656,11 @@ template <typename T, typename A>
 }
 
 template<typename T, typename A>
-tdigest<T, A>::const_iterator::const_iterator(const tdigest& tdigest_, const bool is_end):
-  centroids_()
+tdigest<T, A>::const_iterator::const_iterator(const tdigest<T, A>& tdigest_, const bool is_end):
+  centroids_(tdigest_.get_allocator())
 {
   // Create a copy of the tdigest to generate the centroids after processing the buffered values
-  tdigest tmp(tdigest_);
+  tdigest<T, A> tmp(tdigest_);
   tmp.compress();
   centroids_.insert(centroids_.end(), tmp.centroids_.begin(), tmp.centroids_.end());
 

tdigest/test/tdigest_test.cpp

@@ -460,11 +460,14 @@ TEST_CASE("iterate centroids", "[tdigest]") {
   }
 
   auto centroid_count = 0;
+  uint64_t total_weight = 0;
   for (const auto &centroid: td) {
     centroid_count++;
+    total_weight += centroid.second;
   }
   // Ensure that centroids are retrieved for a case where there is buffered values
   REQUIRE(centroid_count == 10);
+  REQUIRE(td.get_total_weight() == total_weight);
 }
 
 } /* namespace datasketches */