Review fixes.

search/engine.cpp

@@ -309,7 +309,6 @@ void Engine::DoSearch(SearchParams const & params, m2::RectD const & viewport,
 
   processor.SetMode(params.GetMode());
   processor.SetSuggestsEnabled(params.GetSuggestsEnabled());
-  processor.SetOnResults(params.m_onResults);
 
   // This flag is needed for consistency with old search algorithm
   // only. It will be gone when we remove old search code.
@@ -322,7 +321,14 @@ void Engine::DoSearch(SearchParams const & params, m2::RectD const & viewport,
 
   Results res;
 
-  processor.SearchCoordinates(res);
+  try
+  {
+    processor.SearchCoordinates(res);
+  }
+  catch (CancelException const &)
+  {
+    LOG(LDEBUG, ("Search has been cancelled."));
+  }
 
   try
   {
@@ -338,7 +344,7 @@ void Engine::DoSearch(SearchParams const & params, m2::RectD const & viewport,
     if (!processor.IsCancelled())
       EmitResults(params, res);
   }
-  catch (Processor::CancelException const &)
+  catch (CancelException const &)
   {
     LOG(LDEBUG, ("Search has been cancelled."));
   }

search/pre_ranker.hpp

@@ -1,9 +1,7 @@
 #pragma once
 
 #include "search/intermediate_result.hpp"
-#include "search/ranker.hpp"
 
-#include "base/logging.hpp"
 #include "base/macros.hpp"
 
 #include "std/algorithm.hpp"
@@ -13,7 +11,6 @@
 
 namespace search
 {
-class Ranker;
 // Fast and simple pre-ranker for search results.
 class PreRanker
 {
@@ -46,8 +43,6 @@ public:
       fn(result.GetId(), result.GetInfo());
   }
 
-  Ranker * m_ranker;
-
 private:
   vector<PreResult1> m_results;
   size_t const m_limit;

search/processor.cpp

@@ -125,8 +125,7 @@ Processor::Processor(Index const & index, CategoriesHolder const & categories,
       {StringUtf8Multilang::kInternationalCode, StringUtf8Multilang::kEnglishCode},
       {StringUtf8Multilang::kDefaultCode}};
 
-  m_ranker.m_keywordsScorer.SetLanguages(langPriorities);
-  m_preRanker.m_ranker = &m_ranker;
+  m_ranker.SetLanguages(langPriorities);
 
   SetPreferredLocale("en");
 }
@@ -158,7 +157,9 @@ void Processor::SetPreferredLocale(string const & locale)
   // Default initialization.
   // If you want to reset input language, call SetInputLocale before search.
   SetInputLocale(locale);
+#ifdef FIND_LOCALITY_TEST
   m_ranker.SetLocalityFinderLanguage(code);
+#endif  // FIND_LOCALITY_TEST
 }
 
 void Processor::SetInputLocale(string const & locale)
@@ -228,7 +229,7 @@ void Processor::SetQuery(string const & query)
     m_tokens.resize(maxTokensCount);
 
   // Assign tokens and prefix to scorer.
-  m_ranker.m_keywordsScorer.SetKeywords(m_tokens.data(), m_tokens.size(), m_prefix);
+  m_ranker.SetKeywords(m_tokens.data(), m_tokens.size(), m_prefix);
 
   // get preferred types to show in results
   m_preferredTypes.clear();
@@ -253,12 +254,12 @@ void Processor::SetRankPivot(m2::PointD const & pivot)
 
 void Processor::SetLanguage(int id, int8_t lang)
 {
-  m_ranker.m_keywordsScorer.SetLanguage(GetLangIndex(id), lang);
+  m_ranker.SetLanguage(GetLangIndex(id), lang);
 }
 
 int8_t Processor::GetLanguage(int id) const
 {
-  return m_ranker.m_keywordsScorer.GetLanguage(GetLangIndex(id));
+  return m_ranker.GetLanguage(GetLangIndex(id));
 }
 
 m2::PointD Processor::GetPivotPoint() const
@@ -319,53 +320,35 @@ void Processor::SetViewportByIndex(m2::RectD const & viewport, size_t idx, bool
 
 void Processor::ClearCache(size_t ind) { m_viewport[ind].MakeEmpty(); }
 
-size_t Processor::GetCategoryLocales(int8_t(&arr)[3]) const
+TLocales Processor::GetCategoryLocales() const
 {
   static int8_t const enLocaleCode = CategoriesHolder::MapLocaleToInteger("en");
+  TLocales result;
 
   // Prepare array of processing locales. English locale is always present for category matching.
-  size_t count = 0;
   if (m_currentLocaleCode != -1)
-    arr[count++] = m_currentLocaleCode;
+    result.push_back(m_currentLocaleCode);
   if (m_inputLocaleCode != -1 && m_inputLocaleCode != m_currentLocaleCode)
-    arr[count++] = m_inputLocaleCode;
+    result.push_back(m_inputLocaleCode);
   if (enLocaleCode != m_currentLocaleCode && enLocaleCode != m_inputLocaleCode)
-    arr[count++] = enLocaleCode;
+    result.push_back(enLocaleCode);
 
-  return count;
+  return result;
 }
 
 template <typename ToDo>
 void Processor::ForEachCategoryType(StringSliceBase const & slice, ToDo && todo) const
 {
-  int8_t arrLocales[3];
-  int const localesCount = GetCategoryLocales(arrLocales);
-
-  ::search::ForEachCategoryType(slice, arrLocales, localesCount, m_categories, forward<ToDo>(todo));
+  ::search::ForEachCategoryType(slice, GetCategoryLocales(), m_categories, forward<ToDo>(todo));
 }
 
 void Processor::Search(Results & results, size_t limit)
 {
   Geocoder::Params geocoderParams;
-  InitParams(geocoderParams);
-  geocoderParams.m_mode = m_mode;
-  geocoderParams.m_pivot = GetPivotRect();
-  geocoderParams.m_accuratePivotCenter = GetPivotPoint();
-  m_geocoder.SetParams(geocoderParams);
+  InitGeocoderParams(geocoderParams);
 
   Ranker::Params rankerParams;
-  rankerParams.m_currentLocaleCode = m_currentLocaleCode;
-  if (m_mode == Mode::Viewport)
-    rankerParams.m_viewport = GetViewport();
-  rankerParams.m_position = GetPosition();
-  rankerParams.m_pivotRegion = GetPivotRegion();
-  rankerParams.m_preferredTypes = m_preferredTypes;
-  rankerParams.m_suggestsEnabled = m_suggestsEnabled;
-  rankerParams.m_query = m_query;
-  rankerParams.m_tokens = m_tokens;
-  rankerParams.m_prefix = m_prefix;
-  rankerParams.m_numCategoryLocales = GetCategoryLocales(rankerParams.m_categoryLocales);
-  m_ranker.SetParams(rankerParams);
+  InitRankerParams(rankerParams);
 
   if (m_tokens.empty())
     m_ranker.SuggestStrings(results);
@@ -603,6 +586,31 @@ void Processor::InitParams(QueryParams & params)
     params.m_langs.insert(GetLanguage(i));
 }
 
+void Processor::InitGeocoderParams(Geocoder::Params & params)
+{
+  InitParams(params);
+  params.m_mode = m_mode;
+  params.m_pivot = GetPivotRect();
+  params.m_accuratePivotCenter = GetPivotPoint();
+  m_geocoder.SetParams(params);
+}
+
+void Processor::InitRankerParams(Ranker::Params & params)
+{
+  params.m_currentLocaleCode = m_currentLocaleCode;
+  if (m_mode == Mode::Viewport)
+    params.m_viewport = GetViewport();
+  params.m_position = GetPosition();
+  params.m_pivotRegion = GetPivotRegion();
+  params.m_preferredTypes = m_preferredTypes;
+  params.m_suggestsEnabled = m_suggestsEnabled;
+  params.m_query = m_query;
+  params.m_tokens = m_tokens;
+  params.m_prefix = m_prefix;
+  params.m_categoryLocales = GetCategoryLocales();
+  m_ranker.SetParams(params);
+}
+
 void Processor::ClearCaches()
 {
   for (size_t i = 0; i < COUNT_V; ++i)

search/processor.hpp

@@ -79,7 +79,6 @@ public:
   void SetPreferredLocale(string const & locale);
   void SetInputLocale(string const & locale);
   void SetQuery(string const & query);
-  void SetOnResults(TOnResults const & onResults) { m_onResults = onResults; }
   // TODO (@y): this function must be removed.
   void SetRankPivot(m2::PointD const & pivot);
   inline void SetMode(Mode mode) { m_mode = mode; }
@@ -104,11 +103,9 @@ public:
   void SearchCoordinates(Results & res) const;
   //@}
 
-  struct CancelException
-  {
-  };
-
   void InitParams(QueryParams & params);
+  void InitGeocoderParams(Geocoder::Params & params);
+  void InitRankerParams(Ranker::Params & params);
 
   void ClearCaches();
 
@@ -130,14 +127,15 @@ protected:
   friend class PreResult2Maker;
   friend class Ranker;
 
-  size_t GetCategoryLocales(int8_t(&arr)[3]) const;
-
-  template <typename ToDo>
-  void ForEachCategoryType(StringSliceBase const & slice, ToDo && todo) const;
-
   using TMWMVector = vector<shared_ptr<MwmInfo>>;
   using TOffsetsVector = map<MwmSet::MwmId, vector<uint32_t>>;
   using TFHeader = feature::DataHeader;
+  using TLocales = buffer_vector<int8_t, 3>;
+
+  TLocales GetCategoryLocales() const;
+
+  template <typename ToDo>
+  void ForEachCategoryType(StringSliceBase const & slice, ToDo && todo) const;
 
   m2::PointD GetPivotPoint() const;
   m2::RectD GetPivotRect() const;
@@ -147,7 +145,6 @@ protected:
 
   CategoriesHolder const & m_categories;
   storage::CountryInfoGetter const & m_infoGetter;
-  TOnResults m_onResults;
 
   string m_region;
   string m_query;

search/ranker.cpp

@@ -49,7 +49,8 @@ void RemoveDuplicatingLinear(vector<IndexedValue> & values)
                values.end());
 }
 
-void RemoveStringPrefix(string const & str, string & res)
+// Chops off the last query token (the "prefix" one) from |str| and stores the result in |res|.
+void GetStringPrefix(string const & str, string & res)
 {
   search::Delimiters delims;
   // Find start iterator of prefix in input query.
@@ -62,8 +63,8 @@ void RemoveStringPrefix(string const & str, string & res)
 
     if (delims(*prev))
       break;
-    else
-      iter = prev;
+
+    iter = prev;
   }
 
   // Assign result with input string without prefix.
@@ -89,28 +90,29 @@ ftypes::Type GetLocalityIndex(feature::TypesHolder const & types)
 }
 
 /// Makes continuous range for tokens and prefix.
-template <class TIter, class ValueT>
+template <class TIter, class TValue>
 class CombinedIter
 {
-  TIter m_i, m_end;
-  ValueT const * m_val;
+  TIter m_cur;
+  TIter m_end;
+  TValue const * m_val;
 
 public:
-  CombinedIter(TIter i, TIter end, ValueT const * val) : m_i(i), m_end(end), m_val(val) {}
+  CombinedIter(TIter cur, TIter end, TValue const * val) : m_cur(cur), m_end(end), m_val(val) {}
 
-  ValueT const & operator*() const
+  TValue const & operator*() const
   {
-    ASSERT(m_val != 0 || m_i != m_end, ("dereferencing of empty iterator"));
-    if (m_i != m_end)
-      return *m_i;
+    ASSERT(m_val != 0 || m_cur != m_end, ("dereferencing of empty iterator"));
+    if (m_cur != m_end)
+      return *m_cur;
 
     return *m_val;
   }
 
   CombinedIter & operator++()
   {
-    if (m_i != m_end)
-      ++m_i;
+    if (m_cur != m_end)
+      ++m_cur;
     else
       m_val = 0;
     return *this;
@@ -118,12 +120,12 @@ public:
 
   bool operator==(CombinedIter const & other) const
   {
-    return m_val == other.m_val && m_i == other.m_i;
+    return m_val == other.m_val && m_cur == other.m_cur;
   }
 
   bool operator!=(CombinedIter const & other) const
   {
-    return m_val != other.m_val || m_i != other.m_i;
+    return m_val != other.m_val || m_cur != other.m_cur;
   }
 };
 }  // namespace
@@ -133,7 +135,6 @@ class PreResult2Maker
   Ranker & m_ranker;
   Index const & m_index;
   Geocoder::Params const & m_params;
-  Ranker::Params const & m_rankerParams;
   storage::CountryInfoGetter const & m_infoGetter;
 
   unique_ptr<Index::FeaturesLoaderGuard> m_pFV;
@@ -192,8 +193,7 @@ class PreResult2Maker
     feature::TypesHolder holder(ft);
     vector<pair<size_t, size_t>> matched(slice.Size());
     ForEachCategoryType(QuerySlice(slice), m_ranker.m_params.m_categoryLocales,
-                        m_ranker.m_params.m_numCategoryLocales, m_ranker.m_categories,
-                        [&](size_t i, uint32_t t)
+                        m_ranker.m_categories, [&](size_t i, uint32_t t)
                         {
                           ++matched[i].second;
                           if (holder.Has(t))
@@ -218,7 +218,7 @@ class PreResult2Maker
     case SearchModel::SEARCH_TYPE_VILLAGE: return rank /= 1.5;
     case SearchModel::SEARCH_TYPE_CITY:
     {
-      if (m_rankerParams.m_viewport.IsPointInside(center))
+      if (m_ranker.m_params.m_viewport.IsPointInside(center))
         return rank * 2;
 
       storage::CountryInfo info;
@@ -226,7 +226,7 @@ class PreResult2Maker
         m_infoGetter.GetRegionInfo(center, info);
       else
         m_infoGetter.GetRegionInfo(country, info);
-      if (info.IsNotEmpty() && info.m_name == m_rankerParams.m_pivotRegion)
+      if (info.IsNotEmpty() && info.m_name == m_ranker.m_params.m_pivotRegion)
         return rank *= 1.7;
     }
     case SearchModel::SEARCH_TYPE_COUNTRY:
@@ -240,12 +240,8 @@ class PreResult2Maker
 public:
   explicit PreResult2Maker(Ranker & ranker, Index const & index,
                            storage::CountryInfoGetter const & infoGetter,
-                           Geocoder::Params const & params, Ranker::Params const & rankerParams)
-    : m_ranker(ranker)
-    , m_index(index)
-    , m_params(params)
-    , m_rankerParams(rankerParams)
-    , m_infoGetter(infoGetter)
+                           Geocoder::Params const & params)
+    : m_ranker(ranker), m_index(index), m_params(params), m_infoGetter(infoGetter)
   {
   }
 
@@ -258,7 +254,7 @@ public:
 
     LoadFeature(res1.GetId(), ft, center, name, country);
 
-    auto res2 = make_unique<PreResult2>(ft, &res1, center, m_rankerParams.m_position /* pivot */,
+    auto res2 = make_unique<PreResult2>(ft, &res1, center, m_ranker.m_params.m_position /* pivot */,
                                         name, country);
 
     search::RankingInfo info;
@@ -286,7 +282,7 @@ void Ranker::MakePreResult2(Geocoder::Params const & geocoderParams, vector<Inde
   m_preRanker.Filter(m_params.m_viewportSearch);
 
   // Makes PreResult2 vector.
-  PreResult2Maker maker(*this, m_index, m_infoGetter, geocoderParams, m_params);
+  PreResult2Maker maker(*this, m_index, m_infoGetter, geocoderParams);
   m_preRanker.ForEach(
       [&](PreResult1 const & r)
       {
@@ -373,7 +369,7 @@ void Ranker::GetSuggestion(string const & name, string & suggest) const
   if (!prefixMatched || fullPrefixMatched)
     return;
 
-  RemoveStringPrefix(m_params.m_query, suggest);
+  GetStringPrefix(m_params.m_query, suggest);
 
   // Appends unmatched result's tokens to the suggestion.
   for (size_t i = 0; i < tokens.size(); ++i)
@@ -390,15 +386,15 @@ void Ranker::SuggestStrings(Results & res)
   if (m_params.m_prefix.empty() || !m_params.m_suggestsEnabled)
     return;
 
-  string prolog;
-  RemoveStringPrefix(m_params.m_query, prolog);
+  string prologue;
+  GetStringPrefix(m_params.m_query, prologue);
 
-  for (int i = 0; i < m_params.m_numCategoryLocales; ++i)
-    MatchForSuggestions(m_params.m_prefix, m_params.m_categoryLocales[i], prolog, res);
+  for (int i = 0; i < m_params.m_categoryLocales.size(); ++i)
+    MatchForSuggestions(m_params.m_prefix, m_params.m_categoryLocales[i], prologue, res);
 }
 
 void Ranker::MatchForSuggestions(strings::UniString const & token, int8_t locale,
-                                 string const & prolog, Results & res)
+                                 string const & prologue, Results & res)
 {
   for (auto const & suggest : m_suggests)
   {
@@ -409,7 +405,7 @@ void Ranker::MatchForSuggestions(strings::UniString const & token, int8_t locale
         StartsWith(s.begin(), s.end(), token.begin(), token.end()))
     {
       string const utf8Str = strings::ToUtf8(s);
-      Result r(utf8Str, prolog + utf8Str + " ");
+      Result r(utf8Str, prologue + utf8Str + " ");
       MakeResultHighlight(r);
       res.AddResult(move(r));
     }
@@ -518,11 +514,4 @@ void Ranker::ClearCaches()
   m_locality.ClearCache();
 #endif  // FIND_LOCALITY_TEST
 }
-
-void Ranker::SetLocalityFinderLanguage(int8_t code)
-{
-#ifdef FIND_LOCALITY_TEST
-  m_locality.SetLanguage(code);
-#endif  // FIND_LOCALITY_TEST
-}
 }  // namespace search

search/ranker.hpp

@@ -18,6 +18,7 @@
 
 #include "std/set.hpp"
 #include "std/string.hpp"
+#include "std/utility.hpp"
 #include "std/vector.hpp"
 
 #define FIND_LOCALITY_TEST
@@ -28,6 +29,7 @@
 
 class CategoriesHolder;
 class Index;
+
 namespace storage
 {
 class CountryInfoGetter;
@@ -42,6 +44,8 @@ class Ranker
 public:
   struct Params
   {
+    using TLocales = buffer_vector<int8_t, 3>;
+
     bool m_viewportSearch = false;
 
     int8_t m_currentLocaleCode = CategoriesHolder::kEnglishCode;
@@ -57,8 +61,7 @@ public:
     // We need it here to make suggestions.
     strings::UniString m_prefix;
 
-    int8_t m_categoryLocales[3];
-    size_t m_numCategoryLocales = 0;
+    TLocales m_categoryLocales;
   };
 
   Ranker(PreRanker & preRanker, Index const & index, storage::CountryInfoGetter const & infoGetter,
@@ -101,19 +104,41 @@ public:
 
   void ClearCaches();
 
-  void SetLocalityFinderLanguage(int8_t code);
+#ifdef FIND_LOCALITY_TEST
+  inline void SetLocalityFinderLanguage(int8_t code) { m_locality.SetLanguage(code); }
+#endif  // FIND_LOCALITY_TEST
+
+  inline void SetLanguage(pair<int, int> const & ind, int8_t lang)
+  {
+    m_keywordsScorer.SetLanguage(ind, lang);
+  }
+
+  inline int8_t GetLanguage(pair<int, int> const & ind) const
+  {
+    return m_keywordsScorer.GetLanguage(ind);
+  }
+
+  inline void SetLanguages(vector<vector<int8_t>> const & languagePriorities)
+  {
+    m_keywordsScorer.SetLanguages(languagePriorities);
+  }
+
+  inline void SetKeywords(KeywordMatcher::StringT const * keywords, size_t count,
+                          KeywordMatcher::StringT const & prefix)
+  {
+    m_keywordsScorer.SetKeywords(keywords, count, prefix);
+  }
 
   inline void BailIfCancelled() { ::search::BailIfCancelled(m_cancellable); }
 
-  KeywordLangMatcher m_keywordsScorer;
-
+private:
   friend class PreResult2Maker;
 
-private:
   Params m_params;
   ReverseGeocoder const m_reverseGeocoder;
   PreRanker & m_preRanker;
   my::Cancellable const & m_cancellable;
+  KeywordLangMatcher m_keywordsScorer;
 
 #ifdef FIND_LOCALITY_TEST
   mutable LocalityFinder m_locality;