diff --git a/geometry/geometry_tests/tree_test.cpp b/geometry/geometry_tests/tree_test.cpp index fab4144e0c..31b4844ba5 100644 --- a/geometry/geometry_tests/tree_test.cpp +++ b/geometry/geometry_tests/tree_test.cpp @@ -8,7 +8,7 @@ namespace typedef m2::RectD R; struct traits_t { m2::RectD LimitRect(m2::RectD const & r) const { return r; }}; - typedef m4::Tree TreeT; + typedef m4::Tree TTree; template bool RTrue(T const &, T const &) { return true; } template bool RFalse(T const &, T const &) { return false; } @@ -16,7 +16,7 @@ namespace UNIT_TEST(Tree4D_Smoke) { - TreeT theTree; + TTree theTree; R arr[] = { R(0, 0, 1, 1), @@ -52,7 +52,7 @@ UNIT_TEST(Tree4D_Smoke) UNIT_TEST(Tree4D_ReplaceAllInRect) { - TreeT theTree; + TTree theTree; R arr[] = { R(8, 13, 554, 32), R(555, 13, 700, 32), @@ -91,7 +91,7 @@ namespace { void CheckInRect(R const * arr, size_t count, R const & searchR, size_t expected) { - TreeT theTree; + TTree theTree; for (size_t i = 0; i < count; ++i) theTree.Add(arr[i], arr[i]); diff --git a/qt/update_dialog.cpp b/qt/update_dialog.cpp index bdee24c990..71d313a7f3 100644 --- a/qt/update_dialog.cpp +++ b/qt/update_dialog.cpp @@ -427,7 +427,7 @@ namespace qt } void UpdateDialog::OnCountryDownloadProgress(TCountryId const & countryId, - pair const & progress) + MapFilesDownloader::TProgress const & progress) { auto const items = GetTreeItemsByCountryId(countryId); for (auto const item : items) diff --git a/qt/update_dialog.hpp b/qt/update_dialog.hpp index 2faee349d2..275c8b5f6b 100644 --- a/qt/update_dialog.hpp +++ b/qt/update_dialog.hpp @@ -33,7 +33,7 @@ namespace qt //@{ void OnCountryChanged(storage::TCountryId const & countryId); void OnCountryDownloadProgress(storage::TCountryId const & countryId, - pair const & progress); + storage::MapFilesDownloader::TProgress const & progress); //@} void ShowModal(); diff --git a/storage/country.cpp b/storage/country.cpp index 9ef2551f6c..45181b5884 100644 --- a/storage/country.cpp +++ b/storage/country.cpp @@ -24,7 +24,6 @@ TMwmSubtreeAttrs LoadGroupSingleMwmsImpl(int depth, json_t * group, TCountryId c uint32_t mwmCounter = 0; size_t mwmSize = 0; size_t const groupListSize = json_array_size(group); - toDo.ReserveAtDepth(depth, groupListSize); for (size_t i = 0; i < groupListSize; ++i) { json_t * j = json_array_get(group, i); @@ -77,7 +76,6 @@ void LoadGroupTwoComponentMwmsImpl(int depth, json_t * group, TCountryId const & // @TODO(bykoianko) After we stop supporting two component mwms (with routing files) // remove code below. size_t const groupListSize = json_array_size(group); - toDo.ReserveAtDepth(depth, groupListSize); for (size_t i = 0; i < groupListSize; ++i) { json_t * j = json_array_get(group, i); @@ -175,8 +173,6 @@ public: } TMapping GetMapping() const { return m_idsMapping; } - - void ReserveAtDepth(int level, size_t n) { m_cont.ReserveAtDepth(level, n); } }; class DoStoreCountriesTwoComponentMwms @@ -198,8 +194,6 @@ public: } m_cont.AddAtDepth(depth, country); } - - void ReserveAtDepth(int level, size_t n) { m_cont.ReserveAtDepth(level, n); } }; class DoStoreFile2InfoSingleMwms @@ -226,7 +220,6 @@ public: void SetCountriesContainerAttrs(uint32_t, size_t) {} TMapping GetMapping() const { return m_idsMapping; } - void ReserveAtDepth(int, size_t) {} }; class DoStoreFile2InfoTwoComponentMwms @@ -247,7 +240,6 @@ public: CountryInfo info(id); m_file2info[id] = info; } - void ReserveAtDepth(int, size_t) {} }; } // namespace diff --git a/storage/country.hpp b/storage/country.hpp index 27716d73f4..5a4a9ea3aa 100644 --- a/storage/country.hpp +++ b/storage/country.hpp @@ -2,7 +2,7 @@ #include "storage/country_decl.hpp" #include "storage/index.hpp" -#include "storage/simple_tree.hpp" +#include "storage/country_tree.hpp" #include "storage/storage_defines.hpp" #include "platform/local_country_file.hpp" @@ -74,7 +74,7 @@ public: TCountryId const & Name() const { return m_name; } }; -typedef SimpleTree TCountriesContainer; +typedef CountryTree TCountriesContainer; /// @return version of country file or -1 if error was encountered int64_t LoadCountries(string const & jsonBuffer, TCountriesContainer & countries, TMapping * mapping = nullptr); diff --git a/storage/simple_tree.hpp b/storage/country_tree.hpp similarity index 74% rename from storage/simple_tree.hpp rename to storage/country_tree.hpp index 2a43a3df69..e08bd4a962 100644 --- a/storage/simple_tree.hpp +++ b/storage/country_tree.hpp @@ -7,19 +7,19 @@ #include "std/vector.hpp" /// This class is developed for using in Storage. It's a implementation of a tree. -/// It should be filled with AddAtDepth method. Before calling AddAtDepth for a node -/// ReserveAtDepth should be called for the node. The tree can be filled only -/// according to BFS order. +/// It should be filled with AddAtDepth method. +/// This class is used in Storage and filled based on countries.txt (countries_migrate.txt). +/// While filling CountryTree nodes in countries.txt should be visited in DFS order. template -class SimpleTree +class CountryTree { T m_value; /// \brief m_children contains all first generation descendants of the node. /// Note. Once created the order of elements of |m_children| should not be changed. /// See implementation of AddAtDepth and Add methods for details. - vector>> m_children; - SimpleTree * m_parent; + vector>> m_children; + CountryTree * m_parent; static bool IsEqual(T const & v1, T const & v2) { @@ -27,7 +27,7 @@ class SimpleTree } public: - SimpleTree(T const & value = T(), SimpleTree * parent = nullptr) + CountryTree(T const & value = T(), CountryTree * parent = nullptr) : m_value(value), m_parent(parent) { } @@ -38,25 +38,6 @@ public: return m_value; } - /// \brief Reserves child size vector. This method should be called once for every node - /// just before filling children vector of the node to prevent m_children vector - /// relocation while filling. - /// \param level depth of node which children vector will be reserved. - /// \n number of vector elements will be reserved. - void ReserveAtDepth(int level, size_t n) - { - SimpleTree * node = this; - while (level-- > 0 && !node->m_children.empty()) - node = node->m_children.back().get(); - ASSERT_EQUAL(level, -1, ()); - return node->Reserve(n); - } - - void Reserve(size_t n) - { - m_children.reserve(n); - } - /// @return reference is valid only up to the next tree structure modification T & Value() { @@ -66,7 +47,7 @@ public: /// @return reference is valid only up to the next tree structure modification T & AddAtDepth(int level, T const & value) { - SimpleTree * node = this; + CountryTree * node = this; while (level-- > 0 && !node->m_children.empty()) node = node->m_children.back().get(); ASSERT_EQUAL(level, -1, ()); @@ -76,7 +57,7 @@ public: /// @return reference is valid only up to the next tree structure modification T & Add(T const & value) { - m_children.emplace_back(make_unique>(value, this)); + m_children.emplace_back(make_unique>(value, this)); return m_children.back()->Value(); } @@ -86,7 +67,7 @@ public: m_children.clear(); } - bool operator<(SimpleTree const & other) const + bool operator<(CountryTree const & other) const { return Value() < other.Value(); } @@ -97,7 +78,7 @@ public: /// @TODO(bykoianko) The complexity of the method is O(n). But the structure (tree) is built on the start of the program /// and then actively used on run time. This method (and class) should be redesigned to make the function work faster. /// A hash table is being planned to use. - void Find(T const & value, vector const *> & found) const + void Find(T const & value, vector const *> & found) const { if (IsEqual(m_value, value)) found.push_back(this); @@ -105,14 +86,14 @@ public: child->Find(value, found); } - SimpleTree const * const FindFirst(T const & value) const + CountryTree const * const FindFirst(T const & value) const { if (IsEqual(m_value, value)) return this; for (auto const & child : m_children) { - SimpleTree const * const found = child->FindFirst(value); + CountryTree const * const found = child->FindFirst(value); if (found != nullptr) return found; } @@ -124,14 +105,14 @@ public: /// When new countries.txt with unique ids will be added FindLeaf will be removed /// and Find will be used intead. /// @TODO(bykoianko) Remove this method on countries.txt update. - SimpleTree const * const FindFirstLeaf(T const & value) const + CountryTree const * const FindFirstLeaf(T const & value) const { if (IsEqual(m_value, value) && m_children.empty()) return this; // It's a leaf. for (auto const & child : m_children) { - SimpleTree const * const found = child->FindFirstLeaf(value); + CountryTree const * const found = child->FindFirstLeaf(value); if (found != nullptr) return found; } @@ -140,13 +121,13 @@ public: bool HasParent() const { return m_parent != nullptr; } - SimpleTree const & Parent() const + CountryTree const & Parent() const { CHECK(HasParent(), ()); return *m_parent; } - SimpleTree const & Child(size_t index) const + CountryTree const & Child(size_t index) const { ASSERT_LESS(index, m_children.size(), ()); return *m_children[index]; diff --git a/storage/index.hpp b/storage/index.hpp index bee00beda0..a441874c4f 100644 --- a/storage/index.hpp +++ b/storage/index.hpp @@ -10,7 +10,6 @@ namespace storage using TCountryId = string; using TCountriesSet = set; using TCountriesVec = vector; -using TCountriesUnorderedSet = unordered_set; extern const storage::TCountryId kInvalidCountryId; diff --git a/storage/storage.cpp b/storage/storage.cpp index 226191e5eb..bc3466961a 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -32,7 +32,6 @@ namespace storage { namespace { - template void RemoveIf(vector & v, function const & p) { @@ -80,16 +79,37 @@ void DeleteFromDiskWithIndexes(LocalCountryFile const & localFile, MapOptions op TCountriesContainer const & LeafNodeFromCountryId(TCountriesContainer const & root, TCountryId const & countryId) { - SimpleTree const * node = root.FindFirstLeaf(Country(countryId)); + CountryTree const * node = root.FindFirstLeaf(Country(countryId)); CHECK(node, ("Node with id =", countryId, "not found in country tree as a leaf.")); return *node; } -void HashFromQueue(Storage::TQueue const & queue, TCountriesUnorderedSet & hashQueue) +void GetQueuedCountries(Storage::TQueue const & queue, TCountriesSet & countries) { - hashQueue.reserve(queue.size()); for (auto const & country : queue) - hashQueue.insert(country.GetCountryId()); + countries.insert(country.GetCountryId()); +} + +void CorrectJustDownloaded(Storage::TQueue::iterator justDownloadedItem, Storage::TQueue & queue, + TCountriesSet & justDownloaded) +{ + TCountryId const justDownloadedCountry = justDownloadedItem->GetCountryId(); + queue.erase(justDownloadedItem); + if (queue.empty()) + justDownloaded.clear(); + else + justDownloaded.insert(justDownloadedCountry); +} + +bool IsDownloadingStatus(Status status) +{ + return status == Status::EDownloading || status == Status::EInQueue; +} + +bool IsUserAttentionNeededStatus(Status status) +{ + return status == Status::EDownloadFailed || status == Status::EOutOfMemFailed + || status == Status::EOnDiskOutOfDate || status == Status::EUnknown; } } // namespace @@ -292,7 +312,7 @@ Country const & Storage::CountryLeafByCountryId(TCountryId const & countryId) co Country const & Storage::CountryByCountryId(TCountryId const & countryId) const { - SimpleTree const * node = m_countries.FindFirst(Country(countryId)); + CountryTree const * node = m_countries.FindFirst(Country(countryId)); CHECK(node, ("Node with id =", countryId, "not found in country tree.")); return node->Value(); } @@ -530,11 +550,7 @@ void Storage::DownloadNextCountryFromQueue() { OnMapDownloadFinished(countryId, false /* success */, queuedCountry.GetInitOptions()); NotifyStatusChanged(countryId); - m_queue.pop_front(); - if (m_queue.empty()) - m_justDownloaded.clear(); - else - m_justDownloaded.insert(countryId); + CorrectJustDownloaded(m_queue.begin(), m_queue, m_justDownloaded); DownloadNextCountryFromQueue(); return; } @@ -660,8 +676,7 @@ void Storage::OnMapFileDownloadFinished(bool success, } OnMapDownloadFinished(countryId, success, queuedCountry.GetInitOptions()); - m_queue.pop_front(); - m_justDownloaded.insert(countryId); + CorrectJustDownloaded(m_queue.begin(), m_queue, m_justDownloaded); SaveDownloadQueue(); NotifyStatusChanged(countryId); @@ -669,7 +684,7 @@ void Storage::OnMapFileDownloadFinished(bool success, DownloadNextCountryFromQueue(); } -void Storage::ReportProgress(TCountryId const & countryId, pair const & p) +void Storage::ReportProgress(TCountryId const & countryId, MapFilesDownloader::TProgress const & p) { ASSERT_THREAD_CHECKER(m_threadChecker, ()); for (CountryObservers const & o : m_observers) @@ -677,22 +692,26 @@ void Storage::ReportProgress(TCountryId const & countryId, pair const & leafProgress) + MapFilesDownloader::TProgress const & leafProgress) { // Reporting progress for a leaf in country tree. ReportProgress(countryId, leafProgress); // Reporting progress for the parents of the leaf with |countryId|. - TCountriesUnorderedSet hashQueue; - HashFromQueue(m_queue, hashQueue); + TCountriesSet setQueue; + GetQueuedCountries(m_queue, setQueue); - // This lambda will be called for every parent of countryId (except for the root) - // to calculate and report parent's progress. - auto calcProgress = [this, &leafProgress, &hashQueue, &countryId] - (TCountryId const & parentId, TCountriesVec const & descendants) + auto calcProgress = [&] + (TCountryId const & parentId, TCountriesContainer const & parentNode) { - pair localAndRemoteBytes = CalculateProgress(descendants, - countryId, leafProgress, hashQueue); + TCountriesVec descendants; + parentNode.ForEachDescendant([&descendants](TCountriesContainer const & container) + { + descendants.push_back(container.Value().Name()); + }); + + MapFilesDownloader::TProgress localAndRemoteBytes = + CalculateProgress(countryId, descendants, leafProgress, setQueue); ReportProgress(parentId, localAndRemoteBytes); }; @@ -1037,13 +1056,7 @@ bool Storage::DeleteCountryFilesFromDownloader(TCountryId const & countryId, Map // Remove country from the queue if there's nothing to download. if (queuedCountry->GetInitOptions() == MapOptions::Nothing) - { - m_queue.erase(find(m_queue.begin(), m_queue.end(), countryId)); - if (m_queue.empty()) - m_justDownloaded.clear(); - else - m_justDownloaded.insert(countryId); - } + CorrectJustDownloaded(find(m_queue.begin(), m_queue.end(), countryId), m_queue, m_justDownloaded); if (!m_queue.empty() && m_downloader->IsIdle()) { @@ -1215,25 +1228,20 @@ Status Storage::NodeStatus(TCountriesContainer const & node) const if (node.ChildrenCount() == 0) return CountryStatusEx(node.Value().Name()); - // Expandable node status. + // Group node status. Status const kDownloadingInProgress = Status::EDownloading; Status const kUsersAttentionNeeded = Status::EDownloadFailed; - Status const kEverythingsOk = Status::EOnDisk; + Status const kEverythingOk = Status::EOnDisk; - Status result = kEverythingsOk; + Status result = kEverythingOk; auto groupStatusCalculator = [&result, this](TCountriesContainer const & nodeInSubtree) { - set const kDownloadingInProgressSet = { Status::EDownloading, Status::EInQueue }; - set const kUsersAttentionNeededSet = { Status::EDownloadFailed, Status::EOutOfMemFailed, - Status::EUnknown, Status::EOnDiskOutOfDate }; - // The other statuses say that everything is ok (kEverythingsOk). - if (result == kDownloadingInProgress || nodeInSubtree.ChildrenCount() != 0) return; Status status = this->CountryStatusEx(nodeInSubtree.Value().Name()); ASSERT_NOT_EQUAL(status, Status::EUndefined, ()); - if (kDownloadingInProgressSet.count(status) != 0) + if (IsDownloadingStatus(status)) { result = kDownloadingInProgress; return; @@ -1242,7 +1250,7 @@ Status Storage::NodeStatus(TCountriesContainer const & node) const if (result == kUsersAttentionNeeded) return; - if (kUsersAttentionNeededSet.count(status) != 0) + if (IsUserAttentionNeededStatus(status)) { result = kUsersAttentionNeeded; return; @@ -1257,7 +1265,7 @@ void Storage::GetNodeAttrs(TCountryId const & countryId, NodeAttrs & nodeAttrs) { ASSERT_THREAD_CHECKER(m_threadChecker, ()); - vector const *> nodes; + vector const *> nodes; m_countries.Find(Country(countryId), nodes); CHECK(!nodes.empty(), ()); // If nodes.size() > 1 countryId corresponds to a disputed territories. @@ -1283,10 +1291,11 @@ void Storage::GetNodeAttrs(TCountryId const & countryId, NodeAttrs & nodeAttrs) MapFilesDownloader::TProgress downloadingMwmProgress = m_downloader->IsIdle() ? make_pair(0LL, 0LL) : m_downloader->GetDownloadingProgress(); - TCountriesUnorderedSet hashQueue; - HashFromQueue(m_queue, hashQueue); + + TCountriesSet setQueue; + GetQueuedCountries(m_queue, setQueue); nodeAttrs.m_downloadingProgress = - CalculateProgress(descendants, downloadingMwm, downloadingMwmProgress, hashQueue); + CalculateProgress(downloadingMwm, descendants, downloadingMwmProgress, setQueue); nodeAttrs.m_parentInfo.clear(); nodeAttrs.m_parentInfo.reserve(nodes.size()); @@ -1318,13 +1327,16 @@ void Storage::DoClickOnDownloadMap(TCountryId const & countryId) m_downloadMapOnTheMap(countryId); } -pair Storage::CalculateProgress(TCountriesVec const & descendants, - TCountryId const & downloadingMwm, - pair const & downloadingMwmProgress, - TCountriesUnorderedSet const & mwmsInQueue) const +MapFilesDownloader::TProgress Storage::CalculateProgress(TCountryId const & downloadingMwm, + TCountriesVec const & mwms, + MapFilesDownloader::TProgress const & downloadingMwmProgress, + TCountriesSet const & mwmsInQueue) const { - pair localAndRemoteBytes = make_pair(0, 0); - for (auto const & d : descendants) + MapFilesDownloader::TProgress localAndRemoteBytes = make_pair(0, 0); + if (downloadingMwm != kInvalidCountryId) + localAndRemoteBytes = downloadingMwmProgress; + + for (auto const & d : mwms) { if (d == downloadingMwm) continue; @@ -1338,17 +1350,12 @@ pair Storage::CalculateProgress(TCountriesVec const & descenda if (m_justDownloaded.count(d) != 0) { - CountryFile const & localCountryFile = GetCountryFile(d); - localAndRemoteBytes.first += localCountryFile.GetRemoteSize(MapOptions::Map); - localAndRemoteBytes.second += localAndRemoteBytes.first; + size_t const localCountryFileSz = GetCountryFile(d).GetRemoteSize(MapOptions::Map); + localAndRemoteBytes.first += localCountryFileSz; + localAndRemoteBytes.second += localCountryFileSz; } } - if (downloadingMwm != kInvalidCountryId) - { - // Downloading is in progress right now. - localAndRemoteBytes.first += downloadingMwmProgress.first; - localAndRemoteBytes.second += downloadingMwmProgress.second; - } + return localAndRemoteBytes; } } // namespace storage diff --git a/storage/storage.hpp b/storage/storage.hpp index 2e940f90d4..6a8d0a884b 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -69,7 +69,7 @@ struct NodeAttrs /// m_downloadingProgress.first is number of downloaded bytes. /// m_downloadingProgress.second is size of file(s) in bytes to download. /// So m_downloadingProgress.first <= m_downloadingProgress.second. - pair m_downloadingProgress; + MapFilesDownloader::TProgress m_downloadingProgress; NodeStatus m_status; NodeErrorCode m_error; @@ -111,7 +111,7 @@ private: /// When a new mwm file is added to |m_queue| |m_justDownloaded| is cleared. /// Note. This set is necessary for implementation of downloading progress of /// mwm group. - TCountriesUnorderedSet m_justDownloaded; + TCountriesSet m_justDownloaded; /// stores countries whose download has failed recently TCountriesSet m_failedCountries; @@ -173,9 +173,9 @@ private: void LoadCountriesFile(string const & pathToCountriesFile, string const & dataDir, TMapping * mapping = nullptr); - void ReportProgress(TCountryId const & countryId, pair const & p); + void ReportProgress(TCountryId const & countryId, MapFilesDownloader::TProgress const & p); void ReportProgressForHierarchy(TCountryId const & countryId, - pair const & leafProgress); + MapFilesDownloader::TProgress const & leafProgress); /// Called on the main thread by MapFilesDownloader when list of /// suitable servers is received. @@ -509,10 +509,10 @@ private: /// the leaf node in bytes. |downloadingMwmProgress.first| == number of downloaded bytes. /// |downloadingMwmProgress.second| == number of bytes in downloading files. /// |mwmsInQueue| hash table made from |m_queue|. - pair CalculateProgress(TCountriesVec const & descendants, - TCountryId const & downloadingMwm, - pair const & downloadingMwmProgress, - TCountriesUnorderedSet const & mwmsInQueue) const; + MapFilesDownloader::TProgress CalculateProgress(TCountryId const & downloadingMwm, + TCountriesVec const & descendants, + MapFilesDownloader::TProgress const & downloadingMwmProgress, + TCountriesSet const & mwmsInQueue) const; }; template @@ -524,43 +524,43 @@ void Storage::ForEachInSubtree(TCountryId const & root, ToDo && toDo) const ASSERT(false, ("TCountryId =", root, "not found in m_countries.")); return; } - rootNode->ForEachInSubtree([&toDo](TCountriesContainer const & countryContainer) + rootNode->ForEachInSubtree([&toDo](TCountriesContainer const & container) { - Country const & value = countryContainer.Value(); + Country const & value = container.Value(); toDo(value.Name(), value.GetSubtreeMwmCounter() != 1 /* expandableNode. */); }); } -/// Calls functor |toDo| with sigrature +/// Calls functor |toDo| with signature /// void(const TCountryId const & parentId, TCountriesVec const & descendantCountryId) -/// for each parent (including indirect/great parents) except for the main root of the tree. +/// for each ancestor except for the main root of the tree. /// |descendantsCountryId| is a vector of country id of descendats of |parentId|. /// Note. In case of disputable territories several nodes with the same name may be /// present in the country tree. In that case ForEachAncestorExceptForTheRoot calls -/// |toDo| for parents of each branch in the country tree. +/// |toDo| for parents of each way to the root in the country tree. template -void Storage::ForEachAncestorExceptForTheRoot(TCountryId const & childId, ToDo && toDo) const +void Storage::ForEachAncestorExceptForTheRoot(TCountryId const & countryId, ToDo && toDo) const { - vector const *> nodes; - m_countries.Find(Country(childId), nodes); + vector const *> nodes; + m_countries.Find(Country(countryId), nodes); if (nodes.empty()) { - ASSERT(false, ("TCountryId =", childId, "not found in m_countries.")); + ASSERT(false, ("TCountryId =", countryId, "not found in m_countries.")); return; } + TCountriesSet visitedAncestors; // In most cases nodes.size() == 1. In case of disputable territories nodes.size() // may be more than one. It means |childId| is present in the country tree more than once. for (auto const & node : nodes) { - node->ForEachAncestorExceptForTheRoot([&toDo](TCountriesContainer const & countryContainer) + node->ForEachAncestorExceptForTheRoot([&](TCountriesContainer const & container) { - TCountriesVec descendantsCountryId; - countryContainer.ForEachDescendant([&descendantsCountryId](TCountriesContainer const & countryContainer) - { - descendantsCountryId.push_back(countryContainer.Value().Name()); - }); - toDo(countryContainer.Value().Name(), descendantsCountryId); + TCountryId const ancestorId = container.Value().Name(); + if (visitedAncestors.find(ancestorId) != visitedAncestors.end()) + return; // The node was visited before because countryId is present in the tree more than once. + visitedAncestors.insert(ancestorId); + toDo(ancestorId, container); }); } } diff --git a/storage/storage.pro b/storage/storage.pro index 03761dad3d..f811485857 100644 --- a/storage/storage.pro +++ b/storage/storage.pro @@ -16,11 +16,11 @@ HEADERS += \ country_info_getter.hpp \ country_name_getter.hpp \ country_polygon.hpp \ + country_tree.hpp \ http_map_files_downloader.hpp \ index.hpp \ map_files_downloader.hpp \ queued_country.hpp \ - simple_tree.hpp \ storage.hpp \ storage_defines.hpp \ storage_helpers.hpp \ diff --git a/storage/storage_tests/simple_tree_test.cpp b/storage/storage_tests/simple_tree_test.cpp index 697ab15509..bca22cbd3d 100644 --- a/storage/storage_tests/simple_tree_test.cpp +++ b/storage/storage_tests/simple_tree_test.cpp @@ -1,7 +1,6 @@ #include "testing/testing.hpp" -#include "storage/simple_tree.hpp" - +#include "storage/country_tree.hpp" namespace { @@ -17,10 +16,10 @@ struct Calculator }; } // namespace -UNIT_TEST(SimpleTree_Smoke) +UNIT_TEST(CountryTree_Smoke) { - typedef SimpleTree TreeT; - TreeT tree; + typedef CountryTree TTree; + TTree tree; tree.Add(4); tree.Add(3); @@ -47,20 +46,20 @@ UNIT_TEST(SimpleTree_Smoke) TEST_EQUAL(tree.Child(4).Child(0).Parent().Value(), 1, ()); TEST_EQUAL(tree.Child(4).Child(2).Parent().Value(), 1, ()); - Calculator c1; + Calculator c1; tree.ForEachChild(c1); TEST_EQUAL(c1.count, 5, ()); - Calculator c2; + Calculator c2; tree.ForEachDescendant(c2); TEST_EQUAL(c2.count, 8, ()); - Calculator c3; + Calculator c3; tree.Child(4).Child(0).ForEachAncestorExceptForTheRoot(c3); TEST_EQUAL(c3.count, 1, ()); tree.Clear(); - Calculator c4; + Calculator c4; tree.ForEachDescendant(c4); TEST_EQUAL(c4.count, 0, ("Tree should be empty")); } diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index fef2392474..cef5e761f5 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -1307,8 +1307,14 @@ UNIT_TEST(StorageTest_ForEachAncestorExceptForTheRoot) // Two parent case. auto const forEachParentDisputableTerritory - = [](TCountryId const & parentId, TCountriesVec const & descendants) + = [](TCountryId const & parentId, TCountriesContainer const & parentNode) { + TCountriesVec descendants; + parentNode.ForEachDescendant([&descendants](TCountriesContainer const & container) + { + descendants.push_back(container.Value().Name()); + }); + if (parentId == "Country1") { TCountriesVec const expectedDescendants = {"Disputable Territory", "Indisputable Territory Of Country1"}; @@ -1327,8 +1333,14 @@ UNIT_TEST(StorageTest_ForEachAncestorExceptForTheRoot) // One parent case. auto const forEachParentIndisputableTerritory - = [](TCountryId const & parentId, TCountriesVec const & descendants) + = [](TCountryId const & parentId, TCountriesContainer const & parentNode) { + TCountriesVec descendants; + parentNode.ForEachDescendant([&descendants](TCountriesContainer const & container) + { + descendants.push_back(container.Value().Name()); + }); + if (parentId == "Country1") { TCountriesVec const expectedDescendants = {"Disputable Territory", "Indisputable Territory Of Country1"};