Review fixes.

3 files changed, 27 insertions, 29 deletions

diff --git a/coding/coding_tests/compressed_bit_vector_test.cpp b/coding/coding_tests/compressed_bit_vector_test.cpp
index a09075bcfe..42515d5720 100644
--- a/coding/coding_tests/compressed_bit_vector_test.cpp
+++ b/coding/coding_tests/compressed_bit_vector_test.cpp
@@ -229,7 +229,7 @@ UNIT_TEST(CompressedBitVector_SerializationDense)
     cbv->Serialize(writer);
   }
   MemReader reader(buf.data(), buf.size());
-  auto cbv = coding::CompressedBitVectorBuilder::Deserialize(reader);
+  auto cbv = coding::CompressedBitVectorBuilder::DeserializeFromReader(reader);
   TEST(cbv.get(), ());
   TEST_EQUAL(coding::CompressedBitVector::StorageStrategy::Dense, cbv->GetStorageStrategy(), ());
   TEST_EQUAL(setBits.size(), cbv->PopCount(), ());
@@ -254,7 +254,7 @@ UNIT_TEST(CompressedBitVector_SerializationSparse)
     cbv->Serialize(writer);
   }
   MemReader reader(buf.data(), buf.size());
-  auto cbv = coding::CompressedBitVectorBuilder::Deserialize(reader);
+  auto cbv = coding::CompressedBitVectorBuilder::DeserializeFromReader(reader);
   TEST(cbv.get(), ());
   TEST_EQUAL(coding::CompressedBitVector::StorageStrategy::Sparse, cbv->GetStorageStrategy(), ());
   TEST_EQUAL(setBits.size(), cbv->PopCount(), ());
diff --git a/coding/compressed_bit_vector.cpp b/coding/compressed_bit_vector.cpp
index 8ae2157763..496fd63e64 100644
--- a/coding/compressed_bit_vector.cpp
+++ b/coding/compressed_bit_vector.cpp
@@ -231,6 +231,14 @@ void DenseCBV::Serialize(Writer & writer) const
   rw::WriteVectorOfPOD(writer, m_bitGroups);
 }
 
+unique_ptr<CompressedBitVector> DenseCBV::Clone() const
+{
+  DenseCBV * cbv = new DenseCBV();
+  cbv->m_popCount = m_popCount;
+  cbv->m_bitGroups = m_bitGroups;
+  return unique_ptr<CompressedBitVector>(cbv);
+}
+
 SparseCBV::SparseCBV(vector<uint64_t> const & setBits) : m_positions(setBits)
 {
   ASSERT(is_sorted(m_positions.begin(), m_positions.end()), ());
@@ -267,6 +275,13 @@ void SparseCBV::Serialize(Writer & writer) const
   rw::WriteVectorOfPOD(writer, m_positions);
 }
 
+unique_ptr<CompressedBitVector> SparseCBV::Clone() const
+{
+  SparseCBV * cbv = new SparseCBV();
+  cbv->m_positions = m_positions;
+  return unique_ptr<CompressedBitVector>(cbv);
+}
+
 // static
 unique_ptr<CompressedBitVector> CompressedBitVectorBuilder::FromBitPositions(
     vector<uint64_t> const & setBits)
@@ -290,7 +305,7 @@ unique_ptr<CompressedBitVector> CompressedBitVectorBuilder::FromBitGroups(
   while (!bitGroups.empty() && bitGroups.back() == 0)
     bitGroups.pop_back();
   if (bitGroups.empty())
-    return make_unique<SparseCBV>(bitGroups);
+    return make_unique<SparseCBV>(move(bitGroups));
 
   uint64_t const maxBit = kBlockSize * (bitGroups.size() - 1) + bits::CeilLog(bitGroups.back());
   uint64_t popCount = 0;
@@ -312,28 +327,6 @@ unique_ptr<CompressedBitVector> CompressedBitVectorBuilder::FromBitGroups(
   return make_unique<SparseCBV>(setBits);
 }
 
-// static
-unique_ptr<CompressedBitVector> CompressedBitVectorBuilder::FromCBV(CompressedBitVector const & cbv)
-{
-  auto strat = cbv.GetStorageStrategy();
-  switch (strat)
-  {
-  case CompressedBitVector::StorageStrategy::Dense:
-  {
-    DenseCBV const & dense = static_cast<DenseCBV const &>(cbv);
-    auto bitGroups = dense.m_bitGroups;
-    return CompressedBitVectorBuilder::FromBitGroups(move(bitGroups));
-  }
-  case CompressedBitVector::StorageStrategy::Sparse:
-  {
-    SparseCBV const & sparse = static_cast<SparseCBV const &>(cbv);
-    return CompressedBitVectorBuilder::FromBitPositions(sparse.m_positions);
-  }
-  }
-  CHECK(false, ("Unknown strategy when building a compressed bit vector."));
-  return unique_ptr<CompressedBitVector>();
-}
-
 string DebugPrint(CompressedBitVector::StorageStrategy strat)
 {
   switch (strat)
diff --git a/coding/compressed_bit_vector.hpp b/coding/compressed_bit_vector.hpp
index 18fd8f6982..837e35015f 100644
--- a/coding/compressed_bit_vector.hpp
+++ b/coding/compressed_bit_vector.hpp
@@ -63,6 +63,9 @@ public:
   // todo(@pimenov). Think about rewriting Serialize and Deserialize to use the
   // code in old_compressed_bit_vector.{c,h}pp.
   virtual void Serialize(Writer & writer) const = 0;
+
+  // Copies a bit vector and returns a pointer to the copy.
+  virtual unique_ptr<CompressedBitVector> Clone() const = 0;
 };
 
 string DebugPrint(CompressedBitVector::StorageStrategy strat);
@@ -105,6 +108,7 @@ public:
   bool GetBit(uint64_t pos) const override;
   StorageStrategy GetStorageStrategy() const override;
   void Serialize(Writer & writer) const override;
+  unique_ptr<CompressedBitVector> Clone() const override;
 
 private:
   vector<uint64_t> m_bitGroups;
@@ -117,6 +121,8 @@ public:
   friend class CompressedBitVectorBuilder;
   using TIterator = vector<uint64_t>::const_iterator;
 
+  SparseCBV() = default;
+
   explicit SparseCBV(vector<uint64_t> const & setBits);
 
   explicit SparseCBV(vector<uint64_t> && setBits);
@@ -136,6 +142,7 @@ public:
   bool GetBit(uint64_t pos) const override;
   StorageStrategy GetStorageStrategy() const override;
   void Serialize(Writer & writer) const override;
+  unique_ptr<CompressedBitVector> Clone() const override;
 
   inline TIterator Begin() const { return m_positions.cbegin(); }
   inline TIterator End() const { return m_positions.cend(); }
@@ -155,15 +162,13 @@ public:
 
   // Chooses a strategy to store the bit vector with bits from a bitmap obtained
   // by concatenating the elements of bitGroups.
+  static unique_ptr<CompressedBitVector> FromBitGroups(vector<uint64_t> & bitGroups);
   static unique_ptr<CompressedBitVector> FromBitGroups(vector<uint64_t> && bitGroups);
 
-  // Copies a CBV.
-  static unique_ptr<CompressedBitVector> FromCBV(CompressedBitVector const & cbv);
-
   // Reads a bit vector from reader which must contain a valid
   // bit vector representation (see CompressedBitVector::Serialize for the format).
   template <typename TReader>
-  static unique_ptr<CompressedBitVector> Deserialize(TReader & reader)
+  static unique_ptr<CompressedBitVector> DeserializeFromReader(TReader & reader)
   {
     ReaderSource<TReader> src(reader);
     return DeserializeFromSource(src);