Review fixes

5 files changed, 81 insertions, 83 deletions

diff --git a/generator/collector_camera.cpp b/generator/collector_camera.cpp
index 6c0c1c68e1..f8c636703a 100644
--- a/generator/collector_camera.cpp
+++ b/generator/collector_camera.cpp
@@ -56,7 +56,7 @@ void CameraProcessor::ForEachCamera(Fn && toDo) const
 
 void CameraProcessor::ProcessWay(OsmElement const & element)
 {
-  for (auto const node : element.m_nds)
+  for (auto const node : element.m_nodes)
   {
     if (m_speedCameras.find(node) == m_speedCameras.cend())
       continue;
diff --git a/generator/osm_element.cpp b/generator/osm_element.cpp
index 7dd7c62e85..1a06178b5b 100644
--- a/generator/osm_element.cpp
+++ b/generator/osm_element.cpp
@@ -7,9 +7,9 @@
 #include <cstdio>
 #include <sstream>
 
-std::string DebugPrint(OsmElement::EntityType e)
+std::string DebugPrint(OsmElement::EntityType type)
 {
-  switch (e)
+  switch (type)
   {
   case OsmElement::EntityType::Unknown:
     return "unknown";
@@ -31,14 +31,13 @@ std::string DebugPrint(OsmElement::EntityType e)
   UNREACHABLE();
 }
 
-
-void OsmElement::AddTag(std::string_view const & k, std::string_view const & v)
+void OsmElement::AddTag(std::string_view const & key, std::string_view const & value)
 {
   // Seems like source osm data has empty values. They are useless for us.
-  if (k.empty() || v.empty())
+  if (key.empty() || value.empty())
     return;
 
-#define SKIP_KEY(key) if (strncmp(k.data(), key, sizeof(key)-1) == 0) return;
+#define SKIP_KEY(skippedKey) if (strncmp(key.data(), skippedKey, sizeof(key)-1) == 0) return;
   // OSM technical info tags
   SKIP_KEY("created_by");
   SKIP_KEY("source");
@@ -52,7 +51,6 @@ void OsmElement::AddTag(std::string_view const & k, std::string_view const & v)
   SKIP_KEY("artist_name");
   SKIP_KEY("whitewater"); // https://wiki.openstreetmap.org/wiki/Whitewater_sports
 
-
   // In future we can use this tags for improve our search
   SKIP_KEY("old_name");
   SKIP_KEY("alt_name");
@@ -65,22 +63,22 @@ void OsmElement::AddTag(std::string_view const & k, std::string_view const & v)
   SKIP_KEY("official_name");
 #undef SKIP_KEY
 
-  std::string value{std::string{v}};
-  strings::Trim(value);
-  m_tags.emplace_back(std::string{k}, std::move(value));
+  std::string val{std::string{value}};
+  strings::Trim(val);
+  m_tags.emplace_back(std::string{key}, std::move(val));
 }
 
 bool OsmElement::HasTag(std::string_view const & key) const
 {
   return std::any_of(m_tags.begin(), m_tags.end(), [&](auto const & t) {
-    return t.key == key;
+    return t.m_key == key;
   });
 }
 
-bool OsmElement::HasTag(std::string_view const & k, std::string_view const & v) const
+bool OsmElement::HasTag(std::string_view const & key, std::string_view const & value) const
 {
   return std::any_of(m_tags.begin(), m_tags.end(), [&](auto const & t) {
-    return t.m_key == k && t.m_value == v;
+    return t.m_key == key && t.m_value == value;
   });
 }
 
@@ -112,12 +110,12 @@ std::string OsmElement::ToString(std::string const & shift) const
     ss << "Nd ref: " << m_ref;
     break;
   case EntityType::Way:
-    ss << "Way: " << m_id << " nds: " << m_nds.size() << " tags: " << m_tags.size();
-    if (!m_nds.empty())
+    ss << "Way: " << m_id << " nds: " << m_nodes.size() << " tags: " << m_tags.size();
+    if (!m_nodes.empty())
     {
       std::string shift2 = shift;
       shift2 += shift2.empty() ? "\n  " : "  ";
-      for (auto const & e : m_nds)
+      for (auto const & e : m_nodes)
         ss << shift2 << e;
     }
     break;
@@ -165,14 +163,14 @@ std::string_view OsmElement::GetTagValue(std::string_view const & key,
                                          std::string_view const & defaultValue) const
 {
   auto const it = std::find_if(m_tags.cbegin(), m_tags.cend(),
-                               [&key](Tag const & tag) { return tag.key == key; });
+                               [&key](Tag const & tag) { return tag.m_key == key; });
 
-  return it != m_tags.cend() ? it->value : defaultValue;
+  return it != m_tags.cend() ? it->m_value : defaultValue;
 }
 
-std::string DebugPrint(OsmElement const & e)
+std::string DebugPrint(OsmElement const & element)
 {
-  return e.ToString();
+  return element.ToString();
 }
 
 std::string DebugPrint(OsmElement::Tag const & tag)
diff --git a/generator/osm_element.hpp b/generator/osm_element.hpp
index 6b9f14a383..cdc5fd7a66 100644
--- a/generator/osm_element.hpp
+++ b/generator/osm_element.hpp
@@ -35,9 +35,9 @@ struct OsmElement
     Member(uint64_t ref, EntityType type, std::string const & role)
       : m_ref(ref), m_type(type), m_role(role) {}
 
-    bool operator==(Member const & e) const
+    bool operator==(Member const & other) const
     {
-      return m_ref == e.m_ref && m_type == e.m_type && m_role == e.m_role;
+      return m_ref == other.m_ref && m_type == other.m_type && m_role == other.m_role;
     }
 
     uint64_t m_ref = 0;
@@ -48,31 +48,31 @@ struct OsmElement
   struct Tag
   {
     Tag() = default;
-    Tag(std::string const & k, std::string const & v) : m_key(k), m_value(v) {}
+    Tag(std::string const & key, std::string const & value) : m_key(key), m_value(value) {}
 
-    bool operator==(Tag const & e) const
+    bool operator==(Tag const & other) const
     {
-      return m_key == e.m_key && m_value == e.m_value;
+      return m_key == other.m_key && m_value == other.m_value;
     }
 
-    bool operator<(Tag const & e) const
+    bool operator<(Tag const & other) const
     {
-      return m_key == e.m_key ?  m_value < e.m_value : m_key < e.m_key;
+      return m_key == other.m_key ? m_value < other.m_value : m_key < other.m_key;
     }
 
     std::string m_key;
     std::string m_value;
   };
 
-  static EntityType StringToEntityType(std::string const & t)
+  static EntityType StringToEntityType(std::string const & type)
   {
-    if (t == "way")
+    if (type == "way")
       return EntityType::Way;
-    if (t == "node")
+    if (type == "node")
       return EntityType::Node;
-    if (t == "relation")
+    if (type == "relation")
       return EntityType::Relation;
-    ASSERT(false, ("Unknown type", t));
+    ASSERT(false, ("Unknown type", type));
     return EntityType::Unknown;
   }
 
@@ -80,22 +80,22 @@ struct OsmElement
   {
     m_type = EntityType::Unknown;
     m_id = 0;
-    m_lon = 0;
-    m_lat = 0;
+    m_lon = 0.0;
+    m_lat = 0.0;
     m_ref = 0;
     m_k.clear();
     m_v.clear();
     m_memberType = EntityType::Unknown;
     m_role.clear();
 
-    m_nds.clear();
+    m_nodes.clear();
     m_members.clear();
     m_tags.clear();
   }
 
   std::string ToString(std::string const & shift = std::string()) const;
 
-  std::vector<uint64_t> const & Nodes() const { return m_nds; }
+  std::vector<uint64_t> const & Nodes() const { return m_nodes; }
   std::vector<Member> const & Members() const { return m_members; }
   std::vector<Tag> const & Tags() const { return m_tags; }
 
@@ -103,49 +103,49 @@ struct OsmElement
   bool IsWay() const { return m_type == EntityType::Way; }
   bool IsRelation() const { return m_type == EntityType::Relation; }
 
-  bool operator==(OsmElement const & e) const
+  bool operator==(OsmElement const & other) const
   {
-    return m_type == e.m_type
-        && m_id == e.m_id
-        && base::AlmostEqualAbs(m_lon, e.m_lon, 1e-7)
-        && base::AlmostEqualAbs(m_lat, e.m_lat, 1e-7)
-        && m_ref == e.m_ref
-        && m_k == e.m_k
-        && m_v == e.m_v
-        && m_memberType == e.m_memberType
-        && m_role == e.m_role
-        && m_nds == e.m_nds
-        && m_members == e.m_members
-        && m_tags == e.m_tags;
+    return m_type == other.m_type
+        && m_id == other.m_id
+        && base::AlmostEqualAbs(m_lon, other.m_lon, 1e-7)
+        && base::AlmostEqualAbs(m_lat, other.m_lat, 1e-7)
+        && m_ref == other.m_ref
+        && m_k == other.m_k
+        && m_v == other.m_v
+        && m_memberType == other.m_memberType
+        && m_role == other.m_role
+        && m_nodes == other.m_nodes
+        && m_members == other.m_members
+        && m_tags == other.m_tags;
   }
 
-  void AddNd(uint64_t ref) { m_nds.emplace_back(ref); }
+  void AddNd(uint64_t ref) { m_nodes.emplace_back(ref); }
   void AddMember(uint64_t ref, EntityType type, std::string const & role)
   {
     m_members.emplace_back(ref, type, role);
   }
 
-  void AddTag(std::string_view const & k, std::string_view const & v);
+  void AddTag(std::string_view const & key, std::string_view const & value);
   bool HasTag(std::string_view const & key) const;
-  bool HasTag(std::string_view const & k, std::string_view const & v) const;
+  bool HasTag(std::string_view const & key, std::string_view const & value) const;
   bool HasAnyTag(std::unordered_multimap<std::string, std::string> const & tags) const;
 
   template <class Fn>
-  void UpdateTag(std::string const & k, Fn && fn)
+  void UpdateTag(std::string const & key, Fn && fn)
   {
     for (auto & tag : m_tags)
     {
-      if (tag.m_key == k)
+      if (tag.m_key == key)
       {
         fn(tag.m_value);
         return;
       }
     }
 
-    std::string v;
-    fn(v);
-    if (!v.empty())
-      AddTag(k, v);
+    std::string value;
+    fn(value);
+    if (!value.empty())
+      AddTag(key, value);
   }
 
   std::string GetTag(std::string const & key) const;
@@ -160,13 +160,13 @@ struct OsmElement
   EntityType m_memberType = EntityType::Unknown;
   std::string m_role;
 
-  std::vector<uint64_t> m_nds;
+  std::vector<uint64_t> m_nodes;
   std::vector<Member> m_members;
   std::vector<Tag> m_tags;
 };
 
 base::GeoObjectId GetGeoObjectId(OsmElement const & element);
 
-std::string DebugPrint(OsmElement const & e);
-std::string DebugPrint(OsmElement::EntityType e);
+std::string DebugPrint(OsmElement const & element);
+std::string DebugPrint(OsmElement::EntityType type);
 std::string DebugPrint(OsmElement::Tag const & tag);
diff --git a/generator/osm_source.cpp b/generator/osm_source.cpp
index 5e47c92d74..9a3530d1b7 100644
--- a/generator/osm_source.cpp
+++ b/generator/osm_source.cpp
@@ -53,32 +53,32 @@ uint64_t SourceReader::Read(char * buffer, uint64_t bufferSize)
 }
 
 // Functions ---------------------------------------------------------------------------------------
