From edfbda47a76429c169933673adaa8653857a21ca Mon Sep 17 00:00:00 2001
From: Maxim Pimenov <m@maps.me>
Date: Fri, 25 Sep 2015 20:46:42 +0300
Subject: [PATCH] Review fixes.

---
 coding/coding_tests/succinct_trie_test.cpp | 17 +++---
 coding/trie.hpp                            | 16 +++---
 coding/trie_reader.hpp                     | 64 +++++++++++-----------
 3 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/coding/coding_tests/succinct_trie_test.cpp b/coding/coding_tests/succinct_trie_test.cpp
index 7326a69040..7ae8cb6db9 100644
--- a/coding/coding_tests/succinct_trie_test.cpp
+++ b/coding/coding_tests/succinct_trie_test.cpp
@@ -100,19 +100,18 @@ struct SimpleValueList
   vector<uint8_t> m_valueList;
 };
 
-void ReadAllValues(
-    unique_ptr<trie::SuccinctTrieIterator<MemReader, SimpleValueReader>> const & root,
+void ReadAllValues(trie::SuccinctTrieIterator<MemReader, SimpleValueReader> & root,
     vector<uint8_t> & values)
 {
-  for (size_t i = 0; i < root->NumValues(); ++i)
-    values.push_back(root->GetValue(i));
+  for (size_t i = 0; i < root.NumValues(); ++i)
+    values.push_back(root.GetValue(i));
 }
 
 void CollectInSubtree(
     unique_ptr<trie::SuccinctTrieIterator<MemReader, SimpleValueReader>> const & root,
     vector<uint8_t> & collectedValues)
 {
-  ReadAllValues(root, collectedValues);
+  ReadAllValues(*root.get(), collectedValues);
 
   if (auto l = root->GoToEdge(0))
     CollectInSubtree(l, collectedValues);
@@ -223,7 +222,7 @@ UNIT_TEST(SuccinctTrie_MoveToString)
     TEST(it != nullptr, ());
     vector<uint8_t> expectedValues;
     vector<uint8_t> receivedValues;
-    ReadAllValues(it, receivedValues);
+    ReadAllValues(*it.get(), receivedValues);
     TEST_EQUAL(expectedValues, receivedValues, ());
   }
 
@@ -232,7 +231,7 @@ UNIT_TEST(SuccinctTrie_MoveToString)
     TEST(it != nullptr, ());
     vector<uint8_t> expectedValues{1};
     vector<uint8_t> receivedValues;
-    ReadAllValues(it, receivedValues);
+    ReadAllValues(*it.get(), receivedValues);
     TEST_EQUAL(expectedValues, receivedValues, ());
   }
 
@@ -241,7 +240,7 @@ UNIT_TEST(SuccinctTrie_MoveToString)
     TEST(it != nullptr, ());
     vector<uint8_t> expectedValues{2};
     vector<uint8_t> receivedValues;
-    ReadAllValues(it, receivedValues);
+    ReadAllValues(*it.get(), receivedValues);
     TEST_EQUAL(expectedValues, receivedValues, ());
   }
 
@@ -250,7 +249,7 @@ UNIT_TEST(SuccinctTrie_MoveToString)
     TEST(it != nullptr, ());
     vector<uint8_t> expectedValues{3, 4};
     vector<uint8_t> receivedValues;
-    ReadAllValues(it, receivedValues);
+    ReadAllValues(*it.get(), receivedValues);
     TEST_EQUAL(expectedValues, receivedValues, ());
   }
 
diff --git a/coding/trie.hpp b/coding/trie.hpp
index f2c189316d..2d98e937c2 100644
--- a/coding/trie.hpp
+++ b/coding/trie.hpp
@@ -17,7 +17,7 @@ typedef uint32_t TrieChar;
 // However 0 is used because the first byte is actually language id.
 static uint32_t const DEFAULT_CHAR = 0;
 
-template <typename ValueT>
+template <typename TValue>
 class Iterator
 {
   //dbg::ObjectTracker m_tracker;
@@ -30,12 +30,12 @@ public:
   };
 
   buffer_vector<Edge, 8> m_edge;
