From 6b6398af04c87f3d4899c025e65f2b153588710e Mon Sep 17 00:00:00 2001 From: codereader Date: Wed, 1 Apr 2020 06:30:09 +0200 Subject: [PATCH] #5122: Refactor IFilterVisitor to be a functor, replace some insert(std::make_pair()) calls with emplace. --- include/ifilter.h | 22 ++++-------- radiant/filters/BasicFilterSystem.cpp | 28 +++++++-------- radiant/filters/BasicFilterSystem.h | 2 +- radiant/ui/filterdialog/FilterDialog.cpp | 34 ++++++------------ radiant/ui/filters/FilterContextMenu.cpp | 13 +++---- radiant/ui/filters/FilterContextMenu.h | 12 +++---- radiant/ui/filters/FilterMenu.cpp | 6 ++-- radiant/ui/filters/FilterMenu.h | 7 ++-- radiant/ui/menu/FiltersMenu.cpp | 45 +++++++----------------- 9 files changed, 61 insertions(+), 108 deletions(-) diff --git a/include/ifilter.h b/include/ifilter.h index 2039dd1403..4b615f8e82 100644 --- a/include/ifilter.h +++ b/include/ifilter.h @@ -70,17 +70,6 @@ class FilterRule }; typedef std::vector FilterRules; -/** Visitor interface for evaluating the available filters in the - * FilterSystem. - */ -struct IFilterVisitor -{ - virtual ~IFilterVisitor() {} - - // Visit function - virtual void visit(const std::string& filterName) = 0; -}; - const char* const MODULE_FILTERSYSTEM = "FilterSystem"; // Forward declaration @@ -113,13 +102,13 @@ class FilterSystem : */ virtual void updateSubgraph(const scene::INodePtr& root) = 0; - /** Visit the available filters, passing each filter's text - * name to the visitor. + /** + * Visit the available filters, passing each filter's text name to the visitor. * * @param visitor - * Visitor class implementing the IFilterVisitor interface. + * Function object called with the filter name as argument. */ - virtual void forEachFilter(IFilterVisitor& visitor) = 0; + virtual void forEachFilter(const std::function& func) = 0; /** Set the state of the named filter. * @@ -217,7 +206,8 @@ class FilterSystem : virtual bool setFilterRules(const std::string& filter, const FilterRules& ruleSet) = 0; }; -inline FilterSystem& GlobalFilterSystem() { +inline FilterSystem& GlobalFilterSystem() +{ // Cache the reference locally static FilterSystem& _filterSystem( *std::static_pointer_cast( diff --git a/radiant/filters/BasicFilterSystem.cpp b/radiant/filters/BasicFilterSystem.cpp index f2f65fc3af..e2f58ae4c4 100644 --- a/radiant/filters/BasicFilterSystem.cpp +++ b/radiant/filters/BasicFilterSystem.cpp @@ -195,9 +195,7 @@ void BasicFilterSystem::addFiltersFromXML(const xml::NodeList& nodes, bool readO } // Add this XMLFilter to the list of available filters - XMLFilter::Ptr inserted = _availableFilters.insert( - std::make_pair(filterName, filter) - ).first->second; + XMLFilter::Ptr inserted = _availableFilters.emplace(filterName, filter).first->second; bool filterShouldBeActive = activeFilterNames.find(filterName) != activeFilterNames.end(); @@ -207,7 +205,7 @@ void BasicFilterSystem::addFiltersFromXML(const xml::NodeList& nodes, bool readO if (filterShouldBeActive) { adapter->setFilterState(true); - _activeFilters.insert(std::make_pair(filterName, inserted)); + _activeFilters.emplace(filterName, inserted); } } } @@ -221,8 +219,8 @@ XmlFilterEventAdapter::Ptr BasicFilterSystem::ensureEventAdapter(XMLFilter& filt return existing->second; } - auto result = _eventAdapters.emplace(std::make_pair(filter.getName(), - std::make_shared(filter))); + auto result = _eventAdapters.emplace(filter.getName(), + std::make_shared(filter)); return result.first->second; } @@ -299,12 +297,12 @@ void BasicFilterSystem::update() updateScene(); } -void BasicFilterSystem::forEachFilter(IFilterVisitor& visitor) { - +void BasicFilterSystem::forEachFilter(const std::function& func) +{ // Visit each filter on the list, passing the name to the visitor for (const auto& pair : _availableFilters) { - visitor.visit(pair.first); + func(pair.first); } } @@ -328,7 +326,7 @@ void BasicFilterSystem::setFilterState(const std::string& filter, bool state) if (state) { // Copy the filter to the active filters list - _activeFilters.insert(std::make_pair(filter, _availableFilters.find(filter)->second)); + _activeFilters.emplace(filter, _availableFilters.find(filter)->second); } else { @@ -387,7 +385,7 @@ bool BasicFilterSystem::addFilter(const std::string& filterName, const FilterRul } auto filter = std::make_shared(filterName, false); - auto result = _availableFilters.insert(std::make_pair(filterName, filter)); + auto result = _availableFilters.emplace(filterName, filter); // Apply the ruleset filter->setRules(ruleSet); @@ -477,18 +475,18 @@ bool BasicFilterSystem::renameFilter(const std::string& oldFilterName, const std // Re-insert the event adapter using a new key auto adapterPtr = adapter->second; _eventAdapters.erase(adapter); - adapter = _eventAdapters.insert(std::make_pair(newFilterName, adapterPtr)).first; + adapter = _eventAdapters.emplace(newFilterName, adapterPtr).first; adapter->second->setFilterState(wasActive); } // Insert the new filter into the table - auto result = _availableFilters.insert(std::make_pair(newFilterName, f->second)); + auto result = _availableFilters.emplace(newFilterName, f->second); // If this filter is in our active set, enable it if (wasActive) { - _activeFilters.insert(std::make_pair(newFilterName, f->second)); + _activeFilters.emplace(newFilterName, f->second); } // Remove the old filter from the filtertable @@ -526,7 +524,7 @@ bool BasicFilterSystem::isVisible(const FilterRule::Type type, const std::string } // Cache the result and return to caller - _visibilityCache.insert(std::make_pair(name, visFlag)); + _visibilityCache.emplace(name, visFlag); return visFlag; } diff --git a/radiant/filters/BasicFilterSystem.h b/radiant/filters/BasicFilterSystem.h index 1dab042566..9525c490a3 100644 --- a/radiant/filters/BasicFilterSystem.h +++ b/radiant/filters/BasicFilterSystem.h @@ -74,7 +74,7 @@ class BasicFilterSystem : void updateSubgraph(const scene::INodePtr& root) override; // Filter system visit function - void forEachFilter(IFilterVisitor& visitor) override; + void forEachFilter(const std::function& func) override; std::string getFilterEventName(const std::string& filter) override; diff --git a/radiant/ui/filterdialog/FilterDialog.cpp b/radiant/ui/filterdialog/FilterDialog.cpp index 5307b5d484..0a8f735767 100644 --- a/radiant/ui/filterdialog/FilterDialog.cpp +++ b/radiant/ui/filterdialog/FilterDialog.cpp @@ -92,33 +92,19 @@ void FilterDialog::loadFilters() // Clear first, before population _filters.clear(); - // Local helper class to populate the map - class FilterMapPopulator : - public IFilterVisitor + GlobalFilterSystem().forEachFilter([&] (const std::string& filterName) { - FilterMap& _target; - public: - FilterMapPopulator(FilterMap& target) : - _target(target) - {} + // Get the properties + bool state = GlobalFilterSystem().getFilterState(filterName); + bool readOnly = GlobalFilterSystem().filterIsReadOnly(filterName); - void visit(const std::string& filterName) - { - // Get the properties - bool state = GlobalFilterSystem().getFilterState(filterName); - bool readOnly = GlobalFilterSystem().filterIsReadOnly(filterName); - - std::pair result = _target.insert( - FilterMap::value_type(filterName, FilterPtr(new Filter(filterName, state, readOnly))) - ); - - // Copy the ruleset from the given filter - result.first->second->rules = GlobalFilterSystem().getRuleSet(filterName); - } - - } populator(_filters); + auto result = _filters.emplace( + filterName, std::make_shared(filterName, state, readOnly) + ); - GlobalFilterSystem().forEachFilter(populator); + // Copy the ruleset from the given filter + result.first->second->rules = GlobalFilterSystem().getRuleSet(filterName); + }); } void FilterDialog::update() diff --git a/radiant/ui/filters/FilterContextMenu.cpp b/radiant/ui/filters/FilterContextMenu.cpp index 8032a7a253..5275011b2e 100644 --- a/radiant/ui/filters/FilterContextMenu.cpp +++ b/radiant/ui/filters/FilterContextMenu.cpp @@ -20,7 +20,7 @@ FilterContextMenu::FilterContextMenu(OnSelectionFunc& onSelection) : void FilterContextMenu::populate() { // Clear all existing items - for (const MenuItemIdToLayerMapping::value_type& i : _menuItemMapping) + for (const auto& pair : _menuItemMapping) { wxMenu* parentMenu = GetParent(); @@ -31,19 +31,20 @@ void FilterContextMenu::populate() if (parentMenu != nullptr) { - parentMenu->Unbind(wxEVT_MENU, &FilterContextMenu::onActivate, this, i.first); + parentMenu->Unbind(wxEVT_MENU, &FilterContextMenu::onActivate, this, pair.first); } - Delete(i.first); + Delete(pair.first); } _menuItemMapping.clear(); - // Populate the map with all layer names and IDs - GlobalFilterSystem().forEachFilter(*this); + // Populate the map with all filter names + GlobalFilterSystem().forEachFilter( + std::bind(&FilterContextMenu::visitFilter, this, std::placeholders::_1)); } -void FilterContextMenu::visit(const std::string& filterName) +void FilterContextMenu::visitFilter(const std::string& filterName) { // Create a new menuitem wxMenuItem* menuItem = new wxutil::IconTextMenuItem(filterName, FILTER_ICON); diff --git a/radiant/ui/filters/FilterContextMenu.h b/radiant/ui/filters/FilterContextMenu.h index a4009280cd..aa61be3576 100644 --- a/radiant/ui/filters/FilterContextMenu.h +++ b/radiant/ui/filters/FilterContextMenu.h @@ -12,8 +12,7 @@ namespace ui { class FilterContextMenu : - public wxMenu, - public IFilterVisitor + public wxMenu { public: // The function to be called on menu selection, the name of the @@ -23,19 +22,18 @@ class FilterContextMenu : private: OnSelectionFunc _onSelection; - typedef std::map MenuItemIdToLayerMapping; - MenuItemIdToLayerMapping _menuItemMapping; + typedef std::map MenuItemIdToFilterMapping; + MenuItemIdToFilterMapping _menuItemMapping; public: FilterContextMenu(OnSelectionFunc& onSelection); - // IFilterVisitor implementation - void visit(const std::string& filterName) override; - // Loads filer names into the menu, clears existing items first void populate(); private: + void visitFilter(const std::string& filterName); + // wx Callback for menu selections void onActivate(wxCommandEvent& ev); }; diff --git a/radiant/ui/filters/FilterMenu.cpp b/radiant/ui/filters/FilterMenu.cpp index baae1d575a..0afaccfc02 100644 --- a/radiant/ui/filters/FilterMenu.cpp +++ b/radiant/ui/filters/FilterMenu.cpp @@ -18,7 +18,7 @@ FilterMenu::FilterMenu() : _menu(new wxutil::PopupMenu) { // Visit the filters in the FilterSystem to populate the menu - GlobalFilterSystem().forEachFilter(*this); + GlobalFilterSystem().forEachFilter(std::bind(&FilterMenu::visitFilter, this, std::placeholders::_1)); } FilterMenu::~FilterMenu() @@ -36,7 +36,7 @@ FilterMenu::~FilterMenu() _menu = nullptr; } -void FilterMenu::visit(const std::string& filterName) +void FilterMenu::visitFilter(const std::string& filterName) { wxMenuItem* item = _menu->Append(new wxutil::IconTextMenuItem(filterName, MENU_ICON)); item->SetCheckable(true); @@ -50,7 +50,7 @@ void FilterMenu::visit(const std::string& filterName) event->connectMenuItem(item); } - _filterItems.insert(std::make_pair(eventName, item)); + _filterItems.emplace(eventName, item); } wxMenu* FilterMenu::getMenuWidget() diff --git a/radiant/ui/filters/FilterMenu.h b/radiant/ui/filters/FilterMenu.h index e4010d8199..8646982b52 100644 --- a/radiant/ui/filters/FilterMenu.h +++ b/radiant/ui/filters/FilterMenu.h @@ -15,8 +15,7 @@ namespace ui * can be packed into a parent container widget using the getMenuWidget(). */ class FilterMenu : - public IFilterMenu, - public IFilterVisitor + public IFilterMenu { private: std::map _filterItems; @@ -33,8 +32,8 @@ class FilterMenu : // ready for packing into a menu bar. wxMenu* getMenuWidget(); - // IFilterVisitor implementation - void visit(const std::string& filterName); +private: + void visitFilter(const std::string& filterName); }; } // namespace diff --git a/radiant/ui/menu/FiltersMenu.cpp b/radiant/ui/menu/FiltersMenu.cpp index d210b345fc..0fcd5907fe 100644 --- a/radiant/ui/menu/FiltersMenu.cpp +++ b/radiant/ui/menu/FiltersMenu.cpp @@ -4,43 +4,17 @@ #include "ifilter.h" #include "iuimanager.h" -namespace ui { +namespace ui +{ - namespace { + namespace + { // greebo: These are used for the DarkRadiant main menu const std::string MENU_NAME = "main"; const std::string MENU_INSERT_BEFORE = MENU_NAME + "/map"; const std::string MENU_FILTERS_NAME = "filters"; const std::string MENU_PATH = MENU_NAME + "/" + MENU_FILTERS_NAME; const std::string MENU_ICON = "iconFilter16.png"; - - // Local visitor class to populate the filters menu - class MenuPopulatingVisitor : - public IFilterVisitor - { - // The path under which the items get added. - std::string _targetPath; - public: - // Pass the target menu path to the constructor - MenuPopulatingVisitor(const std::string& targetPath) : - _targetPath(targetPath) - {} - - // Visitor function - void visit(const std::string& filterName) - { - // Get the menu manager - IMenuManager& menuManager = GlobalUIManager().getMenuManager(); - - std::string eventName = - GlobalFilterSystem().getFilterEventName(filterName); - - // Create the menu item - menuManager.add(_targetPath, _targetPath + "_" + filterName, - menuItem, filterName, - MENU_ICON, eventName); - } - }; } // Construct menu items @@ -57,8 +31,15 @@ void FiltersMenu::addItemsToMainMenu() ui::menuFolder, "Fi<er", "", ""); // empty icon, empty event // Visit the filters in the FilterSystem to populate the menu - MenuPopulatingVisitor visitor(MENU_PATH); - GlobalFilterSystem().forEachFilter(visitor); + GlobalFilterSystem().forEachFilter([&](const std::string& filterName) + { + std::string eventName = GlobalFilterSystem().getFilterEventName(filterName); + + // Create the menu item + menuManager.add(MENU_PATH, MENU_PATH + "_" + filterName, + menuItem, filterName, + MENU_ICON, eventName); + }); menuManager.add(MENU_PATH, "_FiltersSep1", menuSeparator, "", "", ""); menuManager.add(MENU_PATH, "ActivateAllFilters", menuItem, _("Activate &all Filters"), MENU_ICON, "ActivateAllFilters");