Symbol classes: Review cloning pattern

Re-implement duplicate() using and protected copy constructors and
covariant return type.
Make color replacement an explicit feature, taking a reference.
Rearrange members to minimize padding.
Use std algorithms where possible.
Use std::array for Symbol::number in order to facilitate copying.
Modifies symbol number loading, resolves #426.

src/core/map.cpp

@@ -1657,7 +1657,7 @@ QHash<const Symbol*, Symbol*> Map::importSymbols(
 				}
 			}
 
-			auto new_symbol = symbol->duplicate(&color_map);
+			auto new_symbol = symbol->duplicate(color_map);
 			out_pointermap.insert(symbol, new_symbol);
 			created_symbols.push_back(new_symbol);
 		}

src/core/symbols/area_symbol.cpp

@@ -1,6 +1,6 @@
 /*
  *    Copyright 2012, 2013 Thomas Schöps
- *    Copyright 2012-2017 Kai Pastor
+ *    Copyright 2012-2018 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -567,6 +567,19 @@ AreaSymbol::AreaSymbol() noexcept
 	// nothing else
 }
 
+AreaSymbol::AreaSymbol(const AreaSymbol& proto)
+: Symbol { proto }
+, patterns { proto.patterns }
+, color { proto.color }
+, minimum_area { proto.minimum_area }
+{
+	for (auto& new_pattern : patterns)
+	{
+		if (new_pattern.type == FillPattern::PointPattern)
+			new_pattern.point = new_pattern.point->duplicate();
+	}
+}
+
 AreaSymbol::~AreaSymbol()
 {
 	for (auto& pattern : patterns)
@@ -576,23 +589,22 @@ AreaSymbol::~AreaSymbol()
 	}
 }
 
-Symbol* AreaSymbol::duplicate(const MapColorMap* color_map) const
+
+AreaSymbol* AreaSymbol::duplicate() const
 {
-	auto new_area = new AreaSymbol();
-	new_area->duplicateImplCommon(this);
-	new_area->color = color_map ? color_map->value(color) : color;
-	new_area->minimum_area = minimum_area;
-	new_area->patterns = patterns;
-	for (auto& new_pattern : new_area->patterns)
-	{
-		if (new_pattern.type == FillPattern::PointPattern)
-			new_pattern.point = static_cast<PointSymbol*>(new_pattern.point->duplicate(color_map));
-		else if (new_pattern.type == FillPattern::LinePattern && color_map)
-			new_pattern.line_color = color_map->value(new_pattern.line_color);
-	}
+	return new AreaSymbol(*this);
+}
+
+
+AreaSymbol* AreaSymbol::duplicate(const MapColorMap& color_map) const
+{
+	auto new_area = new AreaSymbol(*this);
+	new_area->replaceColors(color_map);
 	return new_area;
 }
 
+
+
 void AreaSymbol::createRenderables(
         const Object *object,
         const VirtualCoordVector &coords,
@@ -725,6 +737,24 @@ const MapColor* AreaSymbol::guessDominantColor() const
 }
 
 
+void AreaSymbol::replaceColors(const MapColorMap& color_map)
+{
+	color = color_map.value(color);
+	for (auto& pattern : patterns)
+	{
+		switch (pattern.type)
+		{
+		case FillPattern::LinePattern:
+			pattern.line_color = color_map.value(pattern.line_color);
+			break;
+		case FillPattern::PointPattern:
+			pattern.point->replaceColors(color_map);
+			break;
+		}
+	}
+}
+
+
 
 void AreaSymbol::scale(double factor)
 {

src/core/symbols/area_symbol.h

@@ -1,6 +1,6 @@
 /*
  *    Copyright 2012, 2013 Thomas Schöps
- *    Copyright 2012-2017 Kai Pastor
+ *    Copyright 2012-2018 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -233,8 +233,15 @@ friend class OCAD8FileImport;
 	};
 
 	AreaSymbol() noexcept;
+
+protected:
+	explicit AreaSymbol(const AreaSymbol& proto);
+
+public:
 	~AreaSymbol() override;
-	Symbol* duplicate(const MapColorMap* color_map = nullptr) const override;
+
+	AreaSymbol* duplicate() const override;
+	AreaSymbol* duplicate(const MapColorMap& color_map) const override;
 
 	void createRenderables(
 	        const Object *object,
@@ -266,6 +273,7 @@ friend class OCAD8FileImport;
 	void colorDeleted(const MapColor* color) override;
 	bool containsColor(const MapColor* color) const override;
 	const MapColor* guessDominantColor() const override;
+	void replaceColors(const MapColorMap& color_map) override;
 	void scale(double factor) override;
 
 	qreal dimensionForIcon() const override;
@@ -292,9 +300,9 @@ friend class OCAD8FileImport;
 	 */
 	bool equalsImpl(const Symbol* other, Qt::CaseSensitivity case_sensitivity) const override;
 