-void AddElementToCache(cache::IntermediateDataWriter & cache, OsmElement & em)
+void AddElementToCache(cache::IntermediateDataWriter & cache, OsmElement & element)
 {
-  switch (em.m_type)
+  switch (element.m_type)
   {
   case OsmElement::EntityType::Node:
   {
-    auto const pt = MercatorBounds::FromLatLon(em.m_lat, em.m_lon);
-    cache.AddNode(em.m_id, pt.y, pt.x);
+    auto const pt = MercatorBounds::FromLatLon(element.m_lat, element.m_lon);
+    cache.AddNode(element.m_id, pt.y, pt.x);
     break;
   }
   case OsmElement::EntityType::Way:
   {
     // Store way.
-    WayElement way(em.m_id);
-    for (uint64_t nd : em.Nodes())
+    WayElement way(element.m_id);
+    for (uint64_t nd : element.Nodes())
       way.nodes.push_back(nd);
 
     if (way.IsValid())
-      cache.AddWay(em.m_id, way);
+      cache.AddWay(element.m_id, way);
     break;
   }
   case OsmElement::EntityType::Relation:
   {
     // store relation
     RelationElement relation;
-    for (auto const & member : em.Members())
+    for (auto const & member : element.Members())
     {
       switch (member.m_type) {
       case OsmElement::EntityType::Node:
@@ -95,11 +95,11 @@ void AddElementToCache(cache::IntermediateDataWriter & cache, OsmElement & em)
       }
     }
 
-    for (auto const & tag : em.Tags())
+    for (auto const & tag : element.Tags())
       relation.tags.emplace(tag.m_key, tag.m_value);
 
     if (relation.IsValid())
-      cache.AddRelation(em.m_id, relation);
+      cache.AddRelation(element.m_id, relation);
 
     break;
   }
@@ -163,38 +163,38 @@ void ProcessOsmElementsFromO5M(SourceReader & stream, function<void(OsmElement *
   // thus first call of Nodes (Members) after Tags() will not return any results.
   // So don not reorder the "for" loops (!).
 
-  for (auto const & em : dataset)
+  for (auto const & entity : dataset)
   {
     OsmElement p;
-    p.m_id = em.id;
+    p.m_id = entity.id;
 
-    switch (em.type)
+    switch (entity.type)
     {
     case Type::Node:
     {
       p.m_type = OsmElement::EntityType::Node;
-      p.m_lat = em.lat;
-      p.m_lon = em.lon;
+      p.m_lat = entity.lat;
+      p.m_lon = entity.lon;
       break;
     }
     case Type::Way:
     {
       p.m_type = OsmElement::EntityType::Way;
-      for (uint64_t nd : em.Nodes())
+      for (uint64_t nd : entity.Nodes())
         p.AddNd(nd);
       break;
     }
     case Type::Relation:
     {
       p.m_type = OsmElement::EntityType::Relation;
-      for (auto const & member : em.Members())
+      for (auto const & member : entity.Members())
         p.AddMember(member.ref, translate(member.type), member.role);
       break;
     }
     default: break;
     }
 
-    for (auto const & tag : em.Tags())
+    for (auto const & tag : entity.Tags())
       p.AddTag(tag.key, tag.value);
 
     processor(&p);
diff --git a/generator/road_access_generator.cpp b/generator/road_access_generator.cpp
index 9c7f5c5c5a..8c1ce08d5a 100644
--- a/generator/road_access_generator.cpp
+++ b/generator/road_access_generator.cpp
@@ -310,15 +310,15 @@ void RoadAccessTagProcessor::Process(OsmElement const & elem, ofstream & oss)
         << 0 /* wildcard segment Idx */ << endl;
 
   // Barrier tags.
-  for (size_t pointIdx = 0; pointIdx < elem.m_nds.size(); ++pointIdx)
+  for (size_t pointIdx = 0; pointIdx < elem.m_nodes.size(); ++pointIdx)
   {
-    auto const it = m_barriers.find(elem.m_nds[pointIdx]);
+    auto const it = m_barriers.find(elem.m_nodes[pointIdx]);
     if (it == m_barriers.cend())
       continue;
 
     RoadAccess::Type const roadAccessType = it->second;
     // idx == 0 used as wildcard segment Idx, for nodes we store |pointIdx + 1| instead of |pointIdx|.
-    oss << ToString(m_vehicleType) << " " << ToString(roadAccessType) << " " << elem.id << " "
+    oss << ToString(m_vehicleType) << " " << ToString(roadAccessType) << " " << elem.m_id << " "
         << pointIdx + 1 << endl;
   }
 }