Code review.

9 files changed, 35 insertions, 40 deletions

diff --git a/indexer/editable_map_object.cpp b/indexer/editable_map_object.cpp
index 9c63a55091..5d63d3e89b 100644
--- a/indexer/editable_map_object.cpp
+++ b/indexer/editable_map_object.cpp
@@ -195,7 +195,7 @@ void EditableMapObject::SetPointType() { m_geomType = feature::EGeomType::GEOM_P
 // static
 bool EditableMapObject::ValidateBuildingLevels(string const & buildingLevels)
 {
-  if (buildingLevels.size() > log10(kMaximumLevelsEditableByUsers) + 1)
+  if (buildingLevels.size() > 18 /* max number of digits in uint_64 */)
     return false;
 
   uint64_t levels;
@@ -205,6 +205,8 @@ bool EditableMapObject::ValidateBuildingLevels(string const & buildingLevels)
 // static
 bool EditableMapObject::ValidateHouseNumber(string const & houseNumber)
 {
+  // TODO(mgsergio): Make a better validation, use real samples for example.
+
   if (houseNumber.empty())
     return true;
 
@@ -219,7 +221,7 @@ bool EditableMapObject::ValidateHouseNumber(string const & houseNumber)
   for (auto const c : us)
   {
     // Valid house numbers contain at least one digit.
-    if (c >= '0' && c <= '9')
+    if (strings::IsASCIIDigit(c))
       return true;
   }
   return false;
@@ -249,7 +251,7 @@ bool EditableMapObject::ValidateFlats(string const & flats)
 // static
 bool EditableMapObject::ValidatePostCode(string const & postCode)
 {
-  return search::LooksLikePostcode(postCode, false /* check whole matching */);
+  return search::LooksLikePostcode(postCode, false /* IsPrefix */);
 }
 
 // static
@@ -258,24 +260,24 @@ bool EditableMapObject::ValidatePhone(string const & phone)
   if (phone.empty())
     return true;
 
-  auto start = begin(phone);
-  auto const stop = end(phone);
+  auto curr = begin(phone);
+  auto const last = end(phone);
 
   auto const kMaxNumberLen = 15;
   auto const kMinNumberLen = 5;
 
-  if (*start == '+')
-    ++start;
+  if (*curr == '+')
+    ++curr;
 
   auto digitsCount = 0;
-  for (; start != stop; ++start)
+  for (; curr != last; ++curr)
   {
-    auto const isCharValid = isdigit(*start) || *start == '(' ||
-                             *start == ')' || *start == ' ' || *start == '-';
+    auto const isCharValid = isdigit(*curr) || *curr == '(' ||
+                             *curr == ')' || *curr == ' ' || *curr == '-';
     if (!isCharValid)
       return false;
 
-    if (isdigit(*start))
+    if (isdigit(*curr))
       ++digitsCount;
   }
 
@@ -283,19 +285,13 @@ bool EditableMapObject::ValidatePhone(string const & phone)
 }
 
 // static
-bool EditableMapObject::ValidateFax(string const & fax)
-{
-  return ValidatePhone(fax);
-}
-
-// static
 bool EditableMapObject::ValidateWebsite(string const & site)
 {
   if (site.empty())
     return true;
 
   auto const dotPos = find(begin(site), end(site), '.');
-  // Site should contain at least one dot but not at the begining and not at the and.
+  // Site should contain at least one dot but not at the begining/and.
   if (dotPos == end(site) || site.front() == '.' || site.back() == '.')
     return false;
 
diff --git a/indexer/editable_map_object.hpp b/indexer/editable_map_object.hpp
index ac7b0db100..747082bf0d 100644
--- a/indexer/editable_map_object.hpp
+++ b/indexer/editable_map_object.hpp
@@ -111,7 +111,6 @@ public:
   static bool ValidateFlats(string const & flats);
   static bool ValidatePostCode(string const & postCode);
   static bool ValidatePhone(string const & phone);
-  static bool ValidateFax(string const & fax);
   static bool ValidateWebsite(string const & site);
   static bool ValidateEmail(string const & email);
 
diff --git a/indexer/indexer_tests/editable_map_object_test.cpp b/indexer/indexer_tests/editable_map_object_test.cpp
index 2f808d84ae..5857a8f8ce 100644
--- a/indexer/indexer_tests/editable_map_object_test.cpp
+++ b/indexer/indexer_tests/editable_map_object_test.cpp
@@ -2,6 +2,8 @@
 
 #include "indexer/editable_map_object.hpp"
 
+namespace
+{
 using osm::EditableMapObject;
 
 UNIT_TEST(EditableMapObject_SetWebsite)
@@ -10,7 +12,7 @@ UNIT_TEST(EditableMapObject_SetWebsite)
   emo.SetWebsite("https://some.thing.org");
   TEST_EQUAL(emo.GetWebsite(), "https://some.thing.org", ());
 
-  emo.SetWebsite("some.thing.org");
+  emo.SetWebsite("http://some.thing.org");
   TEST_EQUAL(emo.GetWebsite(), "http://some.thing.org", ());
 
   emo.SetWebsite("some.thing.org");
@@ -18,7 +20,6 @@ UNIT_TEST(EditableMapObject_SetWebsite)
 
   emo.SetWebsite("");
   TEST_EQUAL(emo.GetWebsite(), "", ());
-
 }
 
 UNIT_TEST(EditableMapObject_ValidateBuildingLevels)
@@ -54,6 +55,7 @@ UNIT_TEST(EditableMapObject_ValidateFlats)
   TEST(EditableMapObject::ValidateFlats("123-456;a-e"), ());
   TEST(EditableMapObject::ValidateFlats("123-456"), ());
   TEST(EditableMapObject::ValidateFlats("123-456; 43-45"), ());
+  TEST(!EditableMapObject::ValidateFlats("123-456, 43-45"), ());
   TEST(!EditableMapObject::ValidateFlats("234-234 124"), ());
   TEST(!EditableMapObject::ValidateFlats("123-345-567"), ());
   TEST(!EditableMapObject::ValidateFlats("234-234;234("), ());
@@ -84,11 +86,6 @@ UNIT_TEST(EditableMapObject_ValidatePhone)
   TEST(!EditableMapObject::ValidatePhone("000 000 00b"), ());
 }
 
-// See validate phone.
-// UNIT_TEST(EditableMapObject_ValidateFax)
-// {
-// }
-
 UNIT_TEST(EditableMapObject_ValidateWebsite)
 {
   TEST(EditableMapObject::ValidateWebsite(""), ());
@@ -111,3 +108,4 @@ UNIT_TEST(EditableMapObject_ValidateEmail)
   TEST(!EditableMapObject::ValidateEmail("e@ma@i.l"), ());
   TEST(!EditableMapObject::ValidateEmail("e@mail"), ());
 }
+}  // namespace
diff --git a/indexer/postcodes_matcher.cpp b/indexer/postcodes_matcher.cpp
index 9c7889d793..143b462216 100644
--- a/indexer/postcodes_matcher.cpp
+++ b/indexer/postcodes_matcher.cpp
@@ -102,7 +102,7 @@ public:
   // patterns.
   //
   // Complexity: O(total length of tokens in |slice|).
-  bool HasString(StringSliceBase const & slice, bool handleAsPrefix) const
+  bool HasString(StringSliceBase const & slice, bool isPrefix) const
   {
     if (slice.Size() == 0)
       return m_root.m_isLeaf;
@@ -120,7 +120,7 @@ public:
     if (!cur)
       return false;
 
-    if (handleAsPrefix)
+    if (isPrefix)
       return true;
 
     return cur->m_isLeaf;
@@ -159,18 +159,18 @@ PostcodesMatcher const & GetPostcodesMatcher()
 }
 }  // namespace
 