+	std::vector<FillPattern> patterns;
 	const MapColor* color;
 	int minimum_area;	// in mm^2 // FIXME: unit (factor) wrong
-	std::vector<FillPattern> patterns;
 };
 
 

src/core/symbols/combined_symbol.cpp

@@ -1,6 +1,6 @@
 /*
  *    Copyright 2012, 2013 Thomas Schöps
- *    Copyright 2012-2017 Kai Pastor
+ *    Copyright 2012-2018 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -43,39 +43,47 @@
 namespace OpenOrienteering {
 
 CombinedSymbol::CombinedSymbol()
-: Symbol{Symbol::Combined}
-, private_parts{ false, false }
-, parts{ private_parts.size(), nullptr }
+: Symbol { Symbol::Combined }
+, private_parts { false, false }
+, parts { private_parts.size(), nullptr }
 {
 	Q_ASSERT(private_parts.size() == parts.size());
 	// nothing else
 }
 
+
+CombinedSymbol::CombinedSymbol(const CombinedSymbol& proto)
+: Symbol { proto }
+, private_parts { proto.private_parts }
+, parts { proto.parts }
+{
+	std::transform(begin(parts), end(parts), begin(private_parts), begin(parts), [](auto part, auto is_private) {
+		return (part && is_private) ? part->duplicate() : part;
+	});
+}
+
+
 CombinedSymbol::~CombinedSymbol()
 {
-	auto is_private = begin(private_parts);
-	for (auto subsymbol : parts)
-	{
-		if (*is_private)
-			delete subsymbol;
-		++is_private;
-	};
+	std::transform(begin(parts), end(parts), begin(private_parts), begin(parts), [](auto part, auto is_private) {
+		if (part && is_private)
+			delete part;
+		return nullptr;
+	});
 }
 
-Symbol* CombinedSymbol::duplicate(const MapColorMap* color_map) const
+
+CombinedSymbol* CombinedSymbol::duplicate() const
 {
-	auto new_symbol = new CombinedSymbol();
-	new_symbol->duplicateImplCommon(this);
-	new_symbol->parts = parts;
-	new_symbol->private_parts = private_parts;
-	auto is_private = begin(new_symbol->private_parts);
-	for (auto& subsymbol : new_symbol->parts)
-	{
-		if (*is_private)
-			subsymbol = subsymbol->duplicate(color_map);
-		++is_private;
-	}
-	return new_symbol;
+	return new CombinedSymbol(*this);
+}
+
+
+CombinedSymbol* CombinedSymbol::duplicate(const MapColorMap& color_map) const
+{
+	auto new_combined = new CombinedSymbol(*this);
+	new_combined->replaceColors(color_map);
+	return new_combined;
 }
 
 
@@ -162,6 +170,18 @@ const MapColor* CombinedSymbol::guessDominantColor() const
 	return dominant_color;
 }
 
+
+void CombinedSymbol::replaceColors(const MapColorMap& color_map)
+{
+	std::transform(begin(parts), end(parts), begin(private_parts), begin(parts), [&color_map](auto part, auto is_private) {
+		if (part && is_private)
+			const_cast<Symbol*>(part)->replaceColors(color_map);
+		return part;
+	});
+}
+
+
+
 bool CombinedSymbol::symbolChanged(const Symbol* old_symbol, const Symbol* new_symbol)
 {
 	bool have_symbol = false;

src/core/symbols/combined_symbol.h

@@ -1,6 +1,6 @@
 /*
  *    Copyright 2012, 2013 Thomas Schöps
- *    Copyright 2012-2017 Kai Pastor
+ *    Copyright 2012-2018 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -61,8 +61,15 @@ friend class PointSymbolEditorWidget;
 friend class OCAD8FileImport;
 public:
 	CombinedSymbol();
+
+protected:
+	explicit CombinedSymbol(const CombinedSymbol& proto);
+
+public:
 	~CombinedSymbol() override;
-	Symbol* duplicate(const MapColorMap* color_map = nullptr) const override;
+
+	CombinedSymbol* duplicate() const override;
+	CombinedSymbol* duplicate(const MapColorMap& color_map) const override;
 
 	bool validate() const override;
 
@@ -81,6 +88,7 @@ friend class OCAD8FileImport;
 	void colorDeleted(const MapColor* color) override;
 	bool containsColor(const MapColor* color) const override;
 	const MapColor* guessDominantColor() const override;
+	void replaceColors(const MapColorMap& color_map) override;
 	bool symbolChanged(const Symbol* old_symbol, const Symbol* new_symbol) override;
 	bool containsSymbol(const Symbol* symbol) const override;
 	void scale(double factor) override;