From c593df529a0d694fd54d562dd2ae07dca7df8834 Mon Sep 17 00:00:00 2001
From: Daniel McCoy Stephenson <dmccoystephenson@gmail.com>
Date: Mon, 10 Aug 2026 02:05:18 -0600
Subject: [PATCH 1/2] Fix iterator invalidation in Grid::removeLocation

Capture the erase() return value so the loop continues from a valid
iterator, and copy the target id up front so an argument aliasing an
element of the vector is not dereferenced after erasure.

Adds Test 35 covering removal of a location from a grid, which reliably
aborted against the previous implementation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/grid.cpp  | 11 ++++++++---
 src/tests.cpp | 20 ++++++++++++++++----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/grid.cpp b/src/grid.cpp
index e9caa2a..ba472d4 100644
--- a/src/grid.cpp
+++ b/src/grid.cpp
@@ -55,9 +55,14 @@ namespace envlibcpp {
     }
 
     void Grid::removeLocation(Location& location) {
-        for (auto i = locations.begin(); i != locations.end(); i++) {
-            if (i->getId() == location.getId()) {
-                locations.erase(i);
+        // the id is copied up front because `location` may itself refer to an element of
+        // `locations`, which erase() would leave dangling
+        std::string targetId = location.getId();
+        for (auto i = locations.begin(); i != locations.end();) {
+            if (i->getId() == targetId) {
+                i = locations.erase(i);
+            } else {
+                i++;
             }
         }
     }
diff --git a/src/tests.cpp b/src/tests.cpp
index 9c34bbe..3ae4707 100644
--- a/src/tests.cpp
+++ b/src/tests.cpp
@@ -582,10 +582,6 @@ void testAddingLocationToGrid() {
     std::cout << " --- " << "Success" << std::endl;
 }
 
-// Grid::removeLocation is intentionally untested here: it has a pre-existing
-// iterator-invalidation bug (see #31) that a direct test reliably triggers as
-// undefined behavior rather than a clean assertion failure.
-
 void testRetrievingLocationByCoordinates() {
     std::cout << "Test 32 - Retrieving a location by coordinates";
     Grid grid(0, 5);
@@ -619,6 +615,21 @@ void testRetrievingNumEntitiesFromLocation() {
     std::cout << " --- " << "Success" << std::endl;
 }
 
+void testRemovingLocationFromGrid() {
+    std::cout << "Test 35 - Removing a location from a grid";
+    Grid grid(0, 2);
+    size_t initialNumLocations = grid.getLocations().size();
+    Location extraLocation("extra-location", 10, 10);
+    grid.addLocation(extraLocation);
+    assert(grid.getLocations().size() == initialNumLocations + 1);
+    grid.removeLocation(extraLocation);
+    assert(grid.getLocations().size() == initialNumLocations);
+    for (Location& location : grid.getLocations()) {
+        assert(location.getId() != "extra-location");
+    }
+    std::cout << " --- " << "Success" << std::endl;
+}
+
 void seedRandomNumberGenerator() {
     srand (time (NULL));
 }
@@ -662,5 +673,6 @@ int main() {
     testRetrievingLocationByCoordinates();
     testRetrievingEntitiesFromLocation();
     testRetrievingNumEntitiesFromLocation();
+    testRemovingLocationFromGrid();
     return 0;
 }
\ No newline at end of file

From d1f30c39c4176d769f36e5465ebbcd60f873f53e Mon Sep 17 00:00:00 2001
From: Daniel McCoy Stephenson <dmccoystephenson@gmail.com>
Date: Mon, 10 Aug 2026 02:06:30 -0600
Subject: [PATCH 2/2] Cover removal of a location from the middle of a grid

Test 35 only removed the trailing element, which exits the loop
immediately after the erase. Test 36 removes an interior location so the
post-erase continuation path is exercised as well.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/tests.cpp | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/tests.cpp b/src/tests.cpp
index 3ae4707..0c5ccd1 100644
--- a/src/tests.cpp
+++ b/src/tests.cpp
@@ -630,6 +630,20 @@ void testRemovingLocationFromGrid() {
     std::cout << " --- " << "Success" << std::endl;
 }
 
+void testRemovingLocationFromMiddleOfGrid() {
+    std::cout << "Test 36 - Removing a location from the middle of a grid";
+    Grid grid(0, 2);
+    size_t initialNumLocations = grid.getLocations().size();
+    std::string middleId = grid.getLocations()[1].getId();
+    Location target(middleId, 0, 0);
+    grid.removeLocation(target);
+    assert(grid.getLocations().size() == initialNumLocations - 1);
+    for (Location& location : grid.getLocations()) {
+        assert(location.getId() != middleId);
+    }
+    std::cout << " --- " << "Success" << std::endl;
+}
+
 void seedRandomNumberGenerator() {
     srand (time (NULL));
 }
@@ -674,5 +688,6 @@ int main() {
     testRetrievingEntitiesFromLocation();
     testRetrievingNumEntitiesFromLocation();
     testRemovingLocationFromGrid();
+    testRemovingLocationFromMiddleOfGrid();
     return 0;
 }
\ No newline at end of file