Further refactoring for better safety and maintainability

components/spit/src/SPITObject.cpp

@@ -14,16 +14,17 @@
 
 #include "SPITObject.h"
 #include "SPITSocket.h"
+#include "esp_log.h"
 
-unordered_map<string, SPITObject *> SPITObject::sSpitObjects;
-unordered_map<string, SPITObject *> SPITObject::sSpitObjectsRemote;
+unordered_map<string, unique_ptr<SPITObject>> SPITObject::sSpitObjects;
+unordered_map<string, unique_ptr<SPITObject>> SPITObject::sSpitObjectsRemote;
 
 SPITObject *SPITObject::getSPITObject(const string &id) {
     auto i = sSpitObjects.find(id);
     if (i == sSpitObjects.end()) {
         return nullptr;
     }
-    return i->second;
+    return i->second.get();
 }
 /**
  * Returns the reference of the SPITObject with ID == id.
@@ -40,7 +41,7 @@ SPITObject *SPITObject::getSPITObjectRemote(const string &id) {
     if (i == sSpitObjectsRemote.end()) {
         return nullptr;
     }
-    return i->second;
+    return i->second.get();
 }
 /**
  * Returns the reference of the remote SPITObject with ID == id.
@@ -52,16 +53,18 @@ const SPITObject &SPITObject::getSPITObjectRemoteRef(const string &id) {
     return *sSpitObjectsRemote.at(id);
 }
 
-void SPITObject::addSPITObject(SPITObject &spitObject) {
-    sSpitObjects[spitObject.getID()] = &spitObject;
+void SPITObject::addSPITObject(unique_ptr<SPITObject> spitObject) {
+    ESP_LOGD("OBJ", "adding spitObject %s/%s", spitObject->getID().c_str(), spitObject->getName().c_str());
+    sSpitObjects[spitObject->getID()] = move(spitObject);
 }
-void SPITObject::addSPITObjectRemote(SPITObject &spitObject) {
-    sSpitObjectsRemote[spitObject.getID()] = &spitObject;
+void SPITObject::addSPITObjectRemote(unique_ptr<SPITObject> spitObject) {
+    ESP_LOGD("OBJ", "adding spitObject %s/%s", spitObject->getID().c_str(), spitObject->getName().c_str());
+    sSpitObjectsRemote[spitObject->getID()] = move(spitObject);
 }
 bool SPITObject::removeSPITObject(const SPITObject &spitObject) {
     return removeSPITObject(spitObject.getID());
 }
-bool SPITObject::removeSPITObject(const string & spitObjectID) {
+bool SPITObject::removeSPITObject(const string &spitObjectID) {
     auto i = sSpitObjects.find(spitObjectID);
     if (i == sSpitObjects.end()) {
         return false;
@@ -72,7 +75,7 @@ bool SPITObject::removeSPITObject(const string & spitObjectID) {
 bool SPITObject::removeSPITObjectRemote(const SPITObject &spitObject) {
     return removeSPITObjectRemote(spitObject.getID());
 }
-bool SPITObject::removeSPITObjectRemote(const string & spitObjectID) {
+bool SPITObject::removeSPITObjectRemote(const string &spitObjectID) {
     auto i = sSpitObjectsRemote.find(spitObjectID);
     if (i == sSpitObjectsRemote.end()) {
         return false;
@@ -83,8 +86,8 @@ bool SPITObject::removeSPITObjectRemote(const string & spitObjectID) {
 
 SPITObject *SPITObject::getSPITObject(int p_Index) {
     if (p_Index >= 0 && p_Index < sSpitObjects.size()) {
-        unordered_map<string, SPITObject *>::iterator i = next(sSpitObjects.begin(), p_Index);
-        return i->second;
+        auto i = next(sSpitObjects.begin(), p_Index);
+        return i->second.get();
     }
     return nullptr;
 }
@@ -96,7 +99,7 @@ SPITObject *SPITObject::getSPITObject(int p_Index) {
  */
 const SPITObject &SPITObject::getSPITObjectRef(int p_Index) {
     if (p_Index >= 0 && p_Index < sSpitObjects.size()) {
-        unordered_map<string, SPITObject *>::iterator i = next(sSpitObjects.begin(), p_Index);
+        auto i = next(sSpitObjects.begin(), p_Index);
         return *i->second;
     } else {
         throw "SPITObject index out of range";
@@ -104,8 +107,8 @@ const SPITObject &SPITObject::getSPITObjectRef(int p_Index) {
 }
 SPITObject *SPITObject::getSPITObjectRemote(int p_Index) {
     if (p_Index >= 0 && p_Index < sSpitObjectsRemote.size()) {
-        unordered_map<string, SPITObject *>::iterator i = next(sSpitObjectsRemote.begin(), p_Index);
-        return i->second;
+        auto i = next(sSpitObjectsRemote.begin(), p_Index);
+        return i->second.get();
     } else {
         return nullptr;
     }
@@ -118,76 +121,48 @@ SPITObject *SPITObject::getSPITObjectRemote(int p_Index) {
  */
 const SPITObject &SPITObject::getSPITObjectRemoteRef(int p_Index) {
     if (p_Index >= 0 && p_Index < sSpitObjectsRemote.size()) {
-        unordered_map<string, SPITObject *>::iterator i = next(sSpitObjectsRemote.begin(), p_Index);
+        auto i = next(sSpitObjectsRemote.begin(), p_Index);
         return *i->second;
     } else {
         throw "Remote SPITObject index out of range";
     }
 }
 
-void SPITObject::clearSPITObjects() {
-    SPITObject *ptrSPITObject;
-
-    for (auto i = sSpitObjects.begin(); i != sSpitObjects.end(); i++) {
-        ptrSPITObject = i->second;
-        i = sSpitObjects.erase(i);
-        delete ptrSPITObject;
-    }
-}
-void SPITObject::clearSPITObjectsRemote() {
-    SPITObject *ptrSPITObject;
-
-    for (auto i = sSpitObjectsRemote.begin(); i != sSpitObjectsRemote.end(); i++) {
-        ptrSPITObject = i->second;
-        i = sSpitObjectsRemote.erase(i);
-        delete ptrSPITObject;
-    }
-}
-
 /**
  * clears all unconfirmed SPITObjects not created by this SPIT_Client.
  */
 void SPITObject::clearUnconfirmedSPITObjects() {
     for (auto i = sSpitObjects.begin(); i != sSpitObjects.end(); i++) {
-        SPITObject *ptrSPITObject = i->second;
+        SPITObject *ptrSPITObject = i->second.get();
         if (ptrSPITObject->getCreatedByClient() == false && ptrSPITObject->getConfirmed() == false) {
             i = sSpitObjects.erase(i);
-            delete ptrSPITObject;
         }
     }
 }
 
 void SPITObject::eventProjectClosed() {
     for (auto i = sSpitObjects.begin(); i != sSpitObjects.end(); i++) {
-        SPITObject *ptrSPITObject = i->second;
+        SPITObject *ptrSPITObject = i->second.get();
         if (ptrSPITObject->getCreatedByClient() == true) {
             continue;
         }
         i = sSpitObjects.erase(i);
-        delete ptrSPITObject;
     }
     // Remote
-    for (auto i = sSpitObjectsRemote.begin(); i != sSpitObjectsRemote.end(); i++) {
-        SPITObject *ptrSPITObject = i->second;
-        i = sSpitObjectsRemote.erase(i);
-        delete ptrSPITObject;
-    }
+    sSpitObjectsRemote.clear();
 }
 /**
  * sets confirmed = false for all SPITObjects.
  * This should be called when the client has loaded a project.
  */
 void SPITObject::setConfirmedFalse() {
-    SPITObject *ptrSPITObject;
-
     for (auto i = sSpitObjects.begin(); i != sSpitObjects.end(); i++) {
-        ptrSPITObject = i->second;
-        ptrSPITObject->setConfirmed(false);
+        i->second->setConfirmed(false);
     }
 }
 
-SPITObject::SPITObject() {
-    m_ID = createGUID();
+SPITObject::SPITObject(string p_ID) {
+    m_ID = p_ID;
 }
 
 bool SPITObject::setMessageOP(SPIT_Message::ReportSPITObject *p_ReportSPITObject) {
@@ -404,7 +379,7 @@ bool SPITObject::setServerRemote(bool p_BelongstoServer) {
 }
 /**
  * This is a virtual construct - ActionAssignedType is not implemented in the SPITProtocol.
- * This is used when an action is associated with a SPITType. 
+ * This is used when an action is associated with a SPITType.
  * Then this action is done for every SPITObject with the SPITType.
  * @return true = an action is assigned for the SPITType.
  */