#5122: Refactor IFilterVisitor to be a functor, replace some insert(s…

…td::make_pair()) calls with emplace.

include/ifilter.h

@@ -70,17 +70,6 @@ class FilterRule
 };
 typedef std::vector<FilterRule> 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<void(const std::string& name)>& 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<FilterSystem>(

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<XmlFilterEventAdapter>(filter)));
+	auto result = _eventAdapters.emplace(filter.getName(), 
+		std::make_shared<XmlFilterEventAdapter>(filter));
 
 	return result.first->second;
 }
@@ -299,12 +297,12 @@ void BasicFilterSystem::update()
 	updateScene();
 }
 
-void BasicFilterSystem::forEachFilter(IFilterVisitor& visitor) {
-
+void BasicFilterSystem::forEachFilter(const std::function<void(const std::string & name)>& 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<XMLFilter>(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;
 }

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<void(const std::string & name)>& func) override;
 
 	std::string getFilterEventName(const std::string& filter) override;
 

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<FilterMap::iterator, bool> 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<Filter>(filterName, state, readOnly)
+		);
 
-	GlobalFilterSystem().forEachFilter(populator);
+		// Copy the ruleset from the given filter
+		result.first->second->rules = GlobalFilterSystem().getRuleSet(filterName);
+	});
 }
 
 void FilterDialog::update()

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);

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<int, std::string> MenuItemIdToLayerMapping;
-	MenuItemIdToLayerMapping _menuItemMapping;
+	typedef std::map<int, std::string> 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);
 };

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()

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<std::string, wxMenuItem*> _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

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&lter", "", ""); // 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");