Remove every pointer to an entity from its location

README.md

@@ -67,7 +67,7 @@ A newly constructed entity has an environment id and grid id of `-1` and a locat
 
 Adding is likewise unchecked. `addEntity` and `addEntityToLocation` do not test whether the entity is already present, so adding it a second time stores a second pointer to it (in another location, or in the same one), its location id names only the most recent location, and `getNumEntities` counts it twice. To relocate an entity that is already placed, use `moveEntityToNewLocation` or one of the directional move methods, which remove it before adding it again.
 
-Removal takes out at most one pointer per location. An entity added to two different locations is removed from both, but one added twice to the same location keeps a pointer there after `removeEntity`: it is still present and still counted, while its ids have already been reset, so a directional move then throws `std::runtime_error` because its location id `"N/S"` names no location.
+Removal takes out every pointer with a matching id. An entity added to two different locations is removed from both, and one added twice to the same location is removed entirely, so a move leaves a single pointer at the destination. Since matching is by id, a distinct same-id twin stored in that location is taken out too, and that twin keeps its ids.
 
 `Grid::removeLocation` also detaches any entities in the removed location: it resets their location id to `"N/S"` and their grid id to `-1`. It leaves their environment id unchanged, since the grid has no knowledge of its environment.
 

src/location.cpp

@@ -1,5 +1,6 @@
 #include "header/location.h"
 