-  buffer_vector<ValueT, 2> m_value;
+  buffer_vector<TValue, 2> m_value;
 
   virtual ~Iterator() {}
 
-  virtual Iterator<ValueT> * Clone() const = 0;
-  virtual Iterator<ValueT> * GoToEdge(size_t i) const = 0;
+  virtual Iterator<TValue> * Clone() const = 0;
+  virtual Iterator<TValue> * GoToEdge(size_t i) const = 0;
 };
 
 struct EmptyValueReader
@@ -66,16 +66,16 @@ struct FixedSizeValueReader
   }
 };
 
-template <typename ValueT, typename F, typename StringT>
-void ForEachRef(Iterator<ValueT> const & iter, F & f, StringT const & s)
+template <typename TValue, typename F, typename TString>
+void ForEachRef(Iterator<TValue> const & iter, F & f, TString const & s)
 {
   for (size_t i = 0; i < iter.m_value.size(); ++i)
     f(s, iter.m_value[i]);
   for (size_t i = 0; i < iter.m_edge.size(); ++i)
   {
-    StringT s1(s);
+    TString s1(s);
     s1.insert(s1.end(), iter.m_edge[i].m_str.begin(), iter.m_edge[i].m_str.end());
-    unique_ptr<Iterator<ValueT>> const pIter1(iter.GoToEdge(i));
+    unique_ptr<Iterator<TValue>> const pIter1(iter.GoToEdge(i));
     ForEachRef(*pIter1, f, s1);
   }
 }
