From 1c660f706650ee264d24e7e259e289d1c00a8f6e Mon Sep 17 00:00:00 2001 From: Anatoly Serdtcev Date: Tue, 30 Jul 2019 16:54:49 +0300 Subject: [PATCH] [geocoder] Fix for review --- geocoder/geocoder_cli/geocoder_cli.cpp | 2 +- geocoder/hierarchy.cpp | 22 ++++++++++++---------- geocoder/hierarchy.hpp | 4 ++-- geocoder/hierarchy_reader.cpp | 12 ++++++------ geocoder/index.cpp | 6 +++--- geocoder/name_dictionary.cpp | 9 +++++---- geocoder/name_dictionary.hpp | 8 ++++---- 7 files changed, 33 insertions(+), 30 deletions(-) diff --git a/geocoder/geocoder_cli/geocoder_cli.cpp b/geocoder/geocoder_cli/geocoder_cli.cpp index 98bc9b176e..1fa8602c11 100644 --- a/geocoder/geocoder_cli/geocoder_cli.cpp +++ b/geocoder/geocoder_cli/geocoder_cli.cpp @@ -37,7 +37,7 @@ void PrintResults(Hierarchy const & hierarchy, vector const & results) auto const * delimiter = ""; for (size_t i = 0; i < static_cast(Type::Count); ++i) { - if (e->m_normalizedAddress[i]) + if (e->m_normalizedAddress[i] != NameDictionary::kUnspecifiedPosition) { auto type = static_cast(i); cout << delimiter << ToString(type) << ": " << e->GetNormalizedName(type, dictionary); diff --git a/geocoder/hierarchy.cpp b/geocoder/hierarchy.cpp index 718934ec1c..146bf34658 100644 --- a/geocoder/hierarchy.cpp +++ b/geocoder/hierarchy.cpp @@ -17,13 +17,13 @@ namespace geocoder { // Hierarchy::Entry -------------------------------------------------------------------------------- bool Hierarchy::Entry::DeserializeFromJSON(string const & jsonStr, - NameDictionaryMaker & normalizedNameDictionaryMaker, + NameDictionaryBuilder & normalizedNameDictionaryBuilder, ParsingStats & stats) { try { base::Json root(jsonStr.c_str()); - return DeserializeFromJSONImpl(root.get(), jsonStr, normalizedNameDictionaryMaker, stats); + return DeserializeFromJSONImpl(root.get(), jsonStr, normalizedNameDictionaryBuilder, stats); } catch (base::Json::Exception const & e) { @@ -33,9 +33,9 @@ bool Hierarchy::Entry::DeserializeFromJSON(string const & jsonStr, } // todo(@m) Factor out to geojson.hpp? Add geojson to myjansson? -bool Hierarchy::Entry::DeserializeFromJSONImpl(json_t * const root, string const & jsonStr, - NameDictionaryMaker & normalizedNameDictionaryMaker, - ParsingStats & stats) +bool Hierarchy::Entry::DeserializeFromJSONImpl( + json_t * const root, string const & jsonStr, + NameDictionaryBuilder & normalizedNameDictionaryBuilder, ParsingStats & stats) { if (!json_is_object(root)) { @@ -69,18 +69,20 @@ bool Hierarchy::Entry::DeserializeFromJSONImpl(json_t * const root, string const continue; auto normalizedValue = strings::JoinStrings(tokens, " "); - m_normalizedAddress[i] = normalizedNameDictionaryMaker.Add(normalizedValue); + m_normalizedAddress[i] = normalizedNameDictionaryBuilder.Add(normalizedValue); m_type = static_cast(i); } auto const & subregion = m_normalizedAddress[static_cast(Type::Subregion)]; auto const & locality = m_normalizedAddress[static_cast(Type::Locality)]; - if (m_type == Type::Street && !locality && !subregion) + if (m_type == Type::Street && locality == NameDictionary::kUnspecifiedPosition && + subregion == NameDictionary::kUnspecifiedPosition) { ++stats.m_noLocalityStreets; return false; } - if (m_type == Type::Building && !locality && !subregion) + if (m_type == Type::Building && locality == NameDictionary::kUnspecifiedPosition && + subregion == NameDictionary::kUnspecifiedPosition) { ++stats.m_noLocalityBuildings; return false; @@ -144,10 +146,10 @@ bool Hierarchy::IsParentTo(Hierarchy::Entry const & entry, Hierarchy::Entry cons { for (size_t i = 0; i < static_cast(geocoder::Type::Count); ++i) { - if (!entry.m_normalizedAddress[i]) + if (entry.m_normalizedAddress[i] == NameDictionary::kUnspecifiedPosition) continue; - if (!toEntry.m_normalizedAddress[i]) + if (toEntry.m_normalizedAddress[i] == NameDictionary::kUnspecifiedPosition) return false; auto const pos1 = entry.m_normalizedAddress[i]; auto const pos2 = toEntry.m_normalizedAddress[i]; diff --git a/geocoder/hierarchy.hpp b/geocoder/hierarchy.hpp index 71d56d90c8..86c0508562 100644 --- a/geocoder/hierarchy.hpp +++ b/geocoder/hierarchy.hpp @@ -60,10 +60,10 @@ public: struct Entry { bool DeserializeFromJSON(std::string const & jsonStr, - NameDictionaryMaker & normalizedNameDictionaryMaker, + NameDictionaryBuilder & normalizedNameDictionaryBuilder, ParsingStats & stats); bool DeserializeFromJSONImpl(json_t * const root, std::string const & jsonStr, - NameDictionaryMaker & normalizedNameDictionaryMaker, + NameDictionaryBuilder & normalizedNameDictionaryBuilder, ParsingStats & stats); std::string const & GetNormalizedName(Type type, diff --git a/geocoder/hierarchy_reader.cpp b/geocoder/hierarchy_reader.cpp index 12827c87c2..9013ba5957 100644 --- a/geocoder/hierarchy_reader.cpp +++ b/geocoder/hierarchy_reader.cpp @@ -61,7 +61,7 @@ Hierarchy HierarchyReader::Read(unsigned int readersCount) LOG(LINFO, ("Reading entries...")); vector entries; - NameDictionaryMaker nameDictionaryMaker; + NameDictionaryBuilder nameDictionaryBuilder; ParsingStats stats{}; base::thread_pool::computational::ThreadPool threadPool{readersCount}; @@ -86,7 +86,7 @@ Hierarchy HierarchyReader::Read(unsigned int readersCount) if (auto & position = entry.m_normalizedAddress[i]) { auto const & name = taskNameDictionary.Get(position); - position = nameDictionaryMaker.Add(name); + position = nameDictionaryBuilder.Add(name); } } } @@ -118,7 +118,7 @@ Hierarchy HierarchyReader::Read(unsigned int readersCount) ("Entries whose names do not match their most specific addresses:", stats.m_mismatchedNames)); LOG(LINFO, ("(End of stats.)")); - return Hierarchy{move(entries), nameDictionaryMaker.Release()}; + return Hierarchy{move(entries), nameDictionaryBuilder.Release()}; } void HierarchyReader::CheckDuplicateOsmIds(vector const & entries, @@ -168,7 +168,7 @@ HierarchyReader::ParsingResult HierarchyReader::DeserializeEntries( { vector entries; entries.reserve(bufferSize); - NameDictionaryMaker nameDictionaryMaker; + NameDictionaryBuilder nameDictionaryBuilder; ParsingStats stats; for (size_t i = 0; i < bufferSize; ++i) @@ -192,7 +192,7 @@ HierarchyReader::ParsingResult HierarchyReader::DeserializeEntries( auto const osmId = base::GeoObjectId(encodedId); entry.m_osmId = osmId; - if (!entry.DeserializeFromJSON(json, nameDictionaryMaker, stats)) + if (!entry.DeserializeFromJSON(json, nameDictionaryBuilder, stats)) continue; if (entry.m_type == Type::Count) @@ -207,7 +207,7 @@ HierarchyReader::ParsingResult HierarchyReader::DeserializeEntries( entries.push_back(move(entry)); } - return {move(entries), nameDictionaryMaker.Release(), move(stats)}; + return {move(entries), nameDictionaryBuilder.Release(), move(stats)}; } // static diff --git a/geocoder/index.cpp b/geocoder/index.cpp index 1a8a06fab1..3aef2674e0 100644 --- a/geocoder/index.cpp +++ b/geocoder/index.cpp @@ -150,9 +150,9 @@ void Index::AddHouses(unsigned int loadThreadsCount) buildingDoc.m_normalizedAddress[static_cast(Type::Locality)]; NameDictionary::Position relation = NameDictionary::kUnspecifiedPosition; - if (street) + if (street != NameDictionary::kUnspecifiedPosition) relation = street; - else if (locality) + else if (locality != NameDictionary::kUnspecifiedPosition) relation = locality; else continue; @@ -174,7 +174,7 @@ void Index::AddHouses(unsigned int loadThreadsCount) if (indexed) { - auto processedCount = numIndexed.fetch_add(1) + 1; + auto const processedCount = numIndexed.fetch_add(1) + 1; if (processedCount % kLogBatch == 0) LOG(LINFO, ("Indexed", processedCount, "houses")); } diff --git a/geocoder/name_dictionary.cpp b/geocoder/name_dictionary.cpp index 98bbfd4d27..7d45ab5081 100644 --- a/geocoder/name_dictionary.cpp +++ b/geocoder/name_dictionary.cpp @@ -2,6 +2,7 @@ #include "base/assert.hpp" +#include #include namespace geocoder @@ -16,13 +17,13 @@ std::string const & NameDictionary::Get(Position position) const NameDictionary::Position NameDictionary::Add(std::string const & s) { - CHECK_LESS(m_stock.size(), UINT32_MAX, ()); + CHECK_LESS(m_stock.size(), std::numeric_limits::max(), ()); m_stock.push_back(s); return m_stock.size(); // index + 1 } -// NameDictionaryMaker ----------------------------------------------------------------------------- -NameDictionary::Position NameDictionaryMaker::Add(std::string const & s) +// NameDictionaryBuilder ----------------------------------------------------------------------------- +NameDictionary::Position NameDictionaryBuilder::Add(std::string const & s) { auto indexItem = m_index.find(s); if (indexItem != m_index.end()) @@ -34,7 +35,7 @@ NameDictionary::Position NameDictionaryMaker::Add(std::string const & s) return p; } -NameDictionary NameDictionaryMaker::Release() +NameDictionary NameDictionaryBuilder::Release() { m_index.clear(); return std::move(m_dictionary); diff --git a/geocoder/name_dictionary.hpp b/geocoder/name_dictionary.hpp index 344046cb0e..eaac18a6e1 100644 --- a/geocoder/name_dictionary.hpp +++ b/geocoder/name_dictionary.hpp @@ -29,12 +29,12 @@ private: std::vector m_stock; }; -class NameDictionaryMaker +class NameDictionaryBuilder { public: - NameDictionaryMaker() = default; - NameDictionaryMaker(NameDictionaryMaker const &) = delete; - NameDictionaryMaker & operator=(NameDictionaryMaker const &) = delete; + NameDictionaryBuilder() = default; + NameDictionaryBuilder(NameDictionaryBuilder const &) = delete; + NameDictionaryBuilder & operator=(NameDictionaryBuilder const &) = delete; NameDictionary::Position Add(std::string const & s); NameDictionary Release();