From 1923f01008d7de415255638cfc5c280b94f712ab Mon Sep 17 00:00:00 2001 From: vng Date: Thu, 26 Nov 2015 18:07:28 +0300 Subject: [PATCH] Review fixes. --- defines.hpp | 3 +- generator/feature_sorter.cpp | 66 ++++++++----------- indexer/feature_data.cpp | 2 +- indexer/feature_data.hpp | 4 +- indexer/search_index_builder.cpp | 8 ++- ...rse_geocoding.cpp => reverse_geocoder.cpp} | 37 +++++------ ...rse_geocoding.hpp => reverse_geocoder.hpp} | 11 +++- search/search.pro | 4 +- 8 files changed, 63 insertions(+), 72 deletions(-) rename search/{reverse_geocoding.cpp => reverse_geocoder.cpp} (72%) rename search/{reverse_geocoding.hpp => reverse_geocoder.hpp} (69%) diff --git a/defines.hpp b/defines.hpp index 68e0e5a7c9..1d202cfa4d 100644 --- a/defines.hpp +++ b/defines.hpp @@ -26,7 +26,8 @@ #define COMPRESSED_SEARCH_INDEX_FILE_TAG "csdx" #define FEATURE_OFFSETS_FILE_TAG "offs" #define RANKS_FILE_TAG "ranks" -#define SEARCH_TOKENS_FILE_TAG "stokens" +// Temporary addresses section that is used in search index generation. +#define SEARCH_TOKENS_FILE_TAG "addrtags" #define ROUTING_MATRIX_FILE_TAG "mercedes" #define ROUTING_EDGEDATA_FILE_TAG "daewoo" diff --git a/generator/feature_sorter.cpp b/generator/feature_sorter.cpp index e0760899a9..c469e8af67 100644 --- a/generator/feature_sorter.cpp +++ b/generator/feature_sorter.cpp @@ -91,10 +91,21 @@ namespace feature FilesContainerW m_writer; - vector m_geoFile, m_trgFile; + class TmpFile : public FileWriter + { + public: + explicit TmpFile(string const & filePath) : FileWriter(filePath) {} + ~TmpFile() + { + DeleteFileX(GetName()); + } + }; - enum { METADATA = 0, SEARCH_TOKENS = 1 }; - vector m_helperFile; + using TmpFiles = vector>; + TmpFiles m_geoFile, m_trgFile; + + enum { METADATA = 0, SEARCH_TOKENS = 1, FILES_COUNT = 2 }; + TmpFiles m_helperFile; // Mapping from feature id to offset in file section with the correspondent metadata. vector> m_metadataIndex; @@ -104,18 +115,6 @@ namespace feature gen::OsmID2FeatureID m_osm2ft; - static void DeleteFilesIfNeeded(vector const & v) - { - for (FileWriter * w : v) - { - if (w) - { - FileWriter::DeleteFileX(w->GetName()); - delete w; - } - } - } - public: FeaturesCollector2(string const & fName, DataHeader const & header, uint32_t versionDate) : FeaturesCollector(fName + DATA_FILE_TAG), m_writer(fName), @@ -124,20 +123,13 @@ namespace feature for (size_t i = 0; i < m_header.GetScalesCount(); ++i) { string const postfix = strings::to_string(i); - m_geoFile.push_back(new FileWriter(fName + GEOMETRY_FILE_TAG + postfix)); - m_trgFile.push_back(new FileWriter(fName + TRIANGLE_FILE_TAG + postfix)); + m_geoFile.push_back(make_unique(fName + GEOMETRY_FILE_TAG + postfix)); + m_trgFile.push_back(make_unique(fName + TRIANGLE_FILE_TAG + postfix)); } - m_helperFile.resize(2); - m_helperFile[METADATA] = new FileWriter(fName + METADATA_FILE_TAG); - m_helperFile[SEARCH_TOKENS] = new FileWriter(fName + SEARCH_TOKENS_FILE_TAG); - } - - ~FeaturesCollector2() - { - DeleteFilesIfNeeded(m_geoFile); - DeleteFilesIfNeeded(m_trgFile); - DeleteFilesIfNeeded(m_helperFile); + m_helperFile.resize(FILES_COUNT); + m_helperFile[METADATA] = make_unique(fName + METADATA_FILE_TAG); + m_helperFile[SEARCH_TOKENS] = make_unique(fName + SEARCH_TOKENS_FILE_TAG); } void Finish() @@ -161,24 +153,18 @@ namespace feature m_writer.Write(m_datFile.GetName(), DATA_FILE_TAG); // File Writer finalization function with appending to the main mwm file. - auto const finalizeFn = [this](FileWriter * & w, string const & tag, + auto const finalizeFn = [this](unique_ptr w, string const & tag, string const & postfix = string()) { - string const name = w->GetName(); - MY_SCOPE_GUARD(deleteFile, bind(&FileWriter::DeleteFileX, cref(name))); - w->Flush(); - m_writer.Write(name, tag + postfix); - - delete w; - w = 0; + m_writer.Write(w->GetName(), tag + postfix); }; for (size_t i = 0; i < m_header.GetScalesCount(); ++i) { string const postfix = strings::to_string(i); - finalizeFn(m_geoFile[i], GEOMETRY_FILE_TAG, postfix); - finalizeFn(m_trgFile[i], TRIANGLE_FILE_TAG, postfix); + finalizeFn(move(m_geoFile[i]), GEOMETRY_FILE_TAG, postfix); + finalizeFn(move(m_trgFile[i]), TRIANGLE_FILE_TAG, postfix); } { @@ -191,8 +177,8 @@ namespace feature } } - finalizeFn(m_helperFile[METADATA], METADATA_FILE_TAG); - finalizeFn(m_helperFile[SEARCH_TOKENS], SEARCH_TOKENS_FILE_TAG); + finalizeFn(move(m_helperFile[METADATA]), METADATA_FILE_TAG); + finalizeFn(move(m_helperFile[SEARCH_TOKENS]), SEARCH_TOKENS_FILE_TAG); m_writer.Finish(); @@ -530,7 +516,7 @@ namespace feature if (!fb.GetMetadata().Empty()) { - FileWriter * w = m_helperFile[METADATA]; + auto const & w = m_helperFile[METADATA]; uint64_t const offset = w->Pos(); ASSERT_LESS_OR_EQUAL(offset, numeric_limits::max(), ()); diff --git a/indexer/feature_data.cpp b/indexer/feature_data.cpp index 9dd042da2c..c286e4bf6f 100644 --- a/indexer/feature_data.cpp +++ b/indexer/feature_data.cpp @@ -246,7 +246,7 @@ void FeatureParams::AddAddress(string const & s) i = 0; } - AddStreet(s.substr(i, s.size() - i)); + AddStreet(s.substr(i)); } void FeatureParams::AddPlace(string const & s) diff --git a/indexer/feature_data.hpp b/indexer/feature_data.hpp index c1de7d4c4d..26f37d5c00 100644 --- a/indexer/feature_data.hpp +++ b/indexer/feature_data.hpp @@ -277,7 +277,7 @@ public: /// @param[in] fullStoring \n /// - true when saving in temporary files after first generation step \n /// - false when final mwm saving - template void Write(SinkT & sink, bool fullStoring) const + template void Write(TSink & sink, bool fullStoring) const { uint8_t const header = GetHeader(); @@ -295,7 +295,7 @@ public: BaseT::Write(sink, header); } - template void Read(SrcT & src) + template void Read(TSource & src) { using namespace feature; diff --git a/indexer/search_index_builder.cpp b/indexer/search_index_builder.cpp index 41dd76a877..21caff8363 100644 --- a/indexer/search_index_builder.cpp +++ b/indexer/search_index_builder.cpp @@ -1,6 +1,6 @@ #include "indexer/search_index_builder.hpp" -#include "search/reverse_geocoding.hpp" +#include "search/reverse_geocoder.hpp" #include "indexer/categories_holder.hpp" #include "indexer/classificator.hpp" @@ -284,7 +284,7 @@ void AddFeatureNameIndexPairs(FeaturesVectorTest & features, CategoriesHolder & Index mwmIndex; /// @ todo Make some better solution, or legalize MakeTemporary. mwmIndex.RegisterMap(platform::LocalCountryFile::MakeTemporary(features.GetFilePath())); - search::ReverseGeocoding rgc(&mwmIndex); + search::ReverseGeocoder rgc(mwmIndex); while (src.Size() > 0) { @@ -298,7 +298,7 @@ void AddFeatureNameIndexPairs(FeaturesVectorTest & features, CategoriesHolder & FeatureType ft; features.GetVector().GetByIndex(index, ft); - using TStreet = search::ReverseGeocoding::Street; + using TStreet = search::ReverseGeocoder::Street; vector streets; rgc.GetNearbyStreets(ft, street, streets); @@ -321,6 +321,8 @@ void AddFeatureNameIndexPairs(FeaturesVectorTest & features, CategoriesHolder & LOG(LINFO, ("Address: Matched percent", 100 * (1.0 - missing/double(address)))); LOG(LINFO, ("Address: Upper bounds", bounds)); + + /// @todo Delete SEARCH_TOKENS_FILE_TAG section in production code. } } // namespace diff --git a/search/reverse_geocoding.cpp b/search/reverse_geocoder.cpp similarity index 72% rename from search/reverse_geocoding.cpp rename to search/reverse_geocoder.cpp index 1c5f008ffa..1fcf5fa3b5 100644 --- a/search/reverse_geocoding.cpp +++ b/search/reverse_geocoder.cpp @@ -1,8 +1,8 @@ -#include "reverse_geocoding.hpp" +#include "reverse_geocoder.hpp" #include "indexer/feature.hpp" #include "indexer/feature_algo.hpp" -#include "indexer/feature_visibility.hpp" +#include "indexer/ftypes_matcher.hpp" #include "indexer/index.hpp" #include "indexer/scales.hpp" #include "indexer/search_string_utils.hpp" @@ -15,8 +15,8 @@ namespace { double constexpr kLookupRadiusM = 500.0; -size_t const kMaxStreetIndex = 16; -size_t const kPossiblePercent = 10; +size_t constexpr kMaxStreetIndex = 16; +size_t constexpr kSimilarityThresholdPercent = 10; /// @todo Need to check projection here? @@ -38,7 +38,7 @@ double CalculateMinDistance(FeatureType const & ft, m2::PointD const & pt) } // namespace template -void ReverseGeocoding::GetNearbyStreets(FeatureType const & addrFt, TCompare comp, +void ReverseGeocoder::GetNearbyStreets(FeatureType const & addrFt, TCompare comp, vector & streets) { m2::PointD const & center = feature::GetCenter(addrFt); @@ -50,9 +50,7 @@ void ReverseGeocoding::GetNearbyStreets(FeatureType const & addrFt, TCompare com if (ft.GetFeatureType() != feature::GEOM_LINE) return; - static feature::TypeSetChecker checker({"highway"}); - feature::TypesHolder types(ft); - if (!checker.IsEqualR(types.begin(), types.end())) + if (!ftypes::IsStreetChecker::Instance()(ft)) return; string name; @@ -64,7 +62,7 @@ void ReverseGeocoding::GetNearbyStreets(FeatureType const & addrFt, TCompare com streets.push_back({ft.GetID(), CalculateMinDistance(ft, center), comp(name)}); }; - m_index->ForEachInRect(fn, rect, scales::GetUpperScale()); + m_index.ForEachInRect(fn, rect, scales::GetUpperScale()); sort(streets.begin(), streets.end(), [](Street const & s1, Street const & s2) { @@ -72,7 +70,7 @@ void ReverseGeocoding::GetNearbyStreets(FeatureType const & addrFt, TCompare com }); } -void ReverseGeocoding::GetNearbyStreets(FeatureType const & addrFt, string const & keyName, +void ReverseGeocoder::GetNearbyStreets(FeatureType const & addrFt, string const & keyName, vector & streets) { strings::UniString const uniKey1 = strings::MakeUniString(keyName); @@ -83,28 +81,27 @@ void ReverseGeocoding::GetNearbyStreets(FeatureType const & addrFt, string const search::GetStreetNameAsKey(name, key); strings::UniString const uniKey2 = strings::MakeUniString(key); + ASSERT(!uniKey2.empty(), ()); return { strings::EditDistance(uniKey1.begin(), uniKey1.end(), uniKey2.begin(), uniKey2.end()), - uniKey1.size() }; + uniKey2.size() }; }, streets); } -size_t ReverseGeocoding::GetMatchedStreetIndex(vector const & streets) +size_t ReverseGeocoder::GetMatchedStreetIndex(vector const & streets) { - // do limit possible return values + // Do limit possible return values. size_t const count = min(streets.size(), kMaxStreetIndex); - // try to find exact match + // Find the exact match or the best match in kSimilarityTresholdPercent limit. + size_t res = count; + size_t minPercent = kSimilarityThresholdPercent + 1; for (size_t i = 0; i < count; ++i) + { if (streets[i].m_editDistance.first == 0) return i; - // try to find best match in kPossiblePercent limit - size_t res = count; - size_t minPercent = kPossiblePercent + 1; - for (size_t i = 0; i < count; ++i) - { size_t const p = streets[i].m_editDistance.first * 100 / streets[i].m_editDistance.second; - if (p < kPossiblePercent) + if (p < minPercent) { res = i; minPercent = p; diff --git a/search/reverse_geocoding.hpp b/search/reverse_geocoder.hpp similarity index 69% rename from search/reverse_geocoding.hpp rename to search/reverse_geocoder.hpp index 40493ec377..8d1065ac1d 100644 --- a/search/reverse_geocoding.hpp +++ b/search/reverse_geocoder.hpp @@ -13,17 +13,22 @@ class Index; namespace search { -class ReverseGeocoding +class ReverseGeocoder { - Index * m_index; + Index & m_index; public: - ReverseGeocoding(Index * p) : m_index(p) {} + ReverseGeocoder(Index & index) : m_index(index) {} struct Street { FeatureID m_id; + + /// Min distance to the street in meters. double m_distance; + + /// first - edit distance between actual street name and passed key name. + /// second - length of the actual street name. pair m_editDistance; }; diff --git a/search/search.pro b/search/search.pro index f14202ab54..e4420be2bc 100644 --- a/search/search.pro +++ b/search/search.pro @@ -27,7 +27,7 @@ HEADERS += \ region.hpp \ result.hpp \ retrieval.hpp \ - reverse_geocoding.hpp \ + reverse_geocoder.hpp \ search_common.hpp \ search_engine.hpp \ search_query.hpp \ @@ -51,7 +51,7 @@ SOURCES += \ region.cpp \ result.cpp \ retrieval.cpp \ - reverse_geocoding.cpp \ + reverse_geocoder.cpp \ search_engine.cpp \ search_query.cpp \ search_query_params.cpp \