Return the layer list by ref-to-const

codereader · Mar 12, 2020 · bc1727f · bc1727f
1 parent f37dc03
commit bc1727f
Show file tree

Hide file tree

Showing 8 changed files with 11 additions and 12 deletions.
diff --git a/include/ilayer.h b/include/ilayer.h
@@ -43,7 +43,7 @@ class Layered
     /**
      * Return the set of layers to which this object is assigned.
      */
-    virtual LayerList getLayers() const = 0;
+    virtual const LayerList& getLayers() const = 0;
 
 	/**
 	 * greebo: This assigns the given node to the given set of layers. Any previous

diff --git a/libs/scene/LayerValidityCheckWalker.h b/libs/scene/LayerValidityCheckWalker.h
@@ -29,15 +29,15 @@ class LayerValidityCheckWalker :
 	// Returns true if the node has been "fixed"
 	static bool ProcessNode(const INodePtr& node)
 	{
-		LayerList list = node->getLayers();
+		LayerList list = node->getLayers(); // create a copy of the list
 
 		bool fixed = false;
 
-		for (LayerList::iterator i = list.begin(); i != list.end(); ++i)
+		for (auto id : list)
 		{
-			if (!GlobalLayerSystem().layerExists(*i))
+			if (!GlobalLayerSystem().layerExists(id))
 			{
-				node->removeFromLayer(*i);
+				node->removeFromLayer(id);
 				fixed = true;
 			}
 		}

diff --git a/libs/scene/Node.cpp b/libs/scene/Node.cpp
@@ -154,7 +154,7 @@ void Node::removeFromLayer(int layerId)
 	}
 }
 
-LayerList Node::getLayers() const
+const LayerList& Node::getLayers() const
 {
 	return _layers;
 }

diff --git a/libs/scene/Node.h b/libs/scene/Node.h
@@ -106,7 +106,7 @@ class Node :
 	virtual void addToLayer(int layerId) override;
     virtual void removeFromLayer(int layerId) override;
 	virtual void moveToLayer(int layerId) override;
-    virtual LayerList getLayers() const override;
+    virtual const LayerList& getLayers() const override;
 	virtual void assignToLayers(const LayerList& newLayers) override;
 
 	virtual void addChildNode(const INodePtr& node) override;

diff --git a/radiant/layers/LayerSystem.cpp b/radiant/layers/LayerSystem.cpp
@@ -388,8 +388,7 @@ void LayerSystem::removeSelectionFromLayer(int layerID) {
 bool LayerSystem::updateNodeVisibility(const scene::INodePtr& node)
 {
 	// Get the list of layers the node is associated with
-	// greebo: FIXME: Check if returning the LayerList by value is taxing.
-	LayerList layers = node->getLayers();
+	const auto& layers = node->getLayers();
 
 	// We start with the assumption that a node is hidden
 	node->enable(Node::eLayered);

diff --git a/radiant/layers/LayerUsageBreakdown.cpp b/radiant/layers/LayerUsageBreakdown.cpp
@@ -12,7 +12,7 @@ namespace
 {
 	void addNodeMapping(LayerUsageBreakdown& bd, const scene::INodePtr& node)
 	{
-		scene::LayerList layers = node->getLayers();
+		const auto& layers = node->getLayers();
 
 		for (int layerId : layers)
 		{

diff --git a/radiant/layers/SetLayerSelectedWalker.h b/radiant/layers/SetLayerSelectedWalker.h
@@ -33,7 +33,7 @@ class SetLayerSelectedWalker :
 			return true;
 		}
 
-		LayerList layers = node->getLayers();
+		const auto& layers = node->getLayers();
 
 		if (layers.find(_layer) != layers.end())
 		{

diff --git a/radiant/map/format/portable/PortableMapWriter.cpp b/radiant/map/format/portable/PortableMapWriter.cpp
@@ -279,7 +279,7 @@ void PortableMapWriter::endWritePatch(const IPatchNodePtr& patch, std::ostream&
 
 void PortableMapWriter::appendLayerInformation(xml::Node& xmlNode, const scene::INodePtr& sceneNode)
 {
-	auto layers = sceneNode->getLayers();
+	const auto& layers = sceneNode->getLayers();
 	auto layersTag = xmlNode.createChild(TAG_OBJECT_LAYERS);
 
 	// Write the list of node IDs