review fixes

10 files changed, 48 insertions, 27 deletions

diff --git a/base/base_tests/logging_test.cpp b/base/base_tests/logging_test.cpp
index ff5e50ad16..365db44668 100644
--- a/base/base_tests/logging_test.cpp
+++ b/base/base_tests/logging_test.cpp
@@ -18,6 +18,13 @@ namespace
     g_SomeFunctionCalled = true;
     return 3;
   }
+
+  bool g_NegativeFunctionCalled;
+  bool NegativeFunction()
+  {
+    g_NegativeFunctionCalled = true;
+    return false;
+  }
 }
 
 UNIT_TEST(Logging_Level)
@@ -43,3 +50,14 @@ UNIT_TEST(NullMessage)
   char const * ptr = 0;
   LOG(LINFO, ("Null message test", ptr));
 }
+
+UNIT_TEST(Logging_ConditionalLog)
+{
+  g_SomeFunctionCalled = false;
+  CLOG(LINFO, SomeFunction(), ("This should not pass"));
+  TEST(g_SomeFunctionCalled, ());
+
+  g_NegativeFunctionCalled = false;
+  CLOG(LWARNING, NegativeFunction(), ("This should pass"));
+  TEST(g_NegativeFunctionCalled, ());
+}
diff --git a/base/logging.hpp b/base/logging.hpp
index 163041e0fb..147958938a 100644
--- a/base/logging.hpp
+++ b/base/logging.hpp
@@ -87,3 +87,11 @@ using ::my::LCRITICAL;
     if ((level) >= ::my::g_LogLevel)                                    \
       ::my::LogMessage(level, my::SrcPoint(), ::my::impl::Message msg); \
   } while (false)
+
+// Conditional log. Logs @msg with level @level in case when @X returns false.
+#define CLOG(level, X, msg)                                         \
+  do                                                                \
+  {                                                                 \
+    if (!X)                                                         \
+      LOG(level, (SRC(), "CLOG(" #X ")", ::my::impl::Message msg)); \
+  } while (false)
diff --git a/generator/booking_dataset.cpp b/generator/booking_dataset.cpp
index 7cb0a65dad..5cacf1869e 100644
--- a/generator/booking_dataset.cpp
+++ b/generator/booking_dataset.cpp
@@ -6,6 +6,7 @@
 #include "indexer/classificator.hpp"
 #include "indexer/ftypes_matcher.hpp"
 
+#include "base/logging.hpp"
 #include "base/string_utils.hpp"
 
 #include <iomanip>
@@ -22,22 +23,22 @@ BookingHotel::BookingHotel(std::string const & src)
   CHECK_EQUAL(rec.size(), FieldsCount(), ("Error parsing hotels.tsv line:",
                                           boost::replace_all_copy(src, "\t", "\\t")));
 
-  CHECK(strings::to_uint(rec[FieldIndex(Fields::Id)], m_id.Get()), ());
+  CLOG(LERROR, strings::to_uint(rec[FieldIndex(Fields::Id)], m_id.Get()), ());
   // TODO(mgsergio): Use ms::LatLon.
-  CHECK(strings::to_double(rec[FieldIndex(Fields::Latitude)], m_latLon.lat), ());
-  CHECK(strings::to_double(rec[FieldIndex(Fields::Longtitude)], m_latLon.lon), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(Fields::Latitude)], m_latLon.lat), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(Fields::Longtitude)], m_latLon.lon), ());
 
   m_name = rec[FieldIndex(Fields::Name)];
   m_address = rec[FieldIndex(Fields::Address)];
 
-  CHECK(strings::to_uint(rec[FieldIndex(Fields::Stars)], m_stars), ());
-  CHECK(strings::to_uint(rec[FieldIndex(Fields::PriceCategory)], m_priceCategory), ());
-  CHECK(strings::to_double(rec[FieldIndex(Fields::RatingBooking)], m_ratingBooking), ());
-  CHECK(strings::to_double(rec[FieldIndex(Fields::RatingUsers)], m_ratingUser), ());
+  CLOG(LERROR, strings::to_uint(rec[FieldIndex(Fields::Stars)], m_stars), ());
+  CLOG(LERROR, strings::to_uint(rec[FieldIndex(Fields::PriceCategory)], m_priceCategory), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(Fields::RatingBooking)], m_ratingBooking), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(Fields::RatingUsers)], m_ratingUser), ());
 
   m_descUrl = rec[FieldIndex(Fields::DescUrl)];
 
-  CHECK(strings::to_uint(rec[FieldIndex(Fields::Type)], m_type), ());
+  CLOG(LERROR, strings::to_uint(rec[FieldIndex(Fields::Type)], m_type), ());
 
   m_translations = rec[FieldIndex(Fields::Translations)];
 }
diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp
index 8e6599fed2..3b15cbfbc7 100644
--- a/generator/generator_tool/generator_tool.cpp
+++ b/generator/generator_tool/generator_tool.cpp
@@ -87,7 +87,6 @@ DEFINE_string(booking_reference_path, "", "Path to mwm dataset for booking addre
 DEFINE_string(opentable_data, "", "Path to opentable data in .tsv format.");
 DEFINE_string(opentable_reference_path, "", "Path to mwm dataset for opentable addresses matching.");
 DEFINE_string(viator_data, "", "Path to viator data in .tsv format.");
-DEFINE_string(viator_reference_path, "", "Path to mwm dataset for viator cities matching.");
 
 // Printing stuff.
 DEFINE_bool(calc_statistics, false, "Calculate feature statistics for specified mwm bucket files.");
diff --git a/generator/opentable_dataset.cpp b/generator/opentable_dataset.cpp
index fff18b1816..01c55399e1 100644
--- a/generator/opentable_dataset.cpp
+++ b/generator/opentable_dataset.cpp
@@ -6,6 +6,7 @@
 #include "indexer/classificator.hpp"
 #include "indexer/ftypes_matcher.hpp"
 
+#include "base/logging.hpp"
 #include "base/string_utils.hpp"
 
 #include <iomanip>
@@ -22,9 +23,9 @@ OpentableRestaurant::OpentableRestaurant(std::string const & src)
   CHECK_EQUAL(rec.size(), FieldsCount(), ("Error parsing restaurants.tsv line:",
                                           boost::replace_all_copy(src, "\t", "\\t")));
 
-  CHECK(strings::to_uint(rec[FieldIndex(Fields::Id)], m_id.Get()), ());
-  CHECK(strings::to_double(rec[FieldIndex(Fields::Latitude)], m_latLon.lat), ());
-  CHECK(strings::to_double(rec[FieldIndex(Fields::Longtitude)], m_latLon.lon), ());
+  CLOG(LERROR, strings::to_uint(rec[FieldIndex(Fields::Id)], m_id.Get()), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(Fields::Latitude)], m_latLon.lat), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(Fields::Longtitude)], m_latLon.lon), ());
 
   m_name = rec[FieldIndex(Fields::Name)];
   m_address = rec[FieldIndex(Fields::Address)];
diff --git a/generator/sponsored_dataset.hpp b/generator/sponsored_dataset.hpp
index 701bf2666c..6ae9b8cfe3 100644
--- a/generator/sponsored_dataset.hpp
+++ b/generator/sponsored_dataset.hpp
@@ -5,7 +5,6 @@
 #include "base/newtype.hpp"
 
 #include <functional>
-#include <iostream>
 #include <string>
 
 class FeatureBuilder1;
diff --git a/generator/sponsored_dataset_inl.hpp b/generator/sponsored_dataset_inl.hpp
index f829853772..2380a12892 100644
--- a/generator/sponsored_dataset_inl.hpp
+++ b/generator/sponsored_dataset_inl.hpp
@@ -11,8 +11,6 @@
 #include "base/stl_add.hpp"
 #include "base/string_utils.hpp"
 
-#include <iostream>
-
 namespace generator
 {
 class AddressMatcher
diff --git a/generator/utils.cpp b/generator/utils.cpp
index 9981ad8b10..d97d27c8c7 100644
--- a/generator/utils.cpp
+++ b/generator/utils.cpp
@@ -13,7 +13,7 @@ void LoadIndex(Index & index)
   Platform & platform = GetPlatform();
   platform::FindAllLocalMapsInDirectoryAndCleanup(platform.WritableDir(), 0 /* version */,
                                                   -1 /* latestVersion */, localFiles);
-  for (platform::LocalCountryFile const & localFile : localFiles)
+  for (auto const & localFile : localFiles)
   {
     LOG(LINFO, ("Found mwm:", localFile));
     try
diff --git a/generator/viator_dataset.cpp b/generator/viator_dataset.cpp
index 96230a71de..dc88fd093c 100644
--- a/generator/viator_dataset.cpp
+++ b/generator/viator_dataset.cpp
@@ -38,9 +38,9 @@ ViatorCity::ViatorCity(std::string const & src)
   CHECK_EQUAL(rec.size(), FieldsCount(),
               ("Error parsing viator cities, line:", boost::replace_all_copy(src, "\t", "\\t")));
 
-  CHECK(strings::to_uint(rec[FieldIndex(TsvFields::Id)], m_id.Get()), ());
-  CHECK(strings::to_double(rec[FieldIndex(TsvFields::Latitude)], m_latLon.lat), ());
-  CHECK(strings::to_double(rec[FieldIndex(TsvFields::Longtitude)], m_latLon.lon), ());
+  CLOG(LERROR, strings::to_uint(rec[FieldIndex(TsvFields::Id)], m_id.Get()), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(TsvFields::Latitude)], m_latLon.lat), ());
+  CLOG(LERROR, strings::to_double(rec[FieldIndex(TsvFields::Longtitude)], m_latLon.lon), ());
 
   m_name = rec[FieldIndex(TsvFields::Name)];
 }
@@ -83,10 +83,9 @@ ViatorCity::ObjectId ViatorDataset::FindMatchingObjectId(FeatureBuilder1 const &
       return objId;
   }
 
-  if (!nearbyIds.empty())
-    LOG(LWARNING, ("Viator city matching failed! "
-                   "OSM city:", name, "OSM point:", fb.GetKeyPoint(),
-                   "Viator cities:", nearbyIds));
+  CLOG(LWARNING, nearbyIds.empty(),
+       ("Viator city matching failed! OSM city:", name, "OSM point:", fb.GetKeyPoint(),
+        "Viator cities:", nearbyIds));
 
   return ViatorCity::InvalidObjectId();
 }
diff --git a/tools/python/viator_cities.py b/tools/python/viator_cities.py
index c6818f1cef..10dc489420 100644
--- a/tools/python/viator_cities.py
+++ b/tools/python/viator_cities.py
@@ -14,11 +14,9 @@ class ViatorApi(object):
         self.apikey = apikey
 
     def get_locations(self):
-        url = 'http://viatorapi.viator.com/service/taxonomy/locations?apiKey=' + self.apikey
+        url = 'https://viatorapi.viator.com/service/taxonomy/locations?apiKey=' + self.apikey
         stream = urllib2.urlopen(url)
-        payload = stream.read()
-        locations = json.loads(payload)
-        return locations
+        return json.load(stream)
 
 
 def check_errors(locations):