diff --git a/coding/file_name_utils.cpp b/coding/file_name_utils.cpp index f528925f2e..0a470f1aff 100644 --- a/coding/file_name_utils.cpp +++ b/coding/file_name_utils.cpp @@ -33,7 +33,7 @@ void GetNameFromFullPath(string & name) string GetNameFromFullPathWithoutExt(string const & path) { - std::string name = path; + string name = path; GetNameFromFullPath(name); GetNameWithoutExt(name); return name; diff --git a/drape/color.cpp b/drape/color.cpp index b918926104..437326d2e8 100644 --- a/drape/color.cpp +++ b/drape/color.cpp @@ -15,13 +15,11 @@ static const uint8_t MaxChannelValue = 255; Color::Color() : m_rgba(MaxChannelValue) -{ -} +{} Color::Color(uint32_t rgba) : m_rgba(rgba) -{ -} +{} Color::Color(uint8_t red, uint8_t green, uint8_t blue, uint8_t alpha) { diff --git a/drape/color.hpp b/drape/color.hpp index 1fd3cce25f..72d0ca1abc 100644 --- a/drape/color.hpp +++ b/drape/color.hpp @@ -12,7 +12,7 @@ namespace dp struct Color { Color(); - Color(uint32_t rgba); + explicit Color(uint32_t rgba); Color(uint8_t red, uint8_t green, uint8_t blue, uint8_t alpha); uint8_t GetRed() const; diff --git a/iphone/Maps/UI/EditBookmark/MWMBookmarkColorViewController.h b/iphone/Maps/UI/EditBookmark/MWMBookmarkColorViewController.h index c17335ef28..a7c8407b4b 100644 --- a/iphone/Maps/UI/EditBookmark/MWMBookmarkColorViewController.h +++ b/iphone/Maps/UI/EditBookmark/MWMBookmarkColorViewController.h @@ -16,7 +16,7 @@ inline NSString * TitleForBookmarkColor(kml::PredefinedColor color) case kml::PredefinedColor::Brown: return @"brown"; case kml::PredefinedColor::Green: return @"green"; case kml::PredefinedColor::Orange: return @"orange"; - case kml::PredefinedColor::None: return @""; + case kml::PredefinedColor::None: case kml::PredefinedColor::Count: return @""; } } diff --git a/map/bookmark_helpers.cpp b/map/bookmark_helpers.cpp index 7d0ac2447b..7fbf7196c0 100644 --- a/map/bookmark_helpers.cpp +++ b/map/bookmark_helpers.cpp @@ -8,15 +8,11 @@ std::unique_ptr LoadKmlFile(std::string const & file, bool useBinary) { - try - { - return LoadKmlData(FileReader(file), useBinary); - } - catch (std::exception const & e) - { - LOG(LWARNING, ("Exception while loading bookmarks:", e.what(), "file", file)); - } - return nullptr; + auto kmlData = LoadKmlData(FileReader(file), useBinary); + + if (kmlData == nullptr) + LOG(LWARNING, ("Loading bookmarks failed, file", file)); + return kmlData; } std::unique_ptr LoadKmlData(Reader const & reader, bool useBinary) @@ -37,25 +33,33 @@ std::unique_ptr LoadKmlData(Reader const & reader, bool useBinary } catch (Reader::Exception const & e) { - LOG(LWARNING, ("KML ", useBinary ? "binary" : "text", "deserialization failure:", e.what())); - data.reset(); + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "reading failure:", e.what())); + return nullptr; + } + catch (std::exception const & e) + { + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "deserialization failure:", e.what())); + return nullptr; } return data; } bool SaveKmlFile(kml::FileData & kmlData, std::string const & file, bool useBinary) { + bool success = false; try { FileWriter writer(file); - return SaveKmlData(kmlData, writer, useBinary); + success = SaveKmlData(kmlData, writer, useBinary); } catch (std::exception const & e) { - LOG(LWARNING, ("Exception while saving bookmarks:", e.what(), "file", file)); - return false; + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "saving failure:", e.what())); + success = false; } - return true; + if (!success) + LOG(LWARNING, ("Saving bookmarks failed, file", file)); + return success; } bool SaveKmlData(kml::FileData & kmlData, Writer & writer, bool useBinary) @@ -75,7 +79,12 @@ bool SaveKmlData(kml::FileData & kmlData, Writer & writer, bool useBinary) } catch (Writer::Exception const & e) { - LOG(LWARNING, ("KML ", useBinary ? "binary" : "text", "serialization failure:", e.what())); + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "writing failure:", e.what())); + return false; + } + catch (std::exception const & e) + { + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "serialization failure:", e.what())); return false; } return true; diff --git a/map/bookmark_manager.cpp b/map/bookmark_manager.cpp index 81a785ac7e..36a0c6bf1c 100644 --- a/map/bookmark_manager.cpp +++ b/map/bookmark_manager.cpp @@ -55,8 +55,8 @@ uint64_t LoadLastBmCategoryId() uint64_t lastId; std::string val; if (GetPlatform().GetSecureStorage().Load(kLastBookmarkCategoryId, val) && strings::to_uint64(val, lastId)) - return max(static_cast(UserMark::COUNT), lastId); - return static_cast(UserMark::COUNT); + return std::max(static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX), lastId); + return static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX); } void SaveLastBmCategoryId(uint64_t lastId) @@ -66,7 +66,7 @@ void SaveLastBmCategoryId(uint64_t lastId) uint64_t ResetLastBmCategoryId() { - auto const lastId = static_cast(UserMark::COUNT); + auto const lastId = static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX); SaveLastBmCategoryId(lastId); return lastId; } @@ -176,10 +176,7 @@ bool ConvertBeforeUploading(std::string const & filePath, std::string const & co return false; if (!SaveKmlFile(*kmlData, tmpFilePath, false /* binary */)) - { - my::DeleteFileX(tmpFilePath); return false; - } return CreateZipFromPathDeflatedAndDefaultCompression(tmpFilePath, convertedFilePath); } @@ -398,9 +395,9 @@ BookmarkManager::BookmarkManager(Callbacks && callbacks) std::bind(&ConvertAfterDownloading, _1, _2))) { ASSERT(m_callbacks.m_getStringsBundle != nullptr, ()); - ASSERT_GREATER_OR_EQUAL(m_lastGroupID, UserMark::COUNT, ()); - m_userMarkLayers.reserve(UserMark::COUNT - 1); - for (uint32_t i = 1; i < UserMark::COUNT; ++i) + ASSERT_GREATER_OR_EQUAL(m_lastGroupID, UserMark::USER_MARK_TYPES_COUNT_MAX, ()); + m_userMarkLayers.reserve(UserMark::USER_MARK_TYPES_COUNT - 1); + for (uint32_t i = 1; i < UserMark::USER_MARK_TYPES_COUNT; ++i) m_userMarkLayers.emplace_back(std::make_unique(static_cast(i))); m_selectionMark = CreateUserMark(m2::PointD{}); @@ -789,8 +786,8 @@ void BookmarkManager::LoadState() UNUSED_VALUE(settings::Get(kLastEditedBookmarkCategory, m_lastCategoryUrl)); uint32_t color; if (settings::Get(kLastEditedBookmarkColor, color) && - color > static_cast(kml::PredefinedColor::None) && - color < static_cast(kml::PredefinedColor::Count)) + color > static_cast(kml::PredefinedColor::None) && + color < static_cast(kml::PredefinedColor::Count)) { m_lastColor = static_cast(color); } @@ -815,10 +812,8 @@ void BookmarkManager::ClearCategories() m_tracks.clear(); } -std::shared_ptr BookmarkManager::LoadBookmarks(std::string const & dir, - std::string const & ext, - bool binary, - std::vector & filePaths) +BookmarkManager::KMLDataCollectionPtr BookmarkManager::LoadBookmarks(std::string const & dir, std::string const & ext, + bool binary, std::vector & filePaths) { Platform::FilesList files; Platform::GetFilesByExt(dir, ext, files); @@ -891,13 +886,12 @@ void BookmarkManager::LoadBookmarkRoutine(std::string const & filePath, bool isT bool const migrated = migration::IsMigrationCompleted(); - std::string fileSavePath; auto collection = std::make_shared(); auto const savePath = GetKMLPath(filePath); if (savePath) { - fileSavePath = savePath.get(); + auto fileSavePath = savePath.get(); auto kmlData = LoadKmlFile(fileSavePath, false /* useBinary */); if (kmlData != nullptr) { @@ -929,12 +923,6 @@ void BookmarkManager::LoadBookmarkRoutine(std::string const & filePath, bool isT NotifyAboutFile(success, filePath, isTemporaryFile); NotifyAboutFinishAsyncLoading(std::move(collection)); - - if (migrated && success) - { - GetPlatform().RunTask(Platform::Thread::Gui, - [this, fileSavePath]() { m_bookmarkCloud.Init({fileSavePath}); }); - } }); } @@ -951,7 +939,7 @@ void BookmarkManager::NotifyAboutStartAsyncLoading() }); } -void BookmarkManager::NotifyAboutFinishAsyncLoading(std::shared_ptr && collection) +void BookmarkManager::NotifyAboutFinishAsyncLoading(KMLDataCollectionPtr && collection) { if (m_needTeardown) return; @@ -1185,8 +1173,7 @@ kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(kml::CategoryData && da } auto groupId = data.m_id; ASSERT_EQUAL(m_categories.count(groupId), 0, ()); - auto & cat = m_categories[groupId]; - cat = my::make_unique(std::move(data), autoSave); + m_categories[groupId] = my::make_unique(std::move(data), autoSave); m_bmGroupsIdList.push_back(groupId); m_changesTracker.OnAddGroup(groupId); return groupId; @@ -1197,8 +1184,7 @@ kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(std::string const & nam ASSERT_THREAD_CHECKER(m_threadChecker, ()); auto const groupId = ++m_lastGroupID; SaveLastBmCategoryId(m_lastGroupID); - auto & cat = m_categories[groupId]; - cat = my::make_unique(name, groupId, autoSave); + m_categories[groupId] = my::make_unique(name, groupId, autoSave); m_bmGroupsIdList.push_back(groupId); m_changesTracker.OnAddGroup(groupId); return groupId; @@ -1294,7 +1280,7 @@ UserMark const * BookmarkManager::FindNearestUserMark(TTouchRectHolder const & h UserMarkLayer const * BookmarkManager::GetGroup(kml::MarkGroupId groupId) const { ASSERT_THREAD_CHECKER(m_threadChecker, ()); - if (groupId < UserMark::Type::COUNT) + if (groupId < UserMark::Type::USER_MARK_TYPES_COUNT) return m_userMarkLayers[groupId - 1].get(); ASSERT(m_categories.find(groupId) != m_categories.end(), ()); @@ -1304,7 +1290,7 @@ UserMarkLayer const * BookmarkManager::GetGroup(kml::MarkGroupId groupId) const UserMarkLayer * BookmarkManager::GetGroup(kml::MarkGroupId groupId) { ASSERT_THREAD_CHECKER(m_threadChecker, ()); - if (groupId < UserMark::Type::COUNT) + if (groupId < UserMark::Type::USER_MARK_TYPES_COUNT) return m_userMarkLayers[groupId - 1].get(); auto const it = m_categories.find(groupId); @@ -1331,7 +1317,7 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool auto & categoryData = fileData.m_categoryData; auto const it = std::find_if(categoriesForMerge.cbegin(), categoriesForMerge.cend(), - [categoryData](auto const & v) + [&categoryData](auto const & v) { return v.second->GetName() == kml::GetDefaultStr(categoryData.m_name); }); @@ -1420,7 +1406,7 @@ bool BookmarkManager::SaveBookmarkCategory(kml::MarkGroupId groupId, Writer & wr return SaveKmlData(*kmlData, writer, useBinary); } -std::shared_ptr BookmarkManager::PrepareToSaveBookmarks( +BookmarkManager::KMLDataCollectionPtr BookmarkManager::PrepareToSaveBookmarks( kml::GroupIdCollection const & groupIdCollection) { bool migrated = migration::IsMigrationCompleted(); @@ -1429,7 +1415,7 @@ std::shared_ptr BookmarkManager::PrepareToSa std::string const fileExt = migrated ? kKmbExtension : kKmlExtension; if (migrated && !GetPlatform().IsFileExistsByFullPath(fileDir) && !GetPlatform().MkDirChecked(fileDir)) - return std::shared_ptr(); + return nullptr; auto collection = std::make_shared(); @@ -1462,7 +1448,6 @@ bool BookmarkManager::SaveKmlFileSafe(kml::FileData & kmlData, std::string const VERIFY(my::RenameFileX(fileTmp, file), (fileTmp, file)); return true; } - // Remove possibly left tmp file. my::DeleteFileX(fileTmp); return false; } @@ -1606,7 +1591,6 @@ void BookmarkManager::ConvertAllKmlFiles(ConversionHandler && handler) auto fileData = std::make_shared(); - // TODO(darina): Check this refactoring. bool allConverted = true; for (auto const & f : files) { @@ -1732,7 +1716,7 @@ kml::GroupIdSet BookmarkManager::MarksChangesTracker::GetAllGroupIds() const { auto const & groupIds = m_bmManager.GetBmGroupsIdList(); kml::GroupIdSet resultingSet(groupIds.begin(), groupIds.end()); - for (uint32_t i = 1; i < UserMark::COUNT; ++i) + for (uint32_t i = 1; i < UserMark::USER_MARK_TYPES_COUNT; ++i) resultingSet.insert(static_cast(i)); return resultingSet; } diff --git a/map/bookmark_manager.hpp b/map/bookmark_manager.hpp index 581d18e607..c3b1a8fb94 100644 --- a/map/bookmark_manager.hpp +++ b/map/bookmark_manager.hpp @@ -37,6 +37,7 @@ class BookmarkManager final public: using KMLDataCollection = std::vector>>; + using KMLDataCollectionPtr = std::shared_ptr; using AsyncLoadingStartedCallback = std::function; using AsyncLoadingFinishedCallback = std::function; @@ -140,12 +141,13 @@ public: void UpdateViewport(ScreenBase const & screen); void Teardown(); + + static bool IsBookmarkCategory(kml::MarkGroupId groupId) { return groupId >= UserMark::USER_MARK_TYPES_COUNT_MAX; } + static bool IsBookmark(kml::MarkId markId) { return UserMark::GetMarkType(markId) == UserMark::BOOKMARK; } static UserMark::Type GetGroupType(kml::MarkGroupId groupId) { - return groupId >= UserMark::COUNT ? UserMark::BOOKMARK : static_cast(groupId); + return IsBookmarkCategory(groupId) ? UserMark::BOOKMARK : static_cast(groupId); } - static bool IsBookmarkCategory(kml::MarkGroupId groupId) { return groupId >= UserMark::COUNT; } - static bool IsBookmark(kml::MarkId markId) { return UserMark::GetMarkType(markId) == UserMark::BOOKMARK; } template UserMarkT const * GetMark(kml::MarkId markId) const @@ -186,8 +188,6 @@ public: /// Scans and loads all kml files with bookmarks. void LoadBookmarks(); void LoadBookmark(std::string const & filePath, bool isTemporaryFile); - std::shared_ptr LoadBookmarks(std::string const & dir, std::string const & ext, bool binary, - std::vector & filePaths); /// Uses the same file name from which was loaded, or /// creates unique file name on first save and uses it every time. @@ -407,10 +407,12 @@ private: void SaveState() const; void LoadState(); void NotifyAboutStartAsyncLoading(); - void NotifyAboutFinishAsyncLoading(std::shared_ptr && collection); + void NotifyAboutFinishAsyncLoading(KMLDataCollectionPtr && collection); boost::optional GetKMLPath(std::string const & filePath); void NotifyAboutFile(bool success, std::string const & filePath, bool isTemporaryFile); void LoadBookmarkRoutine(std::string const & filePath, bool isTemporaryFile); + KMLDataCollectionPtr LoadBookmarks(std::string const & dir, std::string const & ext, bool binary, + std::vector & filePaths); void CollectDirtyGroups(kml::GroupIdSet & dirtyGroups); @@ -421,7 +423,7 @@ private: void CheckAndResetLastIds(); std::unique_ptr CollectBmGroupKMLData(BookmarkCategory const * group) const; - std::shared_ptr PrepareToSaveBookmarks(kml::GroupIdCollection const & groupIdCollection); + KMLDataCollectionPtr PrepareToSaveBookmarks(kml::GroupIdCollection const & groupIdCollection); bool SaveKmlFileSafe(kml::FileData & kmlData, std::string const & file, bool useBinary); void OnSynchronizationStarted(Cloud::SynchronizationType type); diff --git a/map/user_mark.cpp b/map/user_mark.cpp index f32f5a1ebc..ab6f02e7ea 100644 --- a/map/user_mark.cpp +++ b/map/user_mark.cpp @@ -45,7 +45,7 @@ kml::MarkId GetNextUserMarkId(UserMark::Type type, bool reset = false) return kml::kInvalidMarkId; } - static_assert(UserMark::Type::COUNT <= (1 << kMarkIdTypeBitsCount), "Not enough bits for user mark type."); + static_assert(UserMark::Type::USER_MARK_TYPES_COUNT <= (1 << kMarkIdTypeBitsCount), "Not enough bits for user mark type."); auto const typeBits = static_cast(type) << (sizeof(kml::MarkId) * 8 - kMarkIdTypeBitsCount); if (type == UserMark::Type::BOOKMARK) @@ -147,6 +147,7 @@ string DebugPrint(UserMark::Type type) case UserMark::Type::ROUTING: return "ROUTING"; case UserMark::Type::LOCAL_ADS: return "LOCAL_ADS"; case UserMark::Type::TRANSIT: return "TRANSIT"; - case UserMark::Type::COUNT: return "COUNT"; + case UserMark::Type::USER_MARK_TYPES_COUNT: return "USER_MARK_TYPES_COUNT"; + case UserMark::Type::USER_MARK_TYPES_COUNT_MAX: return "USER_MARK_TYPES_COUNT_MAX"; } } diff --git a/map/user_mark.hpp b/map/user_mark.hpp index b07d29ab42..fd20ca58f9 100644 --- a/map/user_mark.hpp +++ b/map/user_mark.hpp @@ -40,7 +40,8 @@ public: TRANSIT, LOCAL_ADS, DEBUG_MARK, - COUNT, + USER_MARK_TYPES_COUNT, + USER_MARK_TYPES_COUNT_MAX = 1000, }; UserMark(kml::MarkId id, m2::PointD const & ptOrg, UserMark::Type type);