Stop addEntityToLocation claiming an entity it did not place

README.md

@@ -81,7 +81,7 @@ Lookups that find nothing throw `std::runtime_error`:
 
 `Environment::moveEntityToNewLocation` looks up both the entity and the destination before moving anything, so an unknown entity id or location id throws and leaves the entity in place. The directional move methods catch the off-grid case and return `false`, but still throw if `entityId` does not name an entity in the environment.
 
-`addEntityToLocation` does not throw when no grid location has the given location's id. The entity is then placed nowhere, and its grid id, environment id, and location id are left unchanged.
+`addEntityToLocation` does not throw when no grid location has the given location's id. The entity is then placed nowhere, and its grid id, environment id, and location id are left unchanged. `Grid::addEntityToLocation` returns whether it placed the entity; `Environment::addEntityToLocation` returns nothing.
 
 `Grid::getFirstLocation` and `Grid::getRandomLocation` do not check for an empty grid. Calling either on a grid with no locations — including indirectly through `Environment::addEntity` on an environment of size `0` — is undefined behaviour.
 

src/environment.cpp

@@ -37,9 +37,9 @@ namespace envlibcpp {
     }
 
     void Environment::addEntityToLocation(Entity& entity, Location& location) {
-        grid->addEntityToLocation(entity, location);
-        // the grid only claims the entity when the location exists, so neither does the environment
-        if (entity.getGridId() == grid->getId()) {
+        // the grid only places the entity when the location exists, so the environment
+        // claims it only then; its grid id cannot tell, since it may already match
+        if (grid->addEntityToLocation(entity, location)) {
             entity.setEnvironmentId(id);
         }
     }

src/grid.cpp

@@ -97,16 +97,17 @@ namespace envlibcpp {
         firstLocation.addEntity(&entity);
     }
 
-    void Grid::addEntityToLocation(Entity& entity, Location& location) {
+    bool Grid::addEntityToLocation(Entity& entity, Location& location) {
         // the entity goes into the grid's own element, not the argument, which may be a
         // separately constructed Location that merely shares the id
         for (Location& l : locations) {
             if (l.getId() == location.getId()) {
                 entity.setGridId(id);
                 l.addEntity(&entity);
-                break;
+                return true;
             }
         }
+        return false;
     }
 
     void Grid::removeEntity(Entity& entity) {

src/header/grid.h

@@ -25,7 +25,7 @@ namespace envlibcpp {
             void addLocation(Location& location);
             void removeLocation(Location& location);
             void addEntity(Entity& entity);
-            void addEntityToLocation(Entity& entity, Location& location);
+            bool addEntityToLocation(Entity& entity, Location& location);
             void removeEntity(Entity& entity);
             bool isEntityPresent(Entity& entity);
             Location& getLocation(std::string locationId);

src/tests.cpp

@@ -8,6 +8,7 @@
 #include <iostream>
 #include <sstream>
 #include <stdexcept>
+#include <type_traits>
 #include <assert.h>
 
 #include "header/entity.h"
@@ -1287,6 +1288,41 @@ void testLocationLookupErrorsNameWhatWasLookedUp() {
     std::cout << " --- " << "Success" << std::endl;
 }
 
+void testAddingEntityToUnknownLocationLeavesEnvironmentIdOfSameIdGridEntity() {
+    std::cout << "Test 77 - Adding an entity to an unknown location leaves its ids alone when its grid id already matches";
+    Grid standalone(7, 2);
+    Entity entity(0, "Daniel");
+    standalone.addEntity(entity);
+    std::string locationId = entity.getLocationId();
+    Environment environment(7, "Earth", 2);
+    Location unknown("no-such-location", 5, 5);
+    environment.addEntityToLocation(entity, unknown);
+    assert(entity.getEnvironmentId() == -1);
+    assert(entity.getGridId() == 7);
+    assert(entity.getLocationId() == locationId);
+    assert(environment.isEntityPresent(entity) == false);
+    std::cout << " --- " << "Success" << std::endl;
+}
+
+void testEnvironmentCannotBeCopiedOrAssigned() {
+    std::cout << "Test 78 - An environment cannot be copied or assigned";
+    static_assert(!std::is_copy_constructible<Environment>::value, "a copy would share the Grid* and double-delete it");
+    static_assert(!std::is_copy_assignable<Environment>::value, "an assignment would share the Grid* and double-delete it");
+    std::cout << " --- " << "Success" << std::endl;
+}
+
+void testAddingEntityToLocationInGridReportsWhetherItWasPlaced() {
+    std::cout << "Test 79 - Adding an entity to a location in a grid reports whether it was placed";
+    Grid grid(0, 2);
+    Entity entity(0, "Daniel");
+    Location unknown("no-such-location", 5, 5);
+    assert(grid.addEntityToLocation(entity, unknown) == false);
+    assert(grid.getNumEntities() == 0);
+    assert(grid.addEntityToLocation(entity, grid.getFirstLocation()) == true);
+    assert(grid.getFirstLocation().isEntityPresent(&entity));
+    std::cout << " --- " << "Success" << std::endl;
+}
+
 void seedRandomNumberGenerator() {
     srand (time (NULL));
 }
@@ -1372,5 +1408,8 @@ int main() {
     testRemovingLocationDetachesEntityDespiteSameIdTwinElsewhere();
     testRemovingLocationsSharingAnIdDetachesEntityInBoth();
     testLocationLookupErrorsNameWhatWasLookedUp();
+    testAddingEntityToUnknownLocationLeavesEnvironmentIdOfSameIdGridEntity();
+    testEnvironmentCannotBeCopiedOrAssigned();
+    testAddingEntityToLocationInGridReportsWhetherItWasPlaced();
     return 0;
 }