Review fixes.

5 files changed, 105 insertions, 124 deletions

diff --git a/generator/altitude_generator.cpp b/generator/altitude_generator.cpp
index 2a282ece0d..c8eb5e2d81 100644
--- a/generator/altitude_generator.cpp
+++ b/generator/altitude_generator.cpp
@@ -15,11 +15,10 @@
 #include "coding/reader.hpp"
 #include "coding/succinct_mapper.hpp"
 #include "coding/varint.hpp"
+#include "coding/internal/file_data.hpp"
 
 #include "geometry/latlon.hpp"
 
-#include "coding/internal/file_data.hpp"
-
 #include "base/assert.hpp"
 #include "base/logging.hpp"
 #include "base/scope_guard.hpp"
@@ -43,14 +42,12 @@ namespace
 {
 using namespace routing;
 
-AltitudeHeader::TAltitudeSectionVersion constexpr kAltitudeSectionVersion = 0;
-
-class SrtmGetter : public IAltitudeGetter
+class SrtmGetter : public AltitudeGetter
 {
 public:
-  SrtmGetter(string const & srtmPath) : m_srtmManager(srtmPath) {}
+  SrtmGetter(string const & srtmDir) : m_srtmManager(srtmDir) {}
 
-  // IAltitudeGetter overrides:
+  // AltitudeGetter overrides:
   feature::TAltitude GetAltitude(m2::PointD const & p) override
   {
     return m_srtmManager.GetHeight(MercatorBounds::ToLatLon(p));
@@ -63,10 +60,21 @@ private:
 class Processor
 {
 public:
-  using TFeatureAltitude = pair<uint32_t, Altitude>;
-  using TFeatureAltitudes = vector<TFeatureAltitude>;
+  struct FeatureAltitude
+  {
+    FeatureAltitude() : m_featureId(0) {}
+    FeatureAltitude(uint32_t featureId, Altitudes const & altitudes)
+      : m_featureId(featureId), m_altitudes(altitudes)
+    {
+    }
+
+    uint32_t m_featureId;
+    Altitudes m_altitudes;
+  };
 
-  Processor(IAltitudeGetter & altitudeGetter)
+  using TFeatureAltitudes = vector<FeatureAltitude>;
+
+  Processor(AltitudeGetter & altitudeGetter)
     : m_altitudeGetter(altitudeGetter), m_minAltitude(kInvalidAltitude)
   {
   }
@@ -89,7 +97,7 @@ public:
     }
 
     bool hasAltitude = false;
-    MY_SCOPE_GUARD(removeTmpDir, [&] ()
+    MY_SCOPE_GUARD(altitudeAvailabilityBuilding, [&] ()
     {
       m_altitudeAvailabilityBuilder.push_back(hasAltitude);
     });
@@ -103,16 +111,12 @@ public:
       return;
 
     TAltitudes altitudes;
-    bool valid = true;
     TAltitude minFeatureAltitude = kInvalidAltitude;
     for (size_t i = 0; i < pointsCount; ++i)
     {
       TAltitude const a = m_altitudeGetter.GetAltitude(f.GetPoint(i));
       if (a == kInvalidAltitude)
-      {
-        valid = false;
         return;
-      }
 
       if (minFeatureAltitude == kInvalidAltitude)
         minFeatureAltitude = a;
@@ -121,11 +125,9 @@ public:
 
       altitudes.push_back(a);
     }
-    if (!valid)
-      return;
 
     hasAltitude = true;
-    m_featureAltitudes.push_back(make_pair(id, Altitude(move(altitudes))));
+    m_featureAltitudes.emplace_back(id, Altitudes(move(altitudes)));
 
     if (m_minAltitude == kInvalidAltitude)
       m_minAltitude = minFeatureAltitude;
@@ -141,22 +143,20 @@ public:
   bool IsFeatureAltitudesSorted()
   {
     return is_sorted(m_featureAltitudes.begin(), m_featureAltitudes.end(),
-                     my::LessBy(&Processor::TFeatureAltitude::first));
+                     my::LessBy(&Processor::FeatureAltitude::m_featureId));
   }
 
 private:
-  IAltitudeGetter & m_altitudeGetter;
+  AltitudeGetter & m_altitudeGetter;
   TFeatureAltitudes m_featureAltitudes;
   succinct::bit_vector_builder m_altitudeAvailabilityBuilder;
   TAltitude m_minAltitude;
 };
 }  // namespace
 
-static_assert(sizeof(AltitudeHeader) == 16, "Wrong header size of altitude section.");
-
 namespace routing
 {
-void BuildRoadAltitudes(string const & mwmPath, IAltitudeGetter & altitudeGetter)
+void BuildRoadAltitudes(string const & mwmPath, AltitudeGetter & altitudeGetter)
 {
   try
   {
@@ -174,8 +174,10 @@ void BuildRoadAltitudes(string const & mwmPath, IAltitudeGetter & altitudeGetter
 
     FilesContainerW cont(mwmPath, FileWriter::OP_WRITE_EXISTING);
     FileWriter w = cont.GetWriter(ALTITUDES_FILE_TAG);
+
     AltitudeHeader header;
-    header.minAltitude = processor.GetMinAltitude();
+    header.m_minAltitude = processor.GetMinAltitude();
+
     int64_t const startOffset = w.Pos();
     header.Serialize(w);
     {
@@ -184,7 +186,7 @@ void BuildRoadAltitudes(string const & mwmPath, IAltitudeGetter & altitudeGetter
       succinct::bit_vector_builder & builder = processor.GetAltitudeAvailabilityBuilder();
       succinct::rs_bit_vector(&builder).map(visitor);
     }
-    header.featureTableOffset = w.Pos() - startOffset;
+    header.m_featureTableOffset = w.Pos() - startOffset;
 
     vector<uint32_t> offsets;
     vector<uint8_t> deltas;
@@ -195,7 +197,7 @@ void BuildRoadAltitudes(string const & mwmPath, IAltitudeGetter & altitudeGetter
       for (auto const & a : featureAltitudes)
       {
         offsets.push_back(writer.Pos());
-        a.second.Serialize(header.minAltitude, writer);
+        a.m_altitudes.Serialize(header.m_minAltitude, writer);
       }
     }
     {
@@ -211,27 +213,27 @@ void BuildRoadAltitudes(string const & mwmPath, IAltitudeGetter & altitudeGetter
       succinct::elias_fano(&builder).map(visitor);
     }
     // Writing altitude info.
-    header.altitudeInfoOffset = w.Pos() - startOffset;
+    header.m_altitudesOffset = w.Pos() - startOffset;
     w.Write(deltas.data(), deltas.size());
-    header.endOffset = w.Pos() - startOffset;
+    header.m_endOffset = w.Pos() - startOffset;
 
     // Rewriting header info.
-    header.version = kAltitudeSectionVersion;
     int64_t const endOffset = w.Pos();
     w.Seek(startOffset);
     header.Serialize(w);
     w.Seek(endOffset);
+    LOG(LINFO, (ALTITUDES_FILE_TAG, "section is ready. The size is", endOffset - startOffset));
   }
   catch (RootException const & e)
   {
-    LOG(LERROR, ("An exception happened while creating", ALTITUDES_FILE_TAG, "section. ", e.what()));
+    LOG(LERROR, ("An exception happened while creating", ALTITUDES_FILE_TAG, "section:", e.what()));
   }
 }
 
-void BuildRoadAltitudes(string const & mwmPath, string const & srtmPath)
+void BuildRoadAltitudes(string const & mwmPath, string const & srtmDir)
 {
-  LOG(LINFO, ("mwmPath =", mwmPath, "srtmPath =", srtmPath));
-  SrtmGetter srtmGetter(srtmPath);
+  LOG(LINFO, ("mwmPath =", mwmPath, "srtmDir =", srtmDir));
+  SrtmGetter srtmGetter(srtmDir);
   BuildRoadAltitudes(mwmPath, srtmGetter);
 }
 }  // namespace routing
diff --git a/generator/altitude_generator.hpp b/generator/altitude_generator.hpp
index 2dbd7cf147..6cecdd99e2 100644
--- a/generator/altitude_generator.hpp
+++ b/generator/altitude_generator.hpp
@@ -8,7 +8,7 @@
 
 namespace routing
 {
-class IAltitudeGetter
+class AltitudeGetter
 {
 public:
   virtual feature::TAltitude GetAltitude(m2::PointD const & p) = 0;
@@ -22,8 +22,8 @@ public:
 /// 8                   altitude info offset  4
 /// 12                  end of section        4
 /// 16                  altitude availability feat. table offset - 16
-/// feat. table offset  feature table         alt. info offset - f. table offset
-/// alt. info offset    altitude info         alt. info offset - end of section
-void BuildRoadAltitudes(string const & mwmPath, IAltitudeGetter & altitudeGetter);
-void BuildRoadAltitudes(string const & mwmPath, string const & srtmPath);
+/// feat. table offset  feature table         alt. info offset - feat. table offset
+/// alt. info offset    altitude info         end of section - alt. info offset
+void BuildRoadAltitudes(string const & mwmPath, AltitudeGetter & altitudeGetter);
+void BuildRoadAltitudes(string const & mwmPath, string const & srtmDir);
 }  // namespace routing
diff --git a/generator/generator_tests/altitude_test.cpp b/generator/generator_tests/altitude_test.cpp
index a6f407ef34..935794630e 100644
--- a/generator/generator_tests/altitude_test.cpp
+++ b/generator/generator_tests/altitude_test.cpp
@@ -1,7 +1,6 @@
 #include "testing/testing.hpp"
 
 #include "generator/altitude_generator.hpp"
-
 #include "generator/generator_tests_support/test_feature.hpp"
 #include "generator/generator_tests_support/test_mwm_builder.hpp"
 
@@ -19,7 +18,8 @@
 
 #include "platform/country_file.hpp"
 #include "platform/platform.hpp"
-
+#include "platform/platform_tests_support/scoped_dir.hpp"
+#include "platform/platform_tests_support/scoped_file.hpp"
 #include "platform/platform_tests_support/writable_dir_changer.hpp"
 
 #include "base/logging.hpp"
@@ -31,12 +31,11 @@ using namespace feature;
 using namespace generator;
 using namespace platform;
 using namespace routing;
-using namespace tests_support;
 
 namespace
 {
 // These tests generate mwms with altitude sections and then check if altitudes
-// in the mwms are correct. The mwms are inited with different sets of features defined below.
+// in the mwms are correct. The mwms are initialized with different sets of features.
 // There are several restrictions for these features:
 // * all of them are linear
 // * all coords of these features should be integer (it's necessary for easy implementation of MockAltitudeGetter)
@@ -49,29 +48,29 @@ string const kTestDir = "altitude_generation_test";
 // Temporary mwm name for testing.
 string const kTestMwm = "test";
 
-struct Rounded3DPoint
+struct Point3D
 {
-  Rounded3DPoint(int32_t x, int32_t y, TAltitude a) : point2D(x, y), altitude(a) {}
+  Point3D(int32_t x, int32_t y, TAltitude a) : m_point(x, y), m_altitude(a) {}
 
-  m2::PointI point2D;
-  TAltitude altitude;
+  m2::PointI m_point;
+  TAltitude m_altitude;
 };
 
-using TRounded3DGeom = vector<Rounded3DPoint>;
+using TPoint3DList = vector<Point3D>;
 
-TRounded3DGeom const kRoad1 = {{0, -1, -1}, {0, 0, 0}, {0, 1, 1}};
-TRounded3DGeom const kRoad2 = {{0, 1, 1}, {5, 1, 1}, {10, 1, 1}};
-TRounded3DGeom const kRoad3 = {{10, 1, 1}, {15, 6, 100}, {20, 11, 110}};
-TRounded3DGeom const kRoad4 = {{-10, 1, -1}, {-20, 6, -100}, {-20, -11, -110}};
+TPoint3DList const kRoad1 = {{0, -1, -1}, {0, 0, 0}, {0, 1, 1}};
+TPoint3DList const kRoad2 = {{0, 1, 1}, {5, 1, 1}, {10, 1, 1}};
+TPoint3DList const kRoad3 = {{10, 1, 1}, {15, 6, 100}, {20, 11, 110}};
+TPoint3DList const kRoad4 = {{-10, 1, -1}, {-20, 6, -100}, {-20, -11, -110}};
 
-class MockAltitudeGetter : public IAltitudeGetter
+class MockAltitudeGetter : public AltitudeGetter
 {
 public:
   using TMockAltitudes = map<m2::PointI, TAltitude>;
 
   MockAltitudeGetter(TMockAltitudes && altitudes) : m_altitudes(altitudes) {}
 
-  // IAltitudeGetter overrides:
+  // AltitudeGetter overrides:
   TAltitude GetAltitude(m2::PointD const & p) override
   {
     m2::PointI const rounded(static_cast<int32_t>(round(p.x)), static_cast<int32_t>(round(p.y)));
@@ -86,57 +85,43 @@ private:
   TMockAltitudes const & m_altitudes;
 };
 
-template<class T>
-vector<T> ConvertTo2DGeom(TRounded3DGeom const & geom3D)
-{
-  vector<T> dv;
-  dv.clear();
-  for (Rounded3DPoint const & p : geom3D)
-    dv.push_back(T(p.point2D));
-  return dv;
-}
-
-TAltitudes ConvertToAltitudes(TRounded3DGeom const & geom3D)
+vector<m2::PointD> ExtractPoints(TPoint3DList const & geom3D)
 {
-  TAltitudes altitudes;
-  altitudes.clear();
-  for (Rounded3DPoint const & p : geom3D)
-    altitudes.push_back(p.altitude);
-  return altitudes;
+  vector<m2::PointD> result;
+  for (Point3D const & p : geom3D)
+    result.push_back(m2::PointD(p.m_point));
+  return result;
 }
 
-void FillAltitudes(vector<TRounded3DGeom> const & roadFeatures,
+void FillAltitudes(vector<TPoint3DList> const & roads,
                    MockAltitudeGetter::TMockAltitudes & altitudes)
 {
-  for (TRounded3DGeom const & geom3D : roadFeatures)
+  for (TPoint3DList const & geom3D : roads)
   {
-    vector<m2::PointI> const featureGeom = ConvertTo2DGeom<m2::PointI>(geom3D);
-    TAltitudes const featureAlt = ConvertToAltitudes(geom3D);
-    CHECK_EQUAL(featureGeom.size(), featureAlt.size(), ());
-    for (size_t i = 0; i < featureGeom.size(); ++i)
+    for (size_t i = 0; i < geom3D.size(); ++i)
     {
-      auto it = altitudes.find(featureGeom[i]);
+      auto it = altitudes.find(geom3D[i].m_point);
       if (it != altitudes.end())
       {
-        CHECK_EQUAL(it->second, featureAlt[i], ("Point", it->first, "is set with two different altitudes."));
+        CHECK_EQUAL(it->second, geom3D[i].m_altitude, ("Point", it->first, "is set with two different altitudes."));
         continue;
       }
-      altitudes[featureGeom[i]] = featureAlt[i];
+      altitudes[geom3D[i].m_point] = geom3D[i].m_altitude;
     }
   }
 }
 
-void BuildMwmWithoutAltitude(vector<TRounded3DGeom> const & roadFeatures, LocalCountryFile & country)
+void BuildMwmWithoutAltitudes(vector<TPoint3DList> const & roads, LocalCountryFile & country)
 {
-  TestMwmBuilder builder(country, feature::DataHeader::country);
+  generator::tests_support::TestMwmBuilder builder(country, feature::DataHeader::country);
 
-  for (TRounded3DGeom const & geom3D : roadFeatures)
-    builder.Add(TestStreet(ConvertTo2DGeom<m2::PointD>(geom3D), string(), string()));
+  for (TPoint3DList const & geom3D : roads)
+    builder.Add(generator::tests_support::TestStreet(ExtractPoints(geom3D), string(), string()));
 }
 
-void ReadAndTestAltitudeInfo(MwmValue const & mwmValue, string const & mwmPath, MockAltitudeGetter & altitudeGetter)
+void TestAltitudes(MwmValue const & mwmValue, string const & mwmPath, MockAltitudeGetter & altitudeGetter)
 {
-  AltitudeLoader const loader(mwmValue);
+  AltitudeLoader loader(mwmValue);
 
   auto processor = [&altitudeGetter, &loader](FeatureType const & f, uint32_t const & id)
   {
@@ -158,75 +143,71 @@ void ReadAndTestAltitudeInfo(MwmValue const & mwmValue, string const & mwmPath,
   feature::ForEachFromDat(mwmPath, processor);
 }
 
-void TestAltitudeSection(vector<TRounded3DGeom> const & roadFeatures)
+void TestAltitudesBuilding(vector<TPoint3DList> const & roads)
 {
   classificator::Load();
   Platform & platform = GetPlatform();
   string const testDirFullPath = my::JoinFoldersToPath(platform.WritableDir(), kTestDir);
 
-  MY_SCOPE_GUARD(removeTmpDir, [&] ()
-  {
-    GetPlatform().RmDirRecursively(testDirFullPath);
-  });
-
   // Building mwm without altitude section.
-  platform.MkDir(testDirFullPath);
   LocalCountryFile country(testDirFullPath, CountryFile(kTestMwm), 1);
-  BuildMwmWithoutAltitude(roadFeatures, country);
+  platform::tests_support::ScopedDir testScopedDir(kTestDir);
+  platform::tests_support::ScopedFile testScopedMwm(country.GetPath(MapOptions::Map));
+  BuildMwmWithoutAltitudes(roads, country);
 
   // Creating MockAltitudeGetter.
   MockAltitudeGetter::TMockAltitudes altitudes;
-  FillAltitudes(roadFeatures, altitudes);
+  FillAltitudes(roads, altitudes);
   MockAltitudeGetter altitudeGetter(move(altitudes));
 
   // Adding altitude section to mwm.
   string const mwmPath = my::JoinFoldersToPath(testDirFullPath, kTestMwm + DATA_FILE_EXTENSION);
   BuildRoadAltitudes(mwmPath, altitudeGetter);
 
-  // Reading from mwm and testing altitue information.
+  // Reading from mwm and testing altitude information.
   Index index;
-  pair<MwmSet::MwmId, MwmSet::RegResult> regResult = index.RegisterMap(country);
+  auto const regResult = index.RegisterMap(country);
   TEST_EQUAL(regResult.second, MwmSet::RegResult::Success, ());
 
   MwmSet::MwmHandle mwmHandle = index.GetMwmHandleById(regResult.first);
-  CHECK(mwmHandle.IsAlive(), ());
+  TEST(mwmHandle.IsAlive(), ());
 
-  ReadAndTestAltitudeInfo(*mwmHandle.GetValue<MwmValue>(), mwmPath, altitudeGetter);
+  TestAltitudes(*mwmHandle.GetValue<MwmValue>(), mwmPath, altitudeGetter);
 }
-} // namespace
 
-UNIT_TEST(ZeroFeatures_AltitudeGenerationTest)
+UNIT_TEST(AltitudeGenerationTest_ZeroFeatures)
 {
-  vector<TRounded3DGeom> const roadFeatures = {};
-  TestAltitudeSection(roadFeatures);
+  vector<TPoint3DList> const roads = {};
+  TestAltitudesBuilding(roads);
 }
 
-UNIT_TEST(OneRoad_AltitudeGenerationTest)
+UNIT_TEST(AltitudeGenerationTest_OneRoad)
 {
-  vector<TRounded3DGeom> const roadFeatures = {kRoad1};
-  TestAltitudeSection(roadFeatures);
+  vector<TPoint3DList> const roads = {kRoad1};
+  TestAltitudesBuilding(roads);
 }
 
-UNIT_TEST(TwoConnectedRoads_AltitudeGenerationTest)
+UNIT_TEST(AltitudeGenerationTest_TwoConnectedRoads)
 {
-  vector<TRounded3DGeom> const roadFeatures = {kRoad1, kRoad2};
-  TestAltitudeSection(roadFeatures);
+  vector<TPoint3DList> const roads = {kRoad1, kRoad2};
+  TestAltitudesBuilding(roads);
 }
 
-UNIT_TEST(TwoDisconnectedRoads_AltitudeGenerationTest)
+UNIT_TEST(AltitudeGenerationTest_TwoDisconnectedRoads)
 {
-  vector<TRounded3DGeom> const roadFeatures = {kRoad1, kRoad3};
-  TestAltitudeSection(roadFeatures);
+  vector<TPoint3DList> const roads = {kRoad1, kRoad3};
+  TestAltitudesBuilding(roads);
 }
 
-UNIT_TEST(ThreeRoads_AltitudeGenerationTest)
+UNIT_TEST(AltitudeGenerationTest_ThreeRoads)
 {
-  vector<TRounded3DGeom> const roadFeatures = {kRoad1, kRoad2, kRoad3};
-  TestAltitudeSection(roadFeatures);
+  vector<TPoint3DList> const roads = {kRoad1, kRoad2, kRoad3};
+  TestAltitudesBuilding(roads);
 }
 
-UNIT_TEST(FourRoads_AltitudeGenerationTest)
+UNIT_TEST(AltitudeGenerationTest_FourRoads)
 {
-  vector<TRounded3DGeom> const roadFeatures = {kRoad1, kRoad2, kRoad3, kRoad4};
-  TestAltitudeSection(roadFeatures);
+  vector<TPoint3DList> const roads = {kRoad1, kRoad2, kRoad3, kRoad4};
+  TestAltitudesBuilding(roads);
 }
+} // namespace
diff --git a/generator/generator_tests/generator_tests.pro b/generator/generator_tests/generator_tests.pro
index b782763808..a3f1a229c1 100644
--- a/generator/generator_tests/generator_tests.pro
+++ b/generator/generator_tests/generator_tests.pro
@@ -4,7 +4,8 @@ CONFIG -= app_bundle
 TEMPLATE = app
 
 ROOT_DIR = ../..
-DEPENDENCIES = generator_tests_support generator drape_frontend routing search storage indexer drape map platform editor geometry \
+DEPENDENCIES = generator_tests_support platform_tests_support generator drape_frontend routing search storage \
+               indexer drape map platform editor geometry \
                coding base freetype expat fribidi tomcrypt jansson protobuf osrm stats_client \
                minizip succinct pugixml tess2 gflags oauthcpp
 
diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp
index 653b8cea5e..a7f372b320 100644
--- a/generator/generator_tool/generator_tool.cpp
+++ b/generator/generator_tool/generator_tool.cpp
@@ -212,9 +212,12 @@ int main(int argc, char ** argv)
       if (!search::RankTableBuilder::CreateIfNotExists(datFile))
         LOG(LCRITICAL, ("Error generating rank table."));
     }
+
+    if (!FLAGS_srtm_path.empty())
+      routing::BuildRoadAltitudes(datFile, FLAGS_srtm_path);
   }
 
-  string const datFile = path + FLAGS_output + DATA_FILE_EXTENSION;
+  string const datFile = my::JoinFoldersToPath(path, FLAGS_output + DATA_FILE_EXTENSION);
 
   if (FLAGS_calc_statistics)
   {
@@ -249,12 +252,6 @@ int main(int argc, char ** argv)
   if (FLAGS_dump_feature_names != "")
     feature::DumpFeatureNames(datFile, FLAGS_dump_feature_names);
 
-  if (!FLAGS_srtm_path.empty())
-  {
-    string const mwmPath = my::JoinFoldersToPath(path, FLAGS_output + DATA_FILE_EXTENSION);
-    routing::BuildRoadAltitudes(FLAGS_srtm_path, mwmPath);
-  }
-
   if (FLAGS_unpack_mwm)
     UnpackMwm(datFile);