-bool LooksLikePostcode(StringSliceBase const & slice, bool handleAsPrefix)
+bool LooksLikePostcode(StringSliceBase const & slice, bool isPrefix)
 {
-  return GetPostcodesMatcher().HasString(slice, handleAsPrefix);
+  return GetPostcodesMatcher().HasString(slice, isPrefix);
 }
 
-bool LooksLikePostcode(string const & s, bool handleAsPrefix)
+bool LooksLikePostcode(string const & s, bool isPrefix)
 {
   vector<UniString> tokens;
   bool const lastTokenIsPrefix =
       TokenizeStringAndCheckIfLastTokenIsPrefix(s, tokens, search::Delimiters());
 
-  return LooksLikePostcode(NoPrefixStringSlice(tokens), handleAsPrefix && lastTokenIsPrefix);
+  return LooksLikePostcode(NoPrefixStringSlice(tokens), isPrefix && lastTokenIsPrefix);
 }
 
 size_t GetMaxNumTokensInPostcode() { return GetPostcodesMatcher().GetMaxNumTokensInPostcode(); }
diff --git a/indexer/postcodes_matcher.hpp b/indexer/postcodes_matcher.hpp
index 3d0ab900a9..d7af92e984 100644
--- a/indexer/postcodes_matcher.hpp
+++ b/indexer/postcodes_matcher.hpp
@@ -7,11 +7,11 @@
 
 namespace search
 {
-bool LooksLikePostcode(StringSliceBase const & slice, bool handleAsPrefix);
+/// If isPrefix is true returns true if some postcode starts with s.
+/// If isPrefix is false returns true if s equals to some postcode.
+bool LooksLikePostcode(StringSliceBase const & slice, bool isPrefix);
 /// Splits s into tokens and call LooksLikePostcode(TokenSlice) on the result.
-/// If handleAsPrefix is true returns true if some postcode starts with s.
-/// If handleAsPrefix is false returns true if s equals to some postcode.
-bool LooksLikePostcode(string const & s, bool handleAsPrefix);
+bool LooksLikePostcode(string const & s, bool isPrefix);
 
 size_t GetMaxNumTokensInPostcode();
 }  // namespace search