diff --git a/coding/trie_reader.hpp b/coding/trie_reader.hpp
index 204daa0a1f..777aad0bfa 100644
--- a/coding/trie_reader.hpp
+++ b/coding/trie_reader.hpp
@@ -9,17 +9,17 @@
 
 namespace trie
 {
-template <class ValueReaderT>
-class LeafIterator0 : public Iterator<typename ValueReaderT::ValueType>
+template <class TValueReader>
+class LeafIterator0 : public Iterator<typename TValueReader::ValueType>
 {
 public:
-  typedef typename ValueReaderT::ValueType ValueType;
+  typedef typename TValueReader::ValueType ValueType;
 
-  template <class ReaderT>
-  LeafIterator0(ReaderT const & reader, ValueReaderT const & valueReader)
+  template <class TReader>
+  LeafIterator0(TReader const & reader, TValueReader const & valueReader)
   {
     uint32_t const size = static_cast<uint32_t>(reader.Size());
-    ReaderSource<ReaderT> src(reader);
+    ReaderSource<TReader> src(reader);
     while (src.Pos() < size)
     {
       this->m_value.push_back(ValueType());
@@ -32,9 +32,10 @@ public:
     ASSERT_EQUAL(size, src.Pos(), ());
   }
 
-  Iterator<ValueType> * Clone() const { return new LeafIterator0<ValueReaderT>(*this); }
+  // trie::Iterator overrides:
+  Iterator<ValueType> * Clone() const override { return new LeafIterator0<TValueReader>(*this); }
 
-  Iterator<ValueType> * GoToEdge(size_t i) const
+  Iterator<ValueType> * GoToEdge(size_t i) const override
   {
     ASSERT(false, (i));
     UNUSED_VALUE(i);
@@ -42,36 +43,37 @@ public:
   }
 };
 
-template <class ReaderT, class ValueReaderT>
-class IteratorImplBase : public Iterator<typename ValueReaderT::ValueType>
+template <class TReader, class TValueReader>
+class IteratorImplBase : public Iterator<typename TValueReader::ValueType>
 {
 protected:
   enum { IS_READER_IN_MEMORY = 0 };
 };
 
-template <class ValueReaderT>
-class IteratorImplBase<SharedMemReader, ValueReaderT>
-    : public Iterator<typename ValueReaderT::ValueType>
+template <class TValueReader>
+class IteratorImplBase<SharedMemReader, TValueReader>
+    : public Iterator<typename TValueReader::ValueType>
 {
 protected:
   enum { IS_READER_IN_MEMORY = 1 };
 };
 
-template <class ReaderT, class ValueReaderT>
-class Iterator0 : public IteratorImplBase<ReaderT, ValueReaderT>
+template <class TReader, class TValueReader>
+class Iterator0 : public IteratorImplBase<TReader, TValueReader>
 {
 public:
-  typedef typename ValueReaderT::ValueType ValueType;
+  typedef typename TValueReader::ValueType ValueType;
 
-  Iterator0(ReaderT const & reader, ValueReaderT const & valueReader, TrieChar baseChar)
+  Iterator0(TReader const & reader, TValueReader const & valueReader, TrieChar baseChar)
     : m_reader(reader), m_valueReader(valueReader)
   {
     ParseNode(baseChar);
   }
 
-  Iterator<ValueType> * Clone() const { return new Iterator0<ReaderT, ValueReaderT>(*this); }
+  // trie::Iterator overrides:
+  Iterator<ValueType> * Clone() const override { return new Iterator0<TReader, TValueReader>(*this); }
 
-  Iterator<ValueType> * GoToEdge(size_t i) const
+  Iterator<ValueType> * GoToEdge(size_t i) const override
   {
     ASSERT_LESS(i, this->m_edge.size(), ());
     uint32_t const offset = m_edgeInfo[i].m_offset;
@@ -79,16 +81,16 @@ public:
 
     // TODO: Profile to check that MemReader optimization helps?
     /*
-    if (!IteratorImplBase<ReaderT, ValueReaderT>::IS_READER_IN_MEMORY &&
+    if (!IteratorImplBase<TReader, TValueReader>::IS_READER_IN_MEMORY &&
         size < 1024)
     {
       SharedMemReader memReader(size);
       m_reader.Read(offset, memReader.Data(), size);
       if (m_edgeInfo[i].m_isLeaf)
-        return new LeafIterator0<SharedMemReader, ValueReaderT>(
+        return new LeafIterator0<SharedMemReader, TValueReader>(
               memReader, m_valueReader);
       else
-        return new Iterator0<SharedMemReader, ValueReaderT>(
+        return new Iterator0<SharedMemReader, TValueReader>(
               memReader, m_valueReader,
               this->m_edge[i].m_str.back());
     }
@@ -96,9 +98,9 @@ public:
     */
     {
       if (m_edgeInfo[i].m_isLeaf)
-        return new LeafIterator0<ValueReaderT>(m_reader.SubReader(offset, size), m_valueReader);
+        return new LeafIterator0<TValueReader>(m_reader.SubReader(offset, size), m_valueReader);
       else
-        return new Iterator0<ReaderT, ValueReaderT>(m_reader.SubReader(offset, size), m_valueReader,
+        return new Iterator0<TReader, TValueReader>(m_reader.SubReader(offset, size), m_valueReader,
                                                     this->m_edge[i].m_str.back());
     }
   }
@@ -106,7 +108,7 @@ public:
 private:
   void ParseNode(TrieChar baseChar)
   {
-    ReaderSource<ReaderT> src(m_reader);
+    ReaderSource<TReader> src(m_reader);
 
     // [1: header]: [2: min(valueCount, 3)] [6: min(childCount, 63)]
     uint8_t const header = ReadPrimitiveFromSource<uint8_t>(src);
@@ -175,16 +177,16 @@ private:
 
   buffer_vector<EdgeInfo, 9> m_edgeInfo;
 
-  ReaderT m_reader;
-  ValueReaderT m_valueReader;
+  TReader m_reader;
+  TValueReader m_valueReader;
 };
 
 // Returns iterator to the root of the trie.
-template <class ReaderT, class ValueReaderT>
-Iterator<typename ValueReaderT::ValueType> * ReadTrie(ReaderT const & reader,
-                                                      ValueReaderT valueReader = ValueReaderT())
+template <class TReader, class TValueReader>
+Iterator<typename TValueReader::ValueType> * ReadTrie(TReader const & reader,
+                                                      TValueReader valueReader = TValueReader())
 {
-  return new Iterator0<ReaderT, ValueReaderT>(reader, valueReader, DEFAULT_CHAR);
+  return new Iterator0<TReader, TValueReader>(reader, valueReader, DEFAULT_CHAR);
 }
 
 }  // namespace trie