Review fixes.

7 files changed, 57 insertions, 58 deletions

diff --git a/generator/feature_generator.hpp b/generator/feature_generator.hpp
index 7a17cc0b2b..4122d5d134 100644
--- a/generator/feature_generator.hpp
+++ b/generator/feature_generator.hpp
@@ -4,7 +4,7 @@
 
 #include "coding/file_writer.hpp"
 
-#include "std/numeric.hpp"
+#include "std/limits.hpp"
 #include "std/string.hpp"
 #include "std/vector.hpp"
 
@@ -21,6 +21,7 @@ class FeaturesCollector
 
 protected:
   static uint32_t constexpr kInvalidFeatureId = numeric_limits<uint32_t>::max();
+
   FileWriter m_datFile;
   m2::RectD m_bounds;
 
diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp
index 562b10b2d9..00e71766d5 100644
--- a/generator/generator_tests/restriction_collector_test.cpp
+++ b/generator/generator_tests/restriction_collector_test.cpp
@@ -67,8 +67,8 @@ UNIT_TEST(RestrictionTest_InvalidCase)
   TEST_EQUAL(restrictionCollector.m_restrictions, expectedRestrictions, ());
 
   restrictionCollector.RemoveInvalidRestrictions();
-  TEST(restrictionCollector.m_restrictions.empty(), ());
-  TEST(!restrictionCollector.IsValid(), ());
+  TEST(!restrictionCollector.HasRestrictions(), ());
+  TEST(restrictionCollector.IsValid(), ());
 }
 
 UNIT_TEST(RestrictionTest_ParseRestrictions)
diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp
index 774c08a787..b454bc4745 100644
--- a/generator/restriction_collector.cpp
+++ b/generator/restriction_collector.cpp
@@ -12,16 +12,16 @@ namespace
 {
 char const kNo[] = "No";
 char const kOnly[] = "Only";
+char const kDelim[] = ", \t\r\n";
 
 bool ParseLineOfNumbers(strings::SimpleTokenizer & iter, vector<uint64_t> & numbers)
 {
   uint64_t number = 0;
-  while (iter)
+  for (; iter; ++iter)
   {
     if (!strings::to_uint64(*iter, number))
       return false;
     numbers.push_back(number);
-    ++iter;
   }
   return true;
 }
@@ -42,13 +42,12 @@ RestrictionCollector::RestrictionCollector(string const & restrictionPath,
   }
   ComposeRestrictions();
   RemoveInvalidRestrictions();
-  LOG(LINFO, ("m_restrictions.size() =", m_restrictions.size()));
+  LOG(LINFO, ("Number of restrictions: =", m_restrictions.size()));
 }
 
 bool RestrictionCollector::IsValid() const
 {
-  return !m_restrictions.empty() &&
-         find_if(begin(m_restrictions), end(m_restrictions),
+  return find_if(begin(m_restrictions), end(m_restrictions),
                  [](Restriction const & r) { return !r.IsValid(); }) == end(m_restrictions);
 }
 
@@ -58,14 +57,11 @@ bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureId
   if (featureId2OsmIdsStream.fail())
     return false;
 
-  while (featureId2OsmIdsStream)
+  string line;
+  while (getline(featureId2OsmIdsStream, line))
   {
-    string line;
-    if (!getline(featureId2OsmIdsStream, line))
-      return true;
-
     vector<uint64_t> osmIds;
-    strings::SimpleTokenizer iter(line, ",");
+    strings::SimpleTokenizer iter(line, kDelim);
     if (!ParseLineOfNumbers(iter, osmIds))
       return false;
 
@@ -79,19 +75,19 @@ bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureId
   return true;
 }
 
-bool RestrictionCollector::ParseRestrictions(string const & restrictionPath)
+bool RestrictionCollector::ParseRestrictions(string const & path)
 {
-  ifstream restrictionsStream(restrictionPath);
-  if (restrictionsStream.fail())
+  ifstream stream(path);
+  if (stream.fail())
     return false;
 
-  while (restrictionsStream)
+  string line;
+  while (getline(stream, line))
   {
-    string line;
-    if (!getline(restrictionsStream, line))
-      return true;
+    strings::SimpleTokenizer iter(line, kDelim);
+    if (!iter)  // the line is empty
+      return false;
 
-    strings::SimpleTokenizer iter(line, ",");
     Restriction::Type type;
     if (!FromString(*iter, type))
       return false;
@@ -115,7 +111,7 @@ void RestrictionCollector::ComposeRestrictions()
     LinkIndex const & index = osmIdAndIndex.second;
     CHECK_LESS(index.m_restrictionNumber, numRestrictions, ());
     Restriction & restriction = m_restrictions[index.m_restrictionNumber];
-    CHECK_LESS(index.m_linkNumber, restriction.m_links.size(), ());
+    CHECK_LESS(index.m_linkNumber, restriction.m_featureIds.size(), ());
 
     uint64_t const & osmId = osmIdAndIndex.first;
     // Checking if there's an osm id belongs to a restriction is saved only once as feature id.
@@ -132,7 +128,7 @@ void RestrictionCollector::ComposeRestrictions()
 
     uint32_t const & featureId = rangeId.first->second;
     // Adding feature id to restriction coresponded to the osm id.
-    restriction.m_links[index.m_linkNumber] = featureId;
+    restriction.m_featureIds[index.m_linkNumber] = featureId;
   }
 
   my::SortUnique(m_restrictions);
@@ -149,10 +145,10 @@ void RestrictionCollector::RemoveInvalidRestrictions()
 
 void RestrictionCollector::AddRestriction(Restriction::Type type, vector<uint64_t> const & osmIds)
 {
-  size_t const restrictionCount = m_restrictions.size();
+  size_t const numRestrictions = m_restrictions.size();
   m_restrictions.emplace_back(type, osmIds.size());
   for (size_t i = 0; i < osmIds.size(); ++i)
-    m_restrictionIndex.emplace_back(osmIds[i], LinkIndex({restrictionCount, i}));
+    m_restrictionIndex.emplace_back(osmIds[i], LinkIndex({numRestrictions, i}));
 }
 
 void RestrictionCollector::AddFeatureId(uint32_t featureId, vector<uint64_t> const & osmIds)
@@ -175,7 +171,6 @@ string ToString(Restriction::Type const & type)
 
 bool FromString(string str, Restriction::Type & type)
 {
-  str.erase(remove_if(str.begin(), str.end(), isspace), str.end());
   if (str == kNo)
   {
     type = Restriction::Type::No;
@@ -203,7 +198,7 @@ string DebugPrint(RestrictionCollector::LinkIndex const & index)
 string DebugPrint(Restriction const & restriction)
 {
   ostringstream out;
-  out << "m_links:[" << ::DebugPrint(restriction.m_links)
+  out << "m_featureIds:[" << ::DebugPrint(restriction.m_featureIds)
       << "] m_type:" << DebugPrint(restriction.m_type) << " ";
   return out.str();
 }
diff --git a/generator/restriction_collector.hpp b/generator/restriction_collector.hpp
index 1d22c85e2d..a69c80830c 100644
--- a/generator/restriction_collector.hpp
+++ b/generator/restriction_collector.hpp
@@ -15,11 +15,6 @@ namespace routing
 /// their road feature in text file for using later.
 class RestrictionCollector
 {
-  friend void UnitTest_RestrictionTest_ValidCase();
-  friend void UnitTest_RestrictionTest_InvalidCase();
-  friend void UnitTest_RestrictionTest_ParseRestrictions();
-  friend void UnitTest_RestrictionTest_ParseFeatureId2OsmIdsMapping();
-
 public:
   /// \brief Addresses a link in vector<Restriction>.
   struct LinkIndex
@@ -29,30 +24,37 @@ public:
     {
     }
 
-    // Restriction number in restriction vector.
-    size_t m_restrictionNumber = 0;
-    // Link number for a restriction. It's equal to zero or one for most cases.
-    size_t m_linkNumber = 0;
-
     bool operator==(LinkIndex const & index) const
     {
       return m_restrictionNumber == index.m_restrictionNumber && m_linkNumber == index.m_linkNumber;
     }
+
+    // Restriction number in restriction vector.
+    size_t const m_restrictionNumber;
+    // Link number for a restriction. It's equal to zero or one for most cases.
+    size_t const m_linkNumber;
   };
 
   /// \param restrictionPath full path to file with road restrictions in osm id terms.
   /// \param featureId2OsmIdsPath full path to file with mapping from feature id to osm id.
   RestrictionCollector(string const & restrictionPath, string const & featureId2OsmIdsPath);
 
-  /// \returns true if |m_restrictions| is not empty all feature ids in |m_restrictions|
-  /// are set to valid value and false otherwise.
+  bool HasRestrictions() const { return !m_restrictions.empty(); }
+
+  /// \returns true if all restrictions in |m_restrictions| are valid and false otherwise.
   /// \note Empty |m_restrictions| is considered as an invalid restriction.
   /// \note Complexity of the method is up to linear in the size of |m_restrictions|.
   bool IsValid() const;
 
   /// \returns Sorted vector of restrictions.
   RestrictionVec const & GetRestrictions() const { return m_restrictions; }
+
 private:
+  friend void UnitTest_RestrictionTest_ValidCase();
+  friend void UnitTest_RestrictionTest_InvalidCase();
+  friend void UnitTest_RestrictionTest_ParseRestrictions();
+  friend void UnitTest_RestrictionTest_ParseFeatureId2OsmIdsMapping();
+
   /// \brief Parses comma separated text file with line in following format:
   /// <feature id>, <osm id 1 corresponding feature id>, <osm id 2 corresponding feature id>, and so
   /// on
@@ -72,7 +74,7 @@ private:
   /// No, 157616940, 157616940,
   /// No, 157616940, 157617107,
   /// \param featureId2OsmIdsPath path to the text file.
-  bool ParseRestrictions(string const & restrictionPath);
+  bool ParseRestrictions(string const & path);
 
   /// \brief Sets feature id for all restrictions in |m_restrictions|.
   void ComposeRestrictions();
diff --git a/generator/restriction_dumper.cpp b/generator/restriction_dumper.cpp
index dd10593a90..5635ea50a7 100644
--- a/generator/restriction_dumper.cpp
+++ b/generator/restriction_dumper.cpp
@@ -23,24 +23,25 @@ vector<string> const kRestrictionTypesOnly = {"only_right_turn", "only_left_turn
                                               "only_straight_on"};
 
 /// \brief Converts restriction type form string to RestrictionCollector::Type.
-/// \returns Fisrt item is a result of conversion. Second item is true
-/// if convertion was successful and false otherwise.
-pair<Restriction::Type, bool> TagToType(string const & type)
+/// \returns Fisrt true if convertion was successful and false otherwise.
+bool TagToType(string const & tag, Restriction::Type & type)
 {
-  if (find(kRestrictionTypesNo.cbegin(), kRestrictionTypesNo.cend(), type) !=
+  if (find(kRestrictionTypesNo.cbegin(), kRestrictionTypesNo.cend(), tag) !=
       kRestrictionTypesNo.cend())
   {
-    return make_pair(Restriction::Type::No, true);
+    type = Restriction::Type::No;
+    return true;
   }
 
-  if (find(kRestrictionTypesOnly.cbegin(), kRestrictionTypesOnly.cend(), type) !=
+  if (find(kRestrictionTypesOnly.cbegin(), kRestrictionTypesOnly.cend(), tag) !=
       kRestrictionTypesOnly.cend())
   {
-    return make_pair(Restriction::Type::Only, true);
+    type = Restriction::Type::Only;
+    return true;
   }
 
   // Unsupported restriction type.
-  return make_pair(Restriction::Type::No, false);
+  return false;
 }
 }  // namespace
 
@@ -92,12 +93,11 @@ void RestrictionDumper::Write(RelationElement const & relationElement)
   if (tagIt == relationElement.tags.end())
     return;  // Type of the element is different from "restriction".
 
-  auto const typeResult = TagToType(tagIt->second);
-  if (typeResult.second == false)
+  Restriction::Type type = Restriction::Type::No;
+  if (!TagToType(tagIt->second, type))
     return;  // Unsupported restriction type.
 
   // Adding restriction.
-  m_stream << ToString(typeResult.first) << ","  // Restriction type
-           << fromIt->first << ", " << toIt->first << "," << endl;
+  m_stream << ToString(type) << "," << fromIt->first << ", " << toIt->first << endl;
 }
 }  // namespace routing
diff --git a/generator/restriction_dumper.hpp b/generator/restriction_dumper.hpp
index e7735d9b1a..2ba7dbd750 100644
--- a/generator/restriction_dumper.hpp
+++ b/generator/restriction_dumper.hpp
@@ -9,8 +9,6 @@ namespace routing
 {
 class RestrictionDumper
 {
-  ofstream m_stream;
-
 public:
   void Open(string const & fullPath);
   bool IsOpened();
@@ -21,5 +19,8 @@ public:
   /// are ignored.
   // @TODO(bykoianko) It's necessary to process all kind of restrictions.
   void Write(RelationElement const & relationElement);
+
+private:
+    ofstream m_stream;
 };
 }  // namespace routing
diff --git a/generator/restriction_generator.cpp b/generator/restriction_generator.cpp
index bfb9979c65..b9e98db141 100644
--- a/generator/restriction_generator.cpp
+++ b/generator/restriction_generator.cpp
@@ -28,8 +28,8 @@ void SerializeRestrictions(RestrictionVec::const_iterator begin, RestrictionVec:
 
   Restriction::Type const type = begin->m_type;
 
-  Restriction prevRestriction(type, 0);
-  prevRestriction.m_links.resize(feature::RestrictionSerializer::kSupportedLinkNumber, 0);
+  Restriction prevRestriction(type, 0 /* linkNumber */);
+  prevRestriction.m_featureIds.resize(feature::RestrictionSerializer::kSupportedLinkNumber, 0);
   for (auto it = begin; it != end; ++it)
   {
     CHECK_EQUAL(type, it->m_type, ());
@@ -48,7 +48,7 @@ bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPat
   LOG(LINFO,
       ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", featureId2OsmIdsPath, ");"));
   RestrictionCollector restrictionCollector(restrictionPath, featureId2OsmIdsPath);
-  if (!restrictionCollector.IsValid())
+  if (!restrictionCollector.HasRestrictions() || !restrictionCollector.IsValid())
   {
     LOG(LWARNING, ("No valid restrictions for", mwmPath, "It's necessary to check if",
                    restrictionPath, "and", featureId2OsmIdsPath, "are available."));