diff --git a/indexer/string_slice.hpp b/indexer/string_slice.hpp
index 4bf3eb6a13..510cdbe8cb 100644
--- a/indexer/string_slice.hpp
+++ b/indexer/string_slice.hpp
@@ -2,6 +2,7 @@
 
 #include "base/string_utils.hpp"
 
+#include "std/cstdint.hpp"
 #include "std/string.hpp"
 #include "std/vector.hpp"
 
diff --git a/search/processor.cpp b/search/processor.cpp
index 0cc1fb9d26..c81926d3c3 100644
--- a/search/processor.cpp
+++ b/search/processor.cpp
@@ -611,7 +611,7 @@ class PreResult2Maker
 
     feature::TypesHolder holder(ft);
     vector<pair<size_t, size_t>> matched(slice.Size());
-    m_processor.ForEachCategoryTypes(v2::QuerySliceOnTokens(slice), [&](size_t i, uint32_t t)
+    m_processor.ForEachCategoryTypes(v2::QuerySlice(slice), [&](size_t i, uint32_t t)
                                      {
                                        ++matched[i].second;
                                        if (holder.Has(t))
diff --git a/search/v2/geocoder.cpp b/search/v2/geocoder.cpp
index 8bc16e49eb..30981bd454 100644
--- a/search/v2/geocoder.cpp
+++ b/search/v2/geocoder.cpp
@@ -1011,7 +1011,8 @@ void Geocoder::WithPostcodes(TFn && fn)
         break;
 
       TokenSlice slice(m_params, startToken, startToken + n);
-      if (LooksLikePostcode(QuerySliceOnTokens(slice), true /*handleAsPrefix*/))
+      auto const isPrefix = startToken + n == m_numTokens;
+      if (LooksLikePostcode(QuerySlice(slice), isPrefix))
         endToken = startToken + n;
     }
     if (startToken == endToken)
diff --git a/search/v2/token_slice.hpp b/search/v2/token_slice.hpp
index d6cc2bc02d..725ea98355 100644
--- a/search/v2/token_slice.hpp
+++ b/search/v2/token_slice.hpp
@@ -69,10 +69,10 @@ private:
   vector<size_t> m_indexes;
 };
 
-class QuerySliceOnTokens : public StringSliceBase
+class QuerySlice : public StringSliceBase
 {
 public:
-  QuerySliceOnTokens(TokenSlice const & slice) : m_slice(slice) {}
+  QuerySlice(TokenSlice const & slice) : m_slice(slice) {}
 
   // QuerySlice overrides:
   QueryParams::TString const & Get(size_t i) const override { return m_slice.Get(i).front(); }