+#include <algorithm>
 #include <iostream>
 
 namespace envlibcpp {
@@ -31,16 +32,13 @@ namespace envlibcpp {
     }
 
     void Location::removeEntity(Entity* entity) {
-        int index = -1;
-        for (size_t i = 0; i < getNumEntities(); i++) {
-            if (entities[i]->getId() == entity->getId()) {
-                index = i;
-                break;
-            }
-        }
+        // every match goes, since an entity added twice is stored twice and must not stay present
+        auto newEnd = std::remove_if(entities.begin(), entities.end(), [entity](Entity* e) {
+            return e->getId() == entity->getId();
+        });
         // an entity that is not in this location keeps its location id
-        if (index != -1) {
-            entities.erase(entities.begin() + index);
+        if (newEnd != entities.end()) {
+            entities.erase(newEnd, entities.end());
             entity->resetLocationId();
         }
     }

src/tests.cpp

@@ -1089,26 +1089,20 @@ void testMovingEntityInSingleLocationGridAlwaysReturnsFalse() {
     std::cout << " --- " << "Success" << std::endl;
 }
 
-void testRemovingEntityAddedTwiceToOneLocationLeavesOnePointer() {
-    std::cout << "Test 66 - Removing an entity added twice to one location leaves one pointer behind";
+void testRemovingEntityAddedTwiceToOneLocationRemovesBothPointers() {
+    std::cout << "Test 66 - Removing an entity added twice to one location removes both pointers";
     Environment environment(0, "Earth", 2);
     Entity entity(0, "Daniel");
     Location& location = environment.getGrid()->getLocationByCoordinates(0, 0);
     environment.addEntityToLocation(entity, location);
     environment.addEntityToLocation(entity, location);
     environment.removeEntity(entity);
-    assert(location.getNumEntities() == 1);
-    assert(environment.isEntityPresent(entity) == true);
+    assert(location.getNumEntities() == 0);
+    assert(environment.isEntityPresent(entity) == false);
+    assert(environment.getNumEntities() == 0);
     assert(entity.getEnvironmentId() == -1);
     assert(entity.getGridId() == -1);
     assert(entity.getLocationId() == "N/S");
-    bool threw = false;
-    try {
-        environment.moveEntityUp(0);
-    } catch (const std::runtime_error& e) {
-        threw = true;
-    }
-    assert(threw == true);
     std::cout << " --- " << "Success" << std::endl;
 }
 
@@ -1139,6 +1133,69 @@ void testRemovingEntityViaSameIdTwinLeavesOriginalIdsStale() {
     std::cout << " --- " << "Success" << std::endl;
 }
 
+void testRemovingEntityFromLocationRemovesEveryPointerAndKeepsOrder() {
+    std::cout << "Test 69 - Removing an entity from a location removes every pointer and keeps the others in order";
+    Location location("test-location", 0, 0);
+    Entity first(1, "Daniel");
+    Entity repeated(2, "Bob");
+    Entity last(3, "Alice");
+    location.addEntity(&repeated);
+    location.addEntity(&first);
+    location.addEntity(&repeated);
+    location.addEntity(&last);
+    location.removeEntity(&repeated);
+    assert(location.getNumEntities() == 2);
+    assert(location.isEntityPresent(&repeated) == false);
+    assert(location.getEntities()[0] == &first);
+    assert(location.getEntities()[1] == &last);
+    assert(repeated.getLocationId() == "N/S");
+    assert(first.getLocationId() == "test-location");
+    assert(last.getLocationId() == "test-location");
+    std::cout << " --- " << "Success" << std::endl;
+}
+
+void testMovingEntityAddedTwiceToOneLocationLeavesASinglePointer() {
+    std::cout << "Test 70 - Moving an entity added twice to one location leaves a single pointer at the destination";
+    Environment environment(0, "Earth", 2);
+    Entity entity(0, "Daniel");
+    Location& start = environment.getGrid()->getLocationByCoordinates(0, 0);
+    Location& right = environment.getGrid()->getLocationByCoordinates(1, 0);
+    Location& corner = environment.getGrid()->getLocationByCoordinates(1, 1);
+    environment.addEntityToLocation(entity, start);
+    environment.addEntityToLocation(entity, start);
+    assert(environment.moveEntityRight(0) == true);
+    assert(start.getNumEntities() == 0);
+    assert(right.getNumEntities() == 1);
+    assert(environment.getNumEntities() == 1);
+    assert(entity.getLocationId() == right.getId());
+
+    environment.addEntityToLocation(entity, right);
+    environment.moveEntityToNewLocation(0, corner.getId());
+    assert(right.getNumEntities() == 0);
+    assert(corner.getNumEntities() == 1);
+    assert(environment.getNumEntities() == 1);
+    assert(entity.getLocationId() == corner.getId());
+    std::cout << " --- " << "Success" << std::endl;
+}
+
+void testRemovingEntityAlsoRemovesSameIdTwinInItsLocation() {
+    std::cout << "Test 71 - Removing an entity also removes a same-id twin stored in its location";
+    Environment environment(0, "Earth", 2);
+    Entity original(5, "Daniel");
+    Entity twin(5, "Bob");
+    Location& location = environment.getGrid()->getLocationByCoordinates(1, 0);
+    environment.addEntityToLocation(original, location);
+    environment.addEntityToLocation(twin, location);
+    environment.removeEntity(original);
+    assert(location.getNumEntities() == 0);
+    assert(environment.getNumEntities() == 0);
+    assert(original.getLocationId() == "N/S");
+    assert(twin.getEnvironmentId() == 0);
+    assert(twin.getGridId() == 0);
+    assert(twin.getLocationId() == "0-1-0");
+    std::cout << " --- " << "Success" << std::endl;
+}
+
 void seedRandomNumberGenerator() {
     srand (time (NULL));
 }
@@ -1213,8 +1270,11 @@ int main() {
     testRemovingLocationLeavesEntityEnvironmentIdUnchanged();
     testAddingLocationToGridStoresACopy();
     testMovingEntityInSingleLocationGridAlwaysReturnsFalse();
-    testRemovingEntityAddedTwiceToOneLocationLeavesOnePointer();
+    testRemovingEntityAddedTwiceToOneLocationRemovesBothPointers();
     testRemovingEntityAddedToTwoLocationsRemovesBoth();
     testRemovingEntityViaSameIdTwinLeavesOriginalIdsStale();
+    testRemovingEntityFromLocationRemovesEveryPointerAndKeepsOrder();
+    testMovingEntityAddedTwiceToOneLocationLeavesASinglePointer();
+    testRemovingEntityAlsoRemovesSameIdTwinInItsLocation();
     return 0;
 }