From 05a4fcfb5e37a269c0beeb374072a30774c87285 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sat, 11 Dec 2021 20:18:56 +0300 Subject: [PATCH] [storage] Use std::move semantics. Signed-off-by: Viktor Govako --- platform/country_file.cpp | 12 +++------ platform/country_file.hpp | 10 +++----- platform/local_country_file_utils.cpp | 2 ++ .../platform_tests/country_file_tests.cpp | 24 +++++++++++------- .../local_country_file_tests.cpp | 7 ++---- storage/country.hpp | 6 ++--- storage/country_tree.cpp | 25 ++++++++----------- storage/country_tree.hpp | 8 +++--- 8 files changed, 42 insertions(+), 52 deletions(-) diff --git a/platform/country_file.cpp b/platform/country_file.cpp index e68777cb79..6ff6e77eab 100644 --- a/platform/country_file.cpp +++ b/platform/country_file.cpp @@ -32,18 +32,14 @@ namespace platform { CountryFile::CountryFile() : m_mapSize(0) {} -CountryFile::CountryFile(string name) : m_name(std::move(name)), m_mapSize(0) {} - -string const & CountryFile::GetName() const { return m_name; } - -void CountryFile::SetRemoteSize(MwmSize mapSize) +CountryFile::CountryFile(std::string name) +: m_name(std::move(name)), m_mapSize(0) { - m_mapSize = mapSize; } -MwmSize CountryFile::GetRemoteSize() const +CountryFile::CountryFile(std::string name, MwmSize size, std::string sha1) +: m_name(std::move(name)), m_mapSize(size), m_sha1(sha1) { - return m_mapSize; } string GetFileName(string const & countryFile, MapFileType type) diff --git a/platform/country_file.hpp b/platform/country_file.hpp index 5799310bc6..01f0521896 100644 --- a/platform/country_file.hpp +++ b/platform/country_file.hpp @@ -17,18 +17,14 @@ class CountryFile public: CountryFile(); explicit CountryFile(std::string name); + CountryFile(std::string name, MwmSize size, std::string sha1); /// \returns Empty (invalid) CountryFile. bool IsEmpty() const { return m_name.empty(); } /// \returns file name without extensions. - std::string const & GetName() const; - - /// \note Remote size is size of mwm in bytes. - void SetRemoteSize(MwmSize mapSize); - MwmSize GetRemoteSize() const; - - void SetSha1(std::string const & base64Sha1) { m_sha1 = base64Sha1; } + std::string const & GetName() const { return m_name; } + MwmSize GetRemoteSize() const { return m_mapSize; } std::string const & GetSha1() const { return m_sha1; } inline bool operator<(const CountryFile & rhs) const { return m_name < rhs.m_name; } diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp index 32cf860620..0d57d87341 100644 --- a/platform/local_country_file_utils.cpp +++ b/platform/local_country_file_utils.cpp @@ -222,6 +222,8 @@ void FindAllLocalMapsAndCleanup(int64_t latestVersion, string const & dataDir, vector & localFiles) { string const dir = GetDataDirFullPath(dataDir); + + /// @todo Should we search for files in root data folder, except minsk-pass tests? FindAllLocalMapsInDirectoryAndCleanup(dir, 0 /* version */, latestVersion, localFiles); Platform::TFilesWithType fwts; diff --git a/platform/platform_tests/country_file_tests.cpp b/platform/platform_tests/country_file_tests.cpp index 0e0a009120..f3b313b491 100644 --- a/platform/platform_tests/country_file_tests.cpp +++ b/platform/platform_tests/country_file_tests.cpp @@ -13,17 +13,23 @@ namespace platform { UNIT_TEST(CountryFile_Smoke) { - CountryFile countryFile("TestCountryOneComponent"); - TEST_EQUAL("TestCountryOneComponent", countryFile.GetName(), ()); - string const mapFileName = - GetFileName(countryFile.GetName(), MapFileType::Map); + { + CountryFile cf("One"); + TEST_EQUAL("One", cf.GetName(), ()); + string const mapFileName = GetFileName(cf.GetName(), MapFileType::Map); - TEST_EQUAL("TestCountryOneComponent" DATA_FILE_EXTENSION, mapFileName, ()); + TEST_EQUAL("One" DATA_FILE_EXTENSION, mapFileName, ()); + TEST_EQUAL(0, cf.GetRemoteSize(), ()); + } - TEST_EQUAL(0, countryFile.GetRemoteSize(), ()); + { + CountryFile cf("Three", 666, "xxxSHAxxx"); + TEST_EQUAL("Three", cf.GetName(), ()); + string const mapFileName = GetFileName(cf.GetName(), MapFileType::Map); - countryFile.SetRemoteSize(3 /* mapSize */); - - TEST_EQUAL(3, countryFile.GetRemoteSize(), ()); + TEST_EQUAL("Three" DATA_FILE_EXTENSION, mapFileName, ()); + TEST_EQUAL(666, cf.GetRemoteSize(), ()); + TEST_EQUAL("xxxSHAxxx", cf.GetSha1(), ()); + } } } // namespace platform diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp index 58e5ad4c68..f0d58df4ef 100644 --- a/platform/platform_tests/local_country_file_tests.cpp +++ b/platform/platform_tests/local_country_file_tests.cpp @@ -59,9 +59,7 @@ UNIT_TEST(LocalCountryFile_ParseVersion) // Checks basic functionality of LocalCountryFile. UNIT_TEST(LocalCountryFile_Smoke) { - CountryFile countryFile("TestCountry"); - countryFile.SetRemoteSize(1 /* mapSize */); - + CountryFile countryFile("TestCountry", 1 /* size */, "sha1"); LocalCountryFile localFile("/test-dir", countryFile, 150309); TEST_EQUAL("/test-dir/TestCountry" DATA_FILE_EXTENSION, localFile.GetPath(MapFileType::Map), ()); @@ -85,8 +83,7 @@ UNIT_TEST(LocalCountryFile_DiskFiles) { Platform & platform = GetPlatform(); - CountryFile countryFile("TestCountry"); - countryFile.SetRemoteSize(1 /* mapSize */); + CountryFile countryFile("TestCountry", 1 /* size */, "sha1"); for (int64_t version : {1, 150312}) { diff --git a/storage/country.hpp b/storage/country.hpp index d59c933baf..3e832b1351 100644 --- a/storage/country.hpp +++ b/storage/country.hpp @@ -38,7 +38,7 @@ public: { } - void SetFile(platform::CountryFile const & file) { m_file = file; } + void SetFile(platform::CountryFile && file) { m_file = std::move(file); } void SetSubtreeAttrs(MwmCounter subtreeMwmNumber, MwmSize subtreeMwmSizeBytes) { m_subtreeMwmNumber = subtreeMwmNumber; @@ -46,12 +46,10 @@ public: } MwmCounter GetSubtreeMwmCounter() const { return m_subtreeMwmNumber; } MwmSize GetSubtreeMwmSizeBytes() const { return m_subtreeMwmSizeBytes; } - CountryId GetParent() const { return m_parent; } - /// This function valid for current logic - one file for one country (region). - /// If the logic will be changed, replace GetFile with ForEachFile. platform::CountryFile const & GetFile() const { return m_file; } CountryId const & Name() const { return m_name; } + CountryId const & GetParent() const { return m_parent; } private: /// Name in the country node tree. In single mwm case it's a country id. diff --git a/storage/country_tree.cpp b/storage/country_tree.cpp index d9b5e196d3..556ed1f6cd 100644 --- a/storage/country_tree.cpp +++ b/storage/country_tree.cpp @@ -77,13 +77,8 @@ public: { Country country(id, parent); if (mapSize) - { - CountryFile countryFile(id); - countryFile.SetRemoteSize(mapSize); - countryFile.SetSha1(mapSha1); - country.SetFile(countryFile); - } - return &m_countries.AddAtDepth(depth, country); + country.SetFile(CountryFile{id, mapSize, mapSha1}); + return &m_countries.AddAtDepth(depth, std::move(country)); } void InsertOldMwmMapping(CountryId const & newId, CountryId const & oldId) override @@ -171,13 +166,13 @@ public: // CountryTree::Node ------------------------------------------------------------------------------- -CountryTree::Node * CountryTree::Node::AddAtDepth(size_t level, Country const & value) +CountryTree::Node * CountryTree::Node::AddAtDepth(size_t level, Country && value) { Node * node = this; while (--level > 0 && !node->m_children.empty()) node = node->m_children.back().get(); ASSERT_EQUAL(level, 0, ()); - return node->Add(value); + return node->Add(std::move(value)); } CountryTree::Node const & CountryTree::Node::Parent() const @@ -253,9 +248,9 @@ void CountryTree::Node::ForEachAncestorExceptForTheRoot( m_parent->ForEachAncestorExceptForTheRoot(f); } -CountryTree::Node * CountryTree::Node::Add(Country const & value) +CountryTree::Node * CountryTree::Node::Add(Country && value) { - m_children.emplace_back(std::make_unique(value, this)); + m_children.emplace_back(std::make_unique(std::move(value), this)); return m_children.back().get(); } @@ -273,22 +268,22 @@ CountryTree::Node & CountryTree::GetRoot() return *m_countryTree; } -Country & CountryTree::AddAtDepth(size_t level, Country const & value) +Country & CountryTree::AddAtDepth(size_t level, Country && value) { Node * added = nullptr; if (level == 0) { ASSERT(IsEmpty(), ()); - m_countryTree = std::make_unique(value, nullptr); // Creating the root node. + m_countryTree = std::make_unique(std::move(value), nullptr); // Creating the root node. added = m_countryTree.get(); } else { - added = m_countryTree->AddAtDepth(level, value); + added = m_countryTree->AddAtDepth(level, std::move(value)); } ASSERT(added, ()); - m_countryTreeMap.insert(make_pair(value.Name(), added)); + m_countryTreeMap.insert(make_pair(added->Value().Name(), added)); return added->Value(); } diff --git a/storage/country_tree.hpp b/storage/country_tree.hpp index 9e6c91a42a..87e43b8db6 100644 --- a/storage/country_tree.hpp +++ b/storage/country_tree.hpp @@ -29,7 +29,7 @@ public: public: using NodeCallback = std::function; - Node(Country const & value, Node * parent) : m_value(value), m_parent(parent) {} + Node(Country && value, Node * parent) : m_value(std::move(value)), m_parent(parent) {} Country const & Value() const { return m_value; } Country & Value() { return m_value; } @@ -43,7 +43,7 @@ public: /// \param value is a value of node which will be added. /// \note This method does not let to add a node to an arbitrary place in the tree. /// It's posible to add children only from "right side". - Node * AddAtDepth(size_t level, Country const & value); + Node * AddAtDepth(size_t level, Country && value); bool HasParent() const { return m_parent != nullptr; } @@ -74,7 +74,7 @@ public: void ForEachAncestorExceptForTheRoot(NodeCallback const & f) const; private: - Node * Add(Country const & value); + Node * Add(Country && value); Country m_value; @@ -91,7 +91,7 @@ public: Node & GetRoot(); - Country & AddAtDepth(size_t level, Country const & value); + Country & AddAtDepth(size_t level, Country && value); /// Deletes all children and makes tree empty void Clear();