Repository: iluwatar/java-design-patterns
Issue: [Spatial Partition] Error when run App.java
Labels: type: bug
epic: pattern

When run [App.java](https://github.com/iluwatar/java-design-patterns/blob/5b147b00367a500e8dfeac882d4b0d6a0540c6b9/spatial-partition/src/main/java/com/iluwatar/spatialpartition/App.java), there are some problems:
- `ConcurrentModificationException` happen:
```
Exception in thread "main" java.util.ConcurrentModificationException
	at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1597)
	at java.base/java.util.HashMap$ValueIterator.next(HashMap.java:1625)
	at com.iluwatar.spatialpartition.Bubble.handleCollision(Bubble.java:69)
	at com.iluwatar.spatialpartition.App.lambda$noSpatialPartition$0(App.java:75)
	at java.base/java.util.HashMap.forEach(HashMap.java:1421)
	at com.iluwatar.spatialpartition.App.noSpatialPartition(App.java:70)
	at com.iluwatar.spatialpartition.App.main(App.java:125)
```
- Some logs don't work, just log `"Bubble"`
- Some logs don't use Slf4j formatting anchor _{}_.

Issue comments:
I'm working on it
1: at line no 66(var bubblesToCheck = bubbles.values();) and at ln :70      you are iterating it (bubbles.forEach) and inside handleCollision() method you are updating the same map(bubbles,inside pop() allBubbles.remove(this.id);) . 
Thats why once you get the iterator from a collection you can not modify the collection(if map changes the babbles.values() also affected).
2:same for withSpatialPartition() method LN:93 bubbles.forEach
3: same for noSpatialPartition() method LN 70:bubbles.forEach


Pull request review comments:

Patch that resolved the issue:
diff --git a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/App.java b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/App.java
index 47dcd7eb7560..e7b14249a732 100644
--- a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/App.java
+++ b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/App.java
@@ -25,7 +25,8 @@
 package com.iluwatar.spatialpartition;
 
 import java.security.SecureRandom;
-import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -59,9 +60,8 @@
 
 @Slf4j
 public class App {
-  private static final String BUBBLE = "Bubble ";
 
-  static void noSpatialPartition(int numOfMovements, HashMap<Integer, Bubble> bubbles) {
+  static void noSpatialPartition(int numOfMovements, Map<Integer, Bubble> bubbles) {
     //all bubbles have to be checked for collision for all bubbles
     var bubblesToCheck = bubbles.values();
 
@@ -77,11 +77,11 @@ static void noSpatialPartition(int numOfMovements, HashMap<Integer, Bubble> bubb
       numOfMovements--;
     }
     //bubbles not popped
-    bubbles.keySet().stream().map(key -> BUBBLE + key + " not popped").forEach(LOGGER::info);
+    bubbles.keySet().forEach(key -> LOGGER.info("Bubble {} not popped", key));
   }
 
   static void withSpatialPartition(
-      int height, int width, int numOfMovements, HashMap<Integer, Bubble> bubbles) {
+      int height, int width, int numOfMovements, Map<Integer, Bubble> bubbles) {
     //creating quadtree
     var rect = new Rect(width / 2D, height / 2D, width, height);
     var quadTree = new QuadTree(rect, 4);
@@ -100,7 +100,7 @@ static void withSpatialPartition(
       numOfMovements--;
     }
     //bubbles not popped
-    bubbles.keySet().stream().map(key -> BUBBLE + key + " not popped").forEach(LOGGER::info);
+    bubbles.keySet().forEach(key -> LOGGER.info("Bubble {} not popped", key));
   }
 
   /**
@@ -110,15 +110,15 @@ static void withSpatialPartition(
    */
 
   public static void main(String[] args) {
-    var bubbles1 = new HashMap<Integer, Bubble>();
-    var bubbles2 = new HashMap<Integer, Bubble>();
+    var bubbles1 = new ConcurrentHashMap<Integer, Bubble>();
+    var bubbles2 = new ConcurrentHashMap<Integer, Bubble>();
     var rand = new SecureRandom();
     for (int i = 0; i < 10000; i++) {
       var b = new Bubble(rand.nextInt(300), rand.nextInt(300), i, rand.nextInt(2) + 1);
       bubbles1.put(i, b);
       bubbles2.put(i, b);
-      LOGGER.info(BUBBLE, i, " with radius ", b.radius,
-          " added at (", b.coordinateX, ",", b.coordinateY + ")");
+      LOGGER.info("Bubble {} with radius {} added at ({},{})",
+          i, b.radius, b.coordinateX, b.coordinateY);
     }
 
     var start1 = System.currentTimeMillis();
@@ -127,8 +127,7 @@ public static void main(String[] args) {
     var start2 = System.currentTimeMillis();
     App.withSpatialPartition(300, 300, 20, bubbles2);
     var end2 = System.currentTimeMillis();
-    LOGGER.info("Without spatial partition takes ", (end1 - start1), "ms");
-    LOGGER.info("With spatial partition takes ", (end2 - start2), "ms");
+    LOGGER.info("Without spatial partition takes {} ms", (end1 - start1));
+    LOGGER.info("With spatial partition takes {} ms", (end2 - start2));
   }
 }
-
diff --git a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Bubble.java b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Bubble.java
index f1f9fb61d03a..70f8cae48d65 100644
--- a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Bubble.java
+++ b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Bubble.java
@@ -26,7 +26,7 @@
 
 import java.security.SecureRandom;
 import java.util.Collection;
-import java.util.HashMap;
+import java.util.Map;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -58,13 +58,12 @@ boolean touches(Bubble b) {
         <= (this.radius + b.radius) * (this.radius + b.radius);
   }
 
-  void pop(HashMap<Integer, Bubble> allBubbles) {
-    LOGGER.info("Bubble ", this.id,
-        " popped at (", this.coordinateX, ",", this.coordinateY, ")!");
+  void pop(Map<Integer, Bubble> allBubbles) {
+    LOGGER.info("Bubble {} popped at ({},{})!", this.id, this.coordinateX, this.coordinateY);
     allBubbles.remove(this.id);
   }
 
-  void handleCollision(Collection<? extends Point> toCheck, HashMap<Integer, Bubble> allBubbles) {
+  void handleCollision(Collection<? extends Point> toCheck, Map<Integer, Bubble> allBubbles) {
     var toBePopped = false; //if any other bubble collides with it, made true
     for (var point : toCheck) {
       var otherId = point.id;
diff --git a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Point.java b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Point.java
index 81312185247c..1b24e70fa37e 100644
--- a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Point.java
+++ b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/Point.java
@@ -25,7 +25,7 @@
 package com.iluwatar.spatialpartition;
 
 import java.util.Collection;
-import java.util.HashMap;
+import java.util.Map;
 
 /**
  * The abstract Point class which will be extended by any object in the field whose location has to
@@ -65,5 +65,5 @@ public abstract class Point<T> {
    * @param toCheck contains the objects which need to be checked
    * @param all     contains hashtable of all points on field at this time
    */
-  abstract void handleCollision(Collection<? extends Point> toCheck, HashMap<Integer, T> all);
+  abstract void handleCollision(Collection<? extends Point> toCheck, Map<Integer, T> all);
 }
diff --git a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/QuadTree.java b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/QuadTree.java
index 12a84f5a33e9..40c09a1d8f6e 100644
--- a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/QuadTree.java
+++ b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/QuadTree.java
@@ -25,7 +25,8 @@
 package com.iluwatar.spatialpartition;
 
 import java.util.Collection;
-import java.util.Hashtable;
+import java.util.HashMap;
+import java.util.Map;
 
 /**
  * The quadtree data structure is being used to keep track of the objects' locations. It has the
@@ -37,7 +38,7 @@ public class QuadTree {
   Rect boundary;
   int capacity;
   boolean divided;
-  Hashtable<Integer, Point> points;
+  Map<Integer, Point> points;
   QuadTree northwest;
   QuadTree northeast;
   QuadTree southwest;
@@ -47,7 +48,7 @@ public class QuadTree {
     this.boundary = boundary;
     this.capacity = capacity;
     this.divided = false;
-    this.points = new Hashtable<>();
+    this.points = new HashMap<>();
     this.northwest = null;
     this.northeast = null;
     this.southwest = null;
diff --git a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionBubbles.java b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionBubbles.java
index 50d20caa32f7..3551f5ca3b9b 100644
--- a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionBubbles.java
+++ b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionBubbles.java
@@ -25,7 +25,7 @@
 package com.iluwatar.spatialpartition;
 
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Map;
 
 /**
  * This class extends the generic SpatialPartition abstract class and is used in our example to keep
@@ -34,10 +34,10 @@
 
 public class SpatialPartitionBubbles extends SpatialPartitionGeneric<Bubble> {
 
-  private final HashMap<Integer, Bubble> bubbles;
+  private final Map<Integer, Bubble> bubbles;
   private final QuadTree bubblesQuadTree;
 
-  SpatialPartitionBubbles(HashMap<Integer, Bubble> bubbles, QuadTree bubblesQuadTree) {
+  SpatialPartitionBubbles(Map<Integer, Bubble> bubbles, QuadTree bubblesQuadTree) {
     this.bubbles = bubbles;
     this.bubblesQuadTree = bubblesQuadTree;
   }
diff --git a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionGeneric.java b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionGeneric.java
index c1eb8d1f3600..7c03969a08e6 100644
--- a/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionGeneric.java
+++ b/spatial-partition/src/main/java/com/iluwatar/spatialpartition/SpatialPartitionGeneric.java
@@ -24,7 +24,7 @@
  */
 package com.iluwatar.spatialpartition;
 
-import java.util.Hashtable;
+import java.util.Map;
 
 /**
  * This abstract class has 2 fields, one of which is a hashtable containing all objects that
@@ -35,7 +35,7 @@
 
 public abstract class SpatialPartitionGeneric<T> {
 
-  Hashtable<Integer, T> playerPositions;
+  Map<Integer, T> playerPositions;
   QuadTree quadTree;
 
   /**


Test patch: