From 0c79e230b9c0f32d4ad080629e6cd14b9dc50b9b Mon Sep 17 00:00:00 2001
From: Matteo Cypriani <matteo.cypriani@uqat.ca>
Date: Wed, 2 Oct 2013 17:33:53 -0400
Subject: [PATCH] [Positioner] Rework Building::add_area()

Don't take a pointer reference as argument, throw an exception instead
of deleting the area, clarify comments.
---
 owlps-positioner/building.cc          | 30 +++++++++++++++------------
 owlps-positioner/building.hh          |  2 +-
 owlps-positioner/topologyreadercsv.cc |  9 +++++++-
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/owlps-positioner/building.cc b/owlps-positioner/building.cc
index 89f55c2..1a95214 100644
--- a/owlps-positioner/building.cc
+++ b/owlps-positioner/building.cc
@@ -16,6 +16,7 @@
 #include "area.hh"
 #include "waypoint.hh"
 #include "stock.hh"
+#include "posexcept.hh"
 
 
 using namespace std ;
@@ -49,13 +50,16 @@ Building::~Building()
 
 
 /**
- * @param[in,out] area A pointer to the Area to add. If it is NULL,
- * nothing will be added. If the Area it points to already exists in
- * #areas, it is deleted and set to NULL, unless it is the same pointer
- * (see the code to understand!); hmm, maybe we should handle that with
- * exceptions…
+ * If `area` is NULL, nothing is done.
+ * If `area` is a pointer to an Area that is already stored in #areas,
+ * nothing is done.
+ * If `area` has the same name as (but is not identical to) an Area that
+ * is already stored in #areas, an exception is thrown.
+ *
+ * @param area A pointer to the Area to add.
+ * @throws element_already_exists
  */
-void Building::add_area(Area *&area)
+void Building::add_area(const Area *const area)
 {
   if (area == NULL)
     return ;
@@ -66,15 +70,15 @@ void Building::add_area(Area *&area)
   auto i = areas.find(area_name) ;
   if (i != areas.end())
     {
-      // Do not delete if area points to the already stored Area
-      if (i->second != area)
-        {
-          delete area ;
-          area = NULL ;
-        }
-      return ;
+      // Just return if area points to the already stored Area
+      if (i->second == area)
+        return ;
+      // Otherwise throw an exception
+      throw element_already_exists(
+        "an Area named \""+ area_name +"\" is already stored") ;
     }
 
+  // Store the area
   areas[area_name] = area ;
 }
 
diff --git a/owlps-positioner/building.hh b/owlps-positioner/building.hh
index a19bb28..1a24a3d 100644
--- a/owlps-positioner/building.hh
+++ b/owlps-positioner/building.hh
@@ -55,7 +55,7 @@ public:
   //@{
   void set_name(const std::string &_name) ;
   /// Adds an Area to the [list of areas](@ref #areas)
-  void add_area(Area *&area) ;
+  void add_area(const Area *const area) ;
   /// Adds a Waypoint to the [list of waypoints](@ref #waypoints)
   void add_waypoint(Waypoint *const wp) ;
   //@}
diff --git a/owlps-positioner/topologyreadercsv.cc b/owlps-positioner/topologyreadercsv.cc
index 6b83817..2327106 100644
--- a/owlps-positioner/topologyreadercsv.cc
+++ b/owlps-positioner/topologyreadercsv.cc
@@ -68,7 +68,14 @@ void TopologyReaderCSV::process_area_line()
     throw malformed_input_data("Cannot read area max coordinates!") ;
 
   Area *area = new Area(&building, name, p_min, p_max) ;
-  building.add_area(area) ;
+  try
+    {
+      building.add_area(area) ;
+    }
+  catch (element_already_exists e)
+    {
+      delete area ;
+    }
 }