From aa170f02e152d47a01b00eadfd833817cfbd83f6 Mon Sep 17 00:00:00 2001
From: Sean Barbeau <barbeau@cutr.usf.edu>
Date: Wed, 19 Feb 2020 12:51:11 -0500
Subject: [PATCH 1/5] feat(cluster): Support updating existing items

Prior to these changes, when an item instance was removed, updated, and re-added to the ClusterManager, DefaultClusterRenderer.CreateMarkerTask.perform() would use a marker from its own mMarkerCache associated with that item to display that item on the map (even if ClusterManager.clearItems() was previously called). This resulted in the updated item contents not being reflected on the map, because the cached marker was not updated.

This changeset does the following:
* Add new protected methods in DefaultClusterRenderer - onClusterItemUpdated() and onClusterUpdated() - which are called when an existing cached marker is found for an item/cluster when that item/cluster is being added back into the ClusterManager. The default implementation of these methods update the marker contents on the map, but then can also be overridden by applications for custom behavior (similar to onBeforeClusterItemRendered() and onBeforeClusterRendered()).  Following this change, you can now do ClusterManager.clearItems() and re-add the same list instance (or remove() and then add() the same item) and then call cluster() and the map markers will be updated with the new item model contents.
* Add a new ClusterManager.updateItem() helper method for updating the ClusterManager and algorithms with an existing item. The current implementation in NonHierarchicalDistanceBasedAlgorithm is a wrapper for remove() and then add().
* Add more descriptive Javadocs to ClusterManager, DefaultClusterRenderer, and NonHierarchicalDistanceBasedAlgorithm to describe expected behavior, especially that cluster() should be invoked after any changes to ClusterManager to see changes on the map.

One new TODO that could potentially be addressed - it seems like the code to set the marker text when creating the item marker should be refactored into onBeforeClusterItemRendered() to match the implementation of clusters (onBeforeClusterRendered()), and to allow applications extending DefaultClusterRenderer to inherit this default behavior via a call to super.onBeforeClusterItemRendered(). However, moving this code would change the behavior of the library for apps already extending DefaultClusterRenderer that aren't calling super.onBeforeClusterItemRendered() - the marker title and snippet would no longer be updated.
---
 .../android/clustering/ClusterManager.java    |  49 +++++-
 .../android/clustering/algo/Algorithm.java    |   2 +
 .../clustering/algo/GridBasedAlgorithm.java   |  10 +-
 ...NonHierarchicalDistanceBasedAlgorithm.java |   7 +
 .../algo/PreCachingAlgorithmDecorator.java    |  10 +-
 .../algo/ScreenBasedAlgorithmAdapter.java     |   5 +
 .../view/DefaultClusterRenderer.java          | 148 +++++++++++++++---
 7 files changed, 202 insertions(+), 29 deletions(-)

diff --git a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
index b78de4310..c2bdeed46 100644
--- a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
+++ b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
@@ -16,13 +16,9 @@
 
 package com.google.maps.android.clustering;
 
-import android.content.Context;
-import android.os.AsyncTask;
-
 import com.google.android.gms.maps.GoogleMap;
 import com.google.android.gms.maps.model.CameraPosition;
 import com.google.android.gms.maps.model.Marker;
-import com.google.maps.android.collections.MarkerManager;
 import com.google.maps.android.clustering.algo.Algorithm;
 import com.google.maps.android.clustering.algo.NonHierarchicalDistanceBasedAlgorithm;
 import com.google.maps.android.clustering.algo.PreCachingAlgorithmDecorator;
@@ -30,6 +26,10 @@
 import com.google.maps.android.clustering.algo.ScreenBasedAlgorithmAdapter;
 import com.google.maps.android.clustering.view.ClusterRenderer;
 import com.google.maps.android.clustering.view.DefaultClusterRenderer;
+import com.google.maps.android.collections.MarkerManager;
+
+import android.content.Context;
+import android.os.AsyncTask;
 
 import java.util.Collection;
 import java.util.Set;
@@ -147,6 +147,10 @@ public Algorithm<T> getAlgorithm() {
         return mAlgorithm;
     }
 
+    /**
+     * Removes all items from the cluster manager. After calling this method you must invoke
+     * cluster() for the map to be cleared.
+     */
     public void clearItems() {
         mAlgorithm.lock();
         try {
@@ -156,6 +160,11 @@ public void clearItems() {
         }
     }
 
+    /**
+     * Adds items to clusters. After calling this method you must invoke cluster() for the state
+     * of the clusters to be updated on the map.
+     * @param items items to add to clusters
+     */
     public void addItems(Collection<T> items) {
         mAlgorithm.lock();
         try {
@@ -165,6 +174,11 @@ public void addItems(Collection<T> items) {
         }
     }
 
+    /**
+     * Adds an item to a cluster. After calling this method you must invoke cluster() for the state
+     * of the clusters to be updated on the map.
+     * @param myItem item to add to clusters
+     */
     public void addItem(T myItem) {
         mAlgorithm.lock();
         try {
@@ -174,6 +188,11 @@ public void addItem(T myItem) {
         }
     }
 
+    /**
+     * Removes items from clusters. After calling this method you must invoke cluster() for the state
+     * of the clusters to be updated on the map.
+     * @param items items to remove from clusters
+     */
     public void removeItems(Collection<T> items) {
         mAlgorithm.lock();
         try {
@@ -183,6 +202,11 @@ public void removeItems(Collection<T> items) {
         }
     }
 
+    /**
+     * Removes an item from clusters. After calling this method you must invoke cluster() for the state
+     * of the clusters to be updated on the map.
+     * @param item item to remove from clusters
+     */
     public void removeItem(T item) {
         mAlgorithm.lock();
         try {
@@ -193,7 +217,22 @@ public void removeItem(T item) {
     }
 
     /**
-     * Force a re-cluster. You may want to call this after adding new item(s).
+     * Updates an item in clusters. After calling this method you must invoke cluster() for the state
+     * of the clusters to be updated on the map.
+     * @param item item to update in clusters
+     */
+    public void updateItem(T item) {
+        mAlgorithm.lock();
+        try {
+            mAlgorithm.updateItem(item);
+        } finally {
+            mAlgorithm.unlock();
+        }
+    }
+
+    /**
+     * Force a re-cluster on the map. You should call this after adding, removing, updating,
+     * or clearing item(s).
      */
     public void cluster() {
         mClusterTaskLock.writeLock().lock();
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java b/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java
index 25c78c485..d9ef6ba25 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java
@@ -35,6 +35,8 @@ public interface Algorithm<T extends ClusterItem> {
 
     void removeItem(T item);
 
+    void updateItem(T item);
+
     void removeItems(Collection<T> items);
 
     Set<? extends Cluster<T>> getClusters(float zoom);
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java b/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java
index fbc2af14f..85daeda12 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java
@@ -16,8 +16,6 @@
 
 package com.google.maps.android.clustering.algo;
 
-import androidx.collection.LongSparseArray;
-
 import com.google.maps.android.clustering.Cluster;
 import com.google.maps.android.clustering.ClusterItem;
 import com.google.maps.android.geometry.Point;
@@ -28,6 +26,8 @@
 import java.util.HashSet;
 import java.util.Set;
 
+import androidx.collection.LongSparseArray;
+
 /**
  * Groups markers into a grid.
  */
@@ -63,6 +63,12 @@ public void removeItems(Collection<T> items) {
         mItems.removeAll(items);
     }
 
+    @Override
+    public void updateItem(T item) {
+        removeItem(item);
+        addItem(item);
+    }
+
     @Override
     public void setMaxDistanceBetweenClusteredItems(int maxDistance) {
         mGridSize = maxDistance;
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java b/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java
index 546733bf7..802ef7f14 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java
@@ -110,6 +110,13 @@ public void removeItems(Collection<T> items) {
         }
     }
 
+    @Override
+    public void updateItem(T item) {
+        // TODO - Can this be optimized to update the item in-place if the location hasn't changed?
+        removeItem(item);
+        addItem(item);
+    }
+
     @Override
     public Set<? extends Cluster<T>> getClusters(float zoom) {
         final int discreteZoom = (int) zoom;
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java b/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java
index 65a381d94..806086dc5 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java
@@ -16,8 +16,6 @@
 
 package com.google.maps.android.clustering.algo;
 
-import androidx.collection.LruCache;
-
 import com.google.maps.android.clustering.Cluster;
 import com.google.maps.android.clustering.ClusterItem;
 
@@ -28,6 +26,8 @@
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
+import androidx.collection.LruCache;
+
 /**
  * Optimistically fetch clusters for adjacent zoom levels, caching them as necessary.
  */
@@ -73,6 +73,12 @@ public void removeItems(Collection<T> items) {
         clearCache();
     }
 
+    @Override
+    public void updateItem(T item) {
+        mAlgorithm.updateItem(item);
+        clearCache();
+    }
+
     private void clearCache() {
         mCache.evictAll();
     }
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java b/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java
index 2434b6989..d0664efc2 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java
@@ -61,6 +61,11 @@ public void removeItems(Collection<T> items) {
         mAlgorithm.removeItems(items);
     }
 
+    @Override
+    public void updateItem(T item) {
+        mAlgorithm.updateItem(item);
+    }
+
     @Override
     public Set<? extends Cluster<T>> getClusters(float zoom) {
         return mAlgorithm.getClusters(zoom);
diff --git a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
index d0d25b2d6..8e933c8ac 100644
--- a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
+++ b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
@@ -16,6 +16,24 @@
 
 package com.google.maps.android.clustering.view;
 
+import com.google.android.gms.maps.GoogleMap;
+import com.google.android.gms.maps.Projection;
+import com.google.android.gms.maps.model.BitmapDescriptor;
+import com.google.android.gms.maps.model.BitmapDescriptorFactory;
+import com.google.android.gms.maps.model.LatLng;
+import com.google.android.gms.maps.model.LatLngBounds;
+import com.google.android.gms.maps.model.Marker;
+import com.google.android.gms.maps.model.MarkerOptions;
+import com.google.maps.android.R;
+import com.google.maps.android.clustering.Cluster;
+import com.google.maps.android.clustering.ClusterItem;
+import com.google.maps.android.clustering.ClusterManager;
+import com.google.maps.android.collections.MarkerManager;
+import com.google.maps.android.geometry.Point;
+import com.google.maps.android.projection.SphericalMercatorProjection;
+import com.google.maps.android.ui.IconGenerator;
+import com.google.maps.android.ui.SquareTextView;
+
 import android.animation.Animator;
 import android.animation.AnimatorListenerAdapter;
 import android.animation.TimeInterpolator;
@@ -37,24 +55,6 @@
 import android.view.ViewGroup;
 import android.view.animation.DecelerateInterpolator;
 
-import com.google.android.gms.maps.GoogleMap;
-import com.google.android.gms.maps.Projection;
-import com.google.android.gms.maps.model.BitmapDescriptor;
-import com.google.android.gms.maps.model.BitmapDescriptorFactory;
-import com.google.android.gms.maps.model.LatLng;
-import com.google.android.gms.maps.model.LatLngBounds;
-import com.google.android.gms.maps.model.Marker;
-import com.google.android.gms.maps.model.MarkerOptions;
-import com.google.maps.android.collections.MarkerManager;
-import com.google.maps.android.R;
-import com.google.maps.android.clustering.Cluster;
-import com.google.maps.android.clustering.ClusterItem;
-import com.google.maps.android.clustering.ClusterManager;
-import com.google.maps.android.geometry.Point;
-import com.google.maps.android.projection.SphericalMercatorProjection;
-import com.google.maps.android.ui.IconGenerator;
-import com.google.maps.android.ui.SquareTextView;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -737,15 +737,94 @@ public void remove(Marker m) {
 
     /**
      * Called before the marker for a ClusterItem is added to the map.
+     *
+     * The first time ClusterManager.cluster() is invoked on a set of items
+     * onBeforeClusterItemRendered() will be called and onClusterItemUpdated() will not be called.
+     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
+     * again, then onClusterItemUpdated() will be called and onBeforeClusterItemRendered() will not
+     * be called.
+     *
+     * @param item item to be rendered
+     * @param markerOptions the markerOptions representing the provided item
      */
     protected void onBeforeClusterItemRendered(T item, MarkerOptions markerOptions) {
     }
 
+    /**
+     * Called when a cached marker for a ClusterItem already exists on the map so the marker may
+     * be updated to the latest item values. Default implementation updates the title and snippet
+     * of the marker if they have changed and refreshes the info window of the marker if it is open.
+     * Note that the contents of the item may not have changed since the cached marker was created -
+     * implementations of this method are responsible for checking if something changed (if that
+     * matters to the implementation).
+     *
+     * The first time ClusterManager.cluster() is invoked on a set of items
+     * onBeforeClusterItemRendered() will be called and onClusterItemUpdated() will not be called.
+     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
+     * again, then onClusterItemUpdated() will be called and onBeforeClusterItemRendered() will not
+     * be called.
+     *
+     * @param item item being updated
+     * @param marker cached marker that contains a potentially previous state of the item.
+     */
+    protected void onClusterItemUpdated(T item, Marker marker) {
+        boolean changed = false;
+        // Update marker text if the item text changed - same logic as adding marker in CreateMarkerTask.perform()
+        if (item.getTitle() != null && item.getSnippet() != null) {
+            if (!marker.getTitle().equals(item.getTitle())) {
+                marker.setTitle(item.getTitle());
+                changed = true;
+            }
+            if (!marker.getSnippet().equals(item.getSnippet())) {
+                marker.setSnippet(item.getSnippet());
+                changed = true;
+            }
+        } else if (item.getSnippet() != null && !item.getSnippet().equals(marker.getTitle())) {
+            marker.setTitle(item.getSnippet());
+            changed = true;
+        } else if (item.getTitle() != null && !item.getTitle().equals(marker.getTitle())) {
+            marker.setTitle(item.getTitle());
+            changed = true;
+        }
+        // Update marker position if the item changed position
+        if (!marker.getPosition().equals(item.getPosition())) {
+            marker.setPosition(item.getPosition());
+            changed = true;
+        }
+        if (changed && marker.isInfoWindowShown()) {
+            // Force a refresh of marker info window contents
+            marker.showInfoWindow();
+        }
+    }
+
     /**
      * Called before the marker for a Cluster is added to the map.
      * The default implementation draws a circle with a rough count of the number of items.
+     *
+     * The first time ClusterManager.cluster() is invoked on a set of items
+     * onBeforeClusterRendered() will be called and onClusterUpdated() will not be called.
+     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
+     * again, then onClusterUpdated() will be called and onBeforeClusterRendered() will not be
+     * called.
+     *
+     * @param cluster cluster to be rendered
+     * @param markerOptions markerOptions representing the provided cluster
      */
     protected void onBeforeClusterRendered(Cluster<T> cluster, MarkerOptions markerOptions) {
+        // TODO: consider adding anchor(.5, .5) (Individual markers will overlap more often)
+        markerOptions.icon(getDescriptorForCluster(cluster));
+    }
+
+    /**
+     * Gets a BitmapDescriptor for the given cluster that contains a rough count of the number of
+     * items. Used to set the cluster marker icon in the default implementations of
+     * onBeforeClusterRendered() and onClusterUpdated().
+     *
+     * @param cluster cluster to get BitmapDescriptor for
+     * @return a BitmapDescriptor for the marker icon for the given cluster that contains a rough
+     * count of the number of items.
+     */
+    protected BitmapDescriptor getDescriptorForCluster(Cluster<T> cluster) {
         int bucket = getBucket(cluster);
         BitmapDescriptor descriptor = mIcons.get(bucket);
         if (descriptor == null) {
@@ -753,18 +832,44 @@ protected void onBeforeClusterRendered(Cluster<T> cluster, MarkerOptions markerO
             descriptor = BitmapDescriptorFactory.fromBitmap(mIconGenerator.makeIcon(getClusterText(bucket)));
             mIcons.put(bucket, descriptor);
         }
-        // TODO: consider adding anchor(.5, .5) (Individual markers will overlap more often)
-        markerOptions.icon(descriptor);
+        return descriptor;
     }
 
     /**
      * Called after the marker for a Cluster has been added to the map.
+     *
+     * @param cluster the cluster that was just added to the map
+     * @param marker the marker representing the cluster that was just added to the map
      */
     protected void onClusterRendered(Cluster<T> cluster, Marker marker) {
     }
 
+    /**
+     * Called when a cached marker for a Cluster already exists on the map so the marker may
+     * be updated to the latest cluster values. Default implementation updated the icon with a
+     * circle with a rough count of the number of items. Note that the contents of the cluster may
+     * not have changed since the cached marker was created - implementations of this method are
+     * responsible for checking if something changed (if that matters to the implementation).
+     *
+     * The first time ClusterManager.cluster() is invoked on a set of items
+     * onBeforeClusterRendered() will be called and onClusterUpdated() will not be called.
+     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
+     * again, then onClusterUpdated() will be called and onBeforeClusterRendered() will not be
+     * called.
+     *
+     * @param cluster cluster being updated
+     * @param marker cached marker that contains a potentially previous state of the cluster
+     */
+    protected void onClusterUpdated(Cluster<T> cluster, Marker marker) {
+        // TODO: consider adding anchor(.5, .5) (Individual markers will overlap more often)
+        marker.setIcon(getDescriptorForCluster(cluster));
+    }
+
     /**
      * Called after the marker for a ClusterItem has been added to the map.
+     *
+     * @param clusterItem the item that was just added to the map
+     * @param marker the marker representing the item that was just added to the map
      */
     protected void onClusterItemRendered(T clusterItem, Marker marker) {
     }
@@ -842,6 +947,7 @@ private void perform(MarkerModifier markerModifier) {
                         } else {
                             markerOptions.position(item.getPosition());
                         }
+                        // TODO (?) - Refactor set marker text into onBeforeClusterItemRendered()
                         if (!(item.getTitle() == null) && !(item.getSnippet() == null)) {
                             markerOptions.title(item.getTitle());
                             markerOptions.snippet(item.getSnippet());
@@ -859,6 +965,7 @@ private void perform(MarkerModifier markerModifier) {
                         }
                     } else {
                         markerWithPosition = new MarkerWithPosition(marker);
+                        onClusterItemUpdated(item, marker);
                     }
                     onClusterItemRendered(item, marker);
                     newMarkers.add(markerWithPosition);
@@ -880,6 +987,7 @@ private void perform(MarkerModifier markerModifier) {
                 }
             } else {
                 markerWithPosition = new MarkerWithPosition(marker);
+                onClusterUpdated(cluster, marker);
             }
             onClusterRendered(cluster, marker);
             newMarkers.add(markerWithPosition);

From 556d20c2f9655569eaa2f972410e96ec6c381619 Mon Sep 17 00:00:00 2001
From: Sean Barbeau <barbeau@cutr.usf.edu>
Date: Thu, 20 Feb 2020 10:00:54 -0500
Subject: [PATCH 2/5] Add Javadoc links

---
 .../android/clustering/ClusterManager.java    | 22 ++++-----
 .../view/DefaultClusterRenderer.java          | 49 ++++++++++---------
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
index c2bdeed46..c3aadc133 100644
--- a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
+++ b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
@@ -149,7 +149,7 @@ public Algorithm<T> getAlgorithm() {
 
     /**
      * Removes all items from the cluster manager. After calling this method you must invoke
-     * cluster() for the map to be cleared.
+     * {@link #cluster()} for the map to be cleared.
      */
     public void clearItems() {
         mAlgorithm.lock();
@@ -161,8 +161,8 @@ public void clearItems() {
     }
 
     /**
-     * Adds items to clusters. After calling this method you must invoke cluster() for the state
-     * of the clusters to be updated on the map.
+     * Adds items to clusters. After calling this method you must invoke {@link #cluster()} for the
+     * state of the clusters to be updated on the map.
      * @param items items to add to clusters
      */
     public void addItems(Collection<T> items) {
@@ -175,8 +175,8 @@ public void addItems(Collection<T> items) {
     }
 
     /**
-     * Adds an item to a cluster. After calling this method you must invoke cluster() for the state
-     * of the clusters to be updated on the map.
+     * Adds an item to a cluster. After calling this method you must invoke {@link #cluster()} for
+     * the state of the clusters to be updated on the map.
      * @param myItem item to add to clusters
      */
     public void addItem(T myItem) {
@@ -189,8 +189,8 @@ public void addItem(T myItem) {
     }
 
     /**
-     * Removes items from clusters. After calling this method you must invoke cluster() for the state
-     * of the clusters to be updated on the map.
+     * Removes items from clusters. After calling this method you must invoke {@link #cluster()} for
+     * the state of the clusters to be updated on the map.
      * @param items items to remove from clusters
      */
     public void removeItems(Collection<T> items) {
@@ -203,8 +203,8 @@ public void removeItems(Collection<T> items) {
     }
 
     /**
-     * Removes an item from clusters. After calling this method you must invoke cluster() for the state
-     * of the clusters to be updated on the map.
+     * Removes an item from clusters. After calling this method you must invoke {@link #cluster()}
+     * for the state of the clusters to be updated on the map.
      * @param item item to remove from clusters
      */
     public void removeItem(T item) {
@@ -217,8 +217,8 @@ public void removeItem(T item) {
     }
 
     /**
-     * Updates an item in clusters. After calling this method you must invoke cluster() for the state
-     * of the clusters to be updated on the map.
+     * Updates an item in clusters. After calling this method you must invoke {@link #cluster()} for
+     * the state of the clusters to be updated on the map.
      * @param item item to update in clusters
      */
     public void updateItem(T item) {
diff --git a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
index 8e933c8ac..76fc8b396 100644
--- a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
+++ b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
@@ -738,11 +738,12 @@ public void remove(Marker m) {
     /**
      * Called before the marker for a ClusterItem is added to the map.
      *
-     * The first time ClusterManager.cluster() is invoked on a set of items
-     * onBeforeClusterItemRendered() will be called and onClusterItemUpdated() will not be called.
-     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
-     * again, then onClusterItemUpdated() will be called and onBeforeClusterItemRendered() will not
-     * be called.
+     * The first time {@link ClusterManager#cluster()} is invoked on a set of items
+     * {@link #onBeforeClusterItemRendered(ClusterItem, MarkerOptions)} will be called and
+     * {@link #onClusterItemUpdated(ClusterItem, Marker)} will not be called.
+     * If an item is removed and re-added (or updated) and {@link ClusterManager#cluster()} is
+     * invoked again, then {@link #onClusterItemUpdated(ClusterItem, Marker)} will be called and
+     * {@link #onBeforeClusterItemRendered(ClusterItem, MarkerOptions)} will not be called.
      *
      * @param item item to be rendered
      * @param markerOptions the markerOptions representing the provided item
@@ -758,11 +759,12 @@ protected void onBeforeClusterItemRendered(T item, MarkerOptions markerOptions)
      * implementations of this method are responsible for checking if something changed (if that
      * matters to the implementation).
      *
-     * The first time ClusterManager.cluster() is invoked on a set of items
-     * onBeforeClusterItemRendered() will be called and onClusterItemUpdated() will not be called.
-     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
-     * again, then onClusterItemUpdated() will be called and onBeforeClusterItemRendered() will not
-     * be called.
+     * The first time {@link ClusterManager#cluster()} is invoked on a set of items
+     * {@link #onBeforeClusterItemRendered(ClusterItem, MarkerOptions)} will be called and
+     * {@link #onClusterItemUpdated(ClusterItem, Marker)} will not be called.
+     * If an item is removed and re-added (or updated) and {@link ClusterManager#cluster()} is
+     * invoked again, then {@link #onClusterItemUpdated(ClusterItem, Marker)} will be called and
+     * {@link #onBeforeClusterItemRendered(ClusterItem, MarkerOptions)} will not be called.
      *
      * @param item item being updated
      * @param marker cached marker that contains a potentially previous state of the item.
@@ -801,11 +803,12 @@ protected void onClusterItemUpdated(T item, Marker marker) {
      * Called before the marker for a Cluster is added to the map.
      * The default implementation draws a circle with a rough count of the number of items.
      *
-     * The first time ClusterManager.cluster() is invoked on a set of items
-     * onBeforeClusterRendered() will be called and onClusterUpdated() will not be called.
-     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
-     * again, then onClusterUpdated() will be called and onBeforeClusterRendered() will not be
-     * called.
+     * The first time {@link ClusterManager#cluster()} is invoked on a set of items
+     * {@link #onBeforeClusterRendered(Cluster, MarkerOptions)} will be called and
+     * {@link #onClusterUpdated(Cluster, Marker)} will not be called. If an item is removed and
+     * re-added (or updated) and {@link ClusterManager#cluster()} is invoked
+     * again, then {@link #onClusterUpdated(Cluster, Marker)} will be called and
+     * {@link #onBeforeClusterRendered(Cluster, MarkerOptions)} will not be called.
      *
      * @param cluster cluster to be rendered
      * @param markerOptions markerOptions representing the provided cluster
@@ -818,7 +821,8 @@ protected void onBeforeClusterRendered(Cluster<T> cluster, MarkerOptions markerO
     /**
      * Gets a BitmapDescriptor for the given cluster that contains a rough count of the number of
      * items. Used to set the cluster marker icon in the default implementations of
-     * onBeforeClusterRendered() and onClusterUpdated().
+     * {@link #onBeforeClusterRendered(Cluster, MarkerOptions)} and
+     * {@link #onClusterUpdated(Cluster, Marker)}.
      *
      * @param cluster cluster to get BitmapDescriptor for
      * @return a BitmapDescriptor for the marker icon for the given cluster that contains a rough
@@ -851,11 +855,12 @@ protected void onClusterRendered(Cluster<T> cluster, Marker marker) {
      * not have changed since the cached marker was created - implementations of this method are
      * responsible for checking if something changed (if that matters to the implementation).
      *
-     * The first time ClusterManager.cluster() is invoked on a set of items
-     * onBeforeClusterRendered() will be called and onClusterUpdated() will not be called.
-     * If an item is removed and re-added (or updated) and ClusterManager.cluster() is invoked
-     * again, then onClusterUpdated() will be called and onBeforeClusterRendered() will not be
-     * called.
+     * The first time {@link ClusterManager#cluster()} is invoked on a set of items
+     * {@link #onBeforeClusterRendered(Cluster, MarkerOptions)} will be called and
+     * {@link #onClusterUpdated(Cluster, Marker)} will not be called. If an item is removed and
+     * re-added (or updated) and {@link ClusterManager#cluster()} is invoked
+     * again, then {@link #onClusterUpdated(Cluster, Marker)} will be called and
+     * {@link #onBeforeClusterRendered(Cluster, MarkerOptions)} will not be called.
      *
      * @param cluster cluster being updated
      * @param marker cached marker that contains a potentially previous state of the cluster
@@ -995,7 +1000,7 @@ private void perform(MarkerModifier markerModifier) {
     }
 
     /**
-     * A Marker and its position. Marker.getPosition() must be called from the UI thread, so this
+     * A Marker and its position. {@link Marker#getPosition()} must be called from the UI thread, so this
      * object allows lookup from other threads.
      */
     private static class MarkerWithPosition {

From a1b1c17eac8d71ecbb0758bac6f1f514547c21f1 Mon Sep 17 00:00:00 2001
From: Sean Barbeau <barbeau@cutr.usf.edu>
Date: Thu, 20 Feb 2020 11:20:57 -0500
Subject: [PATCH 3/5] Add boolean return values to Algorithm add/remove/update
 operations

* Implementation follows the Java Collection definitions of the matching methods
* Synchronize within update() implementations to ensure they are atomic operations
* Add unit tests for all Algorithm operations
---
 .../android/clustering/ClusterManager.java    | 36 ++++++---
 .../android/clustering/algo/Algorithm.java    | 43 ++++++++--
 .../clustering/algo/GridBasedAlgorithm.java   | 56 ++++++++++---
 ...NonHierarchicalDistanceBasedAlgorithm.java | 80 +++++++++++++++----
 .../algo/PreCachingAlgorithmDecorator.java    | 45 +++++++----
 .../algo/ScreenBasedAlgorithmAdapter.java     | 20 ++---
 .../maps/android/clustering/QuadItemTest.java | 45 +++++++++--
 7 files changed, 250 insertions(+), 75 deletions(-)

diff --git a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
index c3aadc133..174186145 100644
--- a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
+++ b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
@@ -164,70 +164,86 @@ public void clearItems() {
      * Adds items to clusters. After calling this method you must invoke {@link #cluster()} for the
      * state of the clusters to be updated on the map.
      * @param items items to add to clusters
+     * @return true if the cluster manager contents changed as a result of the call
      */
-    public void addItems(Collection<T> items) {
+    public boolean addItems(Collection<T> items) {
+        boolean result;
         mAlgorithm.lock();
         try {
-            mAlgorithm.addItems(items);
+            result = mAlgorithm.addItems(items);
         } finally {
             mAlgorithm.unlock();
         }
+        return result;
     }
 
     /**
      * Adds an item to a cluster. After calling this method you must invoke {@link #cluster()} for
      * the state of the clusters to be updated on the map.
      * @param myItem item to add to clusters
+     * @return true if the cluster manager contents changed as a result of the call
      */
-    public void addItem(T myItem) {
+    public boolean addItem(T myItem) {
+        boolean result;
         mAlgorithm.lock();
         try {
-            mAlgorithm.addItem(myItem);
+            result = mAlgorithm.addItem(myItem);
         } finally {
             mAlgorithm.unlock();
         }
+        return result;
     }
 
     /**
      * Removes items from clusters. After calling this method you must invoke {@link #cluster()} for
      * the state of the clusters to be updated on the map.
      * @param items items to remove from clusters
+     * @return true if the cluster manager contents changed as a result of the call
      */
-    public void removeItems(Collection<T> items) {
+    public boolean removeItems(Collection<T> items) {
+        boolean result;
         mAlgorithm.lock();
         try {
-            mAlgorithm.removeItems(items);
+            result = mAlgorithm.removeItems(items);
         } finally {
             mAlgorithm.unlock();
         }
+        return result;
     }
 
     /**
      * Removes an item from clusters. After calling this method you must invoke {@link #cluster()}
      * for the state of the clusters to be updated on the map.
      * @param item item to remove from clusters
+     * @return true if the item was removed from the cluster manager as a result of this call
      */
-    public void removeItem(T item) {
+    public boolean removeItem(T item) {
+        boolean result;
         mAlgorithm.lock();
         try {
-            mAlgorithm.removeItem(item);
+            result = mAlgorithm.removeItem(item);
         } finally {
             mAlgorithm.unlock();
         }
+        return result;
     }
 
     /**
      * Updates an item in clusters. After calling this method you must invoke {@link #cluster()} for
      * the state of the clusters to be updated on the map.
      * @param item item to update in clusters
+     * @return true if the item was updated in the cluster manager, false if the item is not
+     * contained within the cluster manager and the cluster manager contents are unchanged
      */
-    public void updateItem(T item) {
+    public boolean updateItem(T item) {
+        boolean result;
         mAlgorithm.lock();
         try {
-            mAlgorithm.updateItem(item);
+            result = mAlgorithm.updateItem(item);
         } finally {
             mAlgorithm.unlock();
         }
+        return result;
     }
 
     /**
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java b/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java
index d9ef6ba25..8784afe1c 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/Algorithm.java
@@ -27,17 +27,44 @@
  */
 public interface Algorithm<T extends ClusterItem> {
 
-    void addItem(T item);
-
-    void addItems(Collection<T> items);
+    /**
+     * Adds an item to the algorithm
+     * @param item the item to be added
+     * @return true if the algorithm contents changed as a result of the call
+     */
+    boolean addItem(T item);
+
+    /**
+     * Adds a collection of items to the algorithm
+     * @param items the items to be added
+     * @return true if the algorithm contents changed as a result of the call
+     */
+    boolean addItems(Collection<T> items);
 
     void clearItems();
 
-    void removeItem(T item);
-
-    void updateItem(T item);
-
-    void removeItems(Collection<T> items);
+    /**
+     * Removes an item from the algorithm
+     * @param item the item to be removed
+     * @return true if this algorithm contained the specified element (or equivalently, if this
+     * algorithm changed as a result of the call).
+     */
+    boolean removeItem(T item);
+
+    /**
+     * Updates the provided item in the algorithm
+     * @param item the item to be updated
+     * @return true if the item existed in the algorithm and was updated, or false if the item did
+     * not exist in the algorithm and the algorithm contents remain unchanged.
+     */
+    boolean updateItem(T item);
+
+    /**
+     * Removes a collection of items from the algorithm
+     * @param items the items to be removed
+     * @return true if this algorithm contents changed as a result of the call
+     */
+    boolean removeItems(Collection<T> items);
 
     Set<? extends Cluster<T>> getClusters(float zoom);
 
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java b/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java
index 85daeda12..b0787e733 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/GridBasedAlgorithm.java
@@ -38,14 +38,24 @@ public class GridBasedAlgorithm<T extends ClusterItem> extends AbstractAlgorithm
 
     private final Set<T> mItems = Collections.synchronizedSet(new HashSet<T>());
 
+    /**
+     * Adds an item to the algorithm
+     * @param item the item to be added
+     * @return true if the algorithm contents changed as a result of the call
+     */
     @Override
-    public void addItem(T item) {
-        mItems.add(item);
+    public boolean addItem(T item) {
+        return mItems.add(item);
     }
 
+    /**
+     * Adds a collection of items to the algorithm
+     * @param items the items to be added
+     * @return true if the algorithm contents changed as a result of the call
+     */
     @Override
-    public void addItems(Collection<T> items) {
-        mItems.addAll(items);
+    public boolean addItems(Collection<T> items) {
+        return mItems.addAll(items);
     }
 
     @Override
@@ -53,20 +63,44 @@ public void clearItems() {
         mItems.clear();
     }
 
+    /**
+     * Removes an item from the algorithm
+     * @param item the item to be removed
+     * @return true if this algorithm contained the specified element (or equivalently, if this
+     * algorithm changed as a result of the call).
+     */
     @Override
-    public void removeItem(T item) {
-        mItems.remove(item);
+    public boolean removeItem(T item) {
+        return mItems.remove(item);
     }
 
+    /**
+     * Removes a collection of items from the algorithm
+     * @param items the items to be removed
+     * @return true if this algorithm contents changed as a result of the call
+     */
     @Override
-    public void removeItems(Collection<T> items) {
-        mItems.removeAll(items);
+    public boolean removeItems(Collection<T> items) {
+        return mItems.removeAll(items);
     }
 
+    /**
+     * Updates the provided item in the algorithm
+     * @param item the item to be updated
+     * @return true if the item existed in the algorithm and was updated, or false if the item did
+     * not exist in the algorithm and the algorithm contents remain unchanged.
+     */
     @Override
-    public void updateItem(T item) {
-        removeItem(item);
-        addItem(item);
+    public boolean updateItem(T item) {
+        boolean result;
+        synchronized (mItems) {
+            result = removeItem(item);
+            if (result) {
+                // Only add the item if it was removed (to help prevent accidental duplicates on map)
+                result = addItem(item);
+            }
+        }
+        return result;
     }
 
     @Override
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java b/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java
index 802ef7f14..68ec10162 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.java
@@ -62,20 +62,39 @@ public class NonHierarchicalDistanceBasedAlgorithm<T extends ClusterItem> extend
 
     private static final SphericalMercatorProjection PROJECTION = new SphericalMercatorProjection(1);
 
+    /**
+     * Adds an item to the algorithm
+     * @param item the item to be added
+     * @return true if the algorithm contents changed as a result of the call
+     */
     @Override
-    public void addItem(T item) {
+    public boolean addItem(T item) {
+        boolean result;
         final QuadItem<T> quadItem = new QuadItem<>(item);
         synchronized (mQuadTree) {
-            mItems.add(quadItem);
-            mQuadTree.add(quadItem);
+            result = mItems.add(quadItem);
+            if (result) {
+                mQuadTree.add(quadItem);
+            }
         }
+        return result;
     }
 
+    /**
+     * Adds a collection of items to the algorithm
+     * @param items the items to be added
+     * @return true if the algorithm contents changed as a result of the call
+     */
     @Override
-    public void addItems(Collection<T> items) {
+    public boolean addItems(Collection<T> items) {
+        boolean result = false;
         for (T item : items) {
-            addItem(item);
+            boolean individualResult = addItem(item);
+            if (individualResult) {
+                result = true;
+            }
         }
+        return result;
     }
 
     @Override
@@ -86,35 +105,68 @@ public void clearItems() {
         }
     }
 
+    /**
+     * Removes an item from the algorithm
+     * @param item the item to be removed
+     * @return true if this algorithm contained the specified element (or equivalently, if this
+     * algorithm changed as a result of the call).
+     */
     @Override
-    public void removeItem(T item) {
+    public boolean removeItem(T item) {
+        boolean result;
         // QuadItem delegates hashcode() and equals() to its item so,
         //   removing any QuadItem to that item will remove the item
         final QuadItem<T> quadItem = new QuadItem<>(item);
         synchronized (mQuadTree) {
-            mItems.remove(quadItem);
-            mQuadTree.remove(quadItem);
+            result = mItems.remove(quadItem);
+            if (result) {
+                mQuadTree.remove(quadItem);
+            }
         }
+        return result;
     }
 
+    /**
+     * Removes a collection of items from the algorithm
+     * @param items the items to be removed
+     * @return true if this algorithm contents changed as a result of the call
+     */
     @Override
-    public void removeItems(Collection<T> items) {
+    public boolean removeItems(Collection<T> items) {
+        boolean result = false;
         synchronized (mQuadTree) {
             for (T item : items) {
                 // QuadItem delegates hashcode() and equals() to its item so,
                 //   removing any QuadItem to that item will remove the item
                 final QuadItem<T> quadItem = new QuadItem<>(item);
-                mItems.remove(quadItem);
-                mQuadTree.remove(quadItem);
+                boolean individualResult = mItems.remove(quadItem);
+                if (individualResult) {
+                    mQuadTree.remove(quadItem);
+                    result = true;
+                }
             }
         }
+        return result;
     }
 
+    /**
+     * Updates the provided item in the algorithm
+     * @param item the item to be updated
+     * @return true if the item existed in the algorithm and was updated, or false if the item did
+     * not exist in the algorithm and the algorithm contents remain unchanged.
+     */
     @Override
-    public void updateItem(T item) {
+    public boolean updateItem(T item) {
         // TODO - Can this be optimized to update the item in-place if the location hasn't changed?
-        removeItem(item);
-        addItem(item);
+        boolean result;
+        synchronized (mQuadTree) {
+            result = removeItem(item);
+            if (result) {
+                // Only add the item if it was removed (to help prevent accidental duplicates on map)
+                result = addItem(item);
+            }
+        }
+        return result;
     }
 
     @Override
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java b/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java
index 806086dc5..0a7032e85 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java
@@ -44,15 +44,21 @@ public PreCachingAlgorithmDecorator(Algorithm<T> algorithm) {
     }
 
     @Override
-    public void addItem(T item) {
-        mAlgorithm.addItem(item);
-        clearCache();
+    public boolean addItem(T item) {
+        boolean result = mAlgorithm.addItem(item);
+        if (result) {
+            clearCache();
+        }
+        return result;
     }
 
     @Override
-    public void addItems(Collection<T> items) {
-        mAlgorithm.addItems(items);
-        clearCache();
+    public boolean addItems(Collection<T> items) {
+        boolean result = mAlgorithm.addItems(items);
+        if (result) {
+            clearCache();
+        }
+        return result;
     }
 
     @Override
@@ -62,21 +68,30 @@ public void clearItems() {
     }
 
     @Override
-    public void removeItem(T item) {
-        mAlgorithm.removeItem(item);
-        clearCache();
+    public boolean removeItem(T item) {
+        boolean result = mAlgorithm.removeItem(item);
+        if (result) {
+            clearCache();
+        }
+        return result;
     }
 
     @Override
-    public void removeItems(Collection<T> items) {
-        mAlgorithm.removeItems(items);
-        clearCache();
+    public boolean removeItems(Collection<T> items) {
+        boolean result = mAlgorithm.removeItems(items);
+        if (result) {
+            clearCache();
+        }
+        return result;
     }
 
     @Override
-    public void updateItem(T item) {
-        mAlgorithm.updateItem(item);
-        clearCache();
+    public boolean updateItem(T item) {
+        boolean result = mAlgorithm.updateItem(item);
+        if (result) {
+            clearCache();
+        }
+        return result;
     }
 
     private void clearCache() {
diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java b/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java
index d0664efc2..c84d8c349 100644
--- a/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java
+++ b/library/src/main/java/com/google/maps/android/clustering/algo/ScreenBasedAlgorithmAdapter.java
@@ -37,13 +37,13 @@ public boolean shouldReclusterOnMapMovement() {
     }
 
     @Override
-    public void addItem(T item) {
-        mAlgorithm.addItem(item);
+    public boolean addItem(T item) {
+        return mAlgorithm.addItem(item);
     }
 
     @Override
-    public void addItems(Collection<T> items) {
-        mAlgorithm.addItems(items);
+    public boolean addItems(Collection<T> items) {
+        return mAlgorithm.addItems(items);
     }
 
     @Override
@@ -52,18 +52,18 @@ public void clearItems() {
     }
 
     @Override
-    public void removeItem(T item) {
-        mAlgorithm.removeItem(item);
+    public boolean removeItem(T item) {
+        return mAlgorithm.removeItem(item);
     }
 
     @Override
-    public void removeItems(Collection<T> items) {
-        mAlgorithm.removeItems(items);
+    public boolean removeItems(Collection<T> items) {
+        return mAlgorithm.removeItems(items);
     }
 
     @Override
-    public void updateItem(T item) {
-        mAlgorithm.updateItem(item);
+    public boolean updateItem(T item) {
+        return mAlgorithm.updateItem(item);
     }
 
     @Override
diff --git a/library/src/test/java/com/google/maps/android/clustering/QuadItemTest.java b/library/src/test/java/com/google/maps/android/clustering/QuadItemTest.java
index 8778474e2..c0888b4b3 100644
--- a/library/src/test/java/com/google/maps/android/clustering/QuadItemTest.java
+++ b/library/src/test/java/com/google/maps/android/clustering/QuadItemTest.java
@@ -21,7 +21,9 @@
 
 import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -30,23 +32,48 @@
 public class QuadItemTest {
 
     @Test
-    public void testRemoval() {
-        TestingItem item_1_5 = new TestingItem(0.1, 0.5);
-        TestingItem item_2_3 = new TestingItem(0.2, 0.3);
+    public void testAddRemoveUpdateClear() {
+        ClusterItem item_1_5 = new TestingItem("title1", 0.1, 0.5);
+        ClusterItem item_2_3 = new TestingItem("title2", 0.2, 0.3);
 
         NonHierarchicalDistanceBasedAlgorithm<ClusterItem> algo =
                 new NonHierarchicalDistanceBasedAlgorithm<>();
-        algo.addItem(item_1_5);
-        algo.addItem(item_2_3);
+        assertTrue(algo.addItem(item_1_5));
+        assertTrue(algo.addItem(item_2_3));
 
         assertEquals(2, algo.getItems().size());
 
-        algo.removeItem(item_1_5);
+        assertTrue(algo.removeItem(item_1_5));
 
         assertEquals(1, algo.getItems().size());
 
         assertFalse(algo.getItems().contains(item_1_5));
         assertTrue(algo.getItems().contains(item_2_3));
+
+        // Update the item still in the algorithm
+        ((TestingItem) item_2_3).setTitle("newTitle");
+        assertTrue(algo.updateItem(item_2_3));
+
+        // Try to remove the item that was already removed
+        assertFalse(algo.removeItem(item_1_5));
+
+        // Try to update the item that was already removed
+        assertFalse(algo.updateItem(item_1_5));
+
+        algo.clearItems();
+        assertEquals(0, algo.getItems().size());
+
+        // Test bulk operations
+        List<ClusterItem> items = Arrays.asList(item_1_5, item_2_3);
+        assertTrue(algo.addItems(items));
+
+        // Try to bulk add items that were already added
+        assertFalse(algo.addItems(items));
+
+        assertTrue(algo.removeItems(items));
+
+        // Try to bulk remove items that were already removed
+        assertFalse(algo.removeItems(items));
     }
 
     /**
@@ -73,7 +100,7 @@ public void testInsertionOrder() {
 
     private class TestingItem implements ClusterItem {
         private final LatLng mPosition;
-        private final String mTitle;
+        private String mTitle;
 
         TestingItem(String title, double lat, double lng) {
             mTitle = title;
@@ -99,5 +126,9 @@ public String getTitle() {
         public String getSnippet() {
             return null;
         }
+
+        public void setTitle(String title) {
+            mTitle = title;
+        }
     }
 }

From 45af836aba6a1871a60150ee346c2bb183b298e6 Mon Sep 17 00:00:00 2001
From: Sean Barbeau <barbeau@cutr.usf.edu>
Date: Thu, 20 Feb 2020 15:49:04 -0500
Subject: [PATCH 4/5] Remove unneeded variable holding return values

"finally" clause will always be invoked (even after return), so we can just return the result of mAlgorithm method directly
---
 .../android/clustering/ClusterManager.java    | 20 +++++--------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
index 174186145..8c1cbb468 100644
--- a/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
+++ b/library/src/main/java/com/google/maps/android/clustering/ClusterManager.java
@@ -167,14 +167,12 @@ public void clearItems() {
      * @return true if the cluster manager contents changed as a result of the call
      */
     public boolean addItems(Collection<T> items) {
-        boolean result;
         mAlgorithm.lock();
         try {
-            result = mAlgorithm.addItems(items);
+            return mAlgorithm.addItems(items);
         } finally {
             mAlgorithm.unlock();
         }
-        return result;
     }
 
     /**
@@ -184,14 +182,12 @@ public boolean addItems(Collection<T> items) {
      * @return true if the cluster manager contents changed as a result of the call
      */
     public boolean addItem(T myItem) {
-        boolean result;
         mAlgorithm.lock();
         try {
-            result = mAlgorithm.addItem(myItem);
+            return mAlgorithm.addItem(myItem);
         } finally {
             mAlgorithm.unlock();
         }
-        return result;
     }
 
     /**
@@ -201,14 +197,12 @@ public boolean addItem(T myItem) {
      * @return true if the cluster manager contents changed as a result of the call
      */
     public boolean removeItems(Collection<T> items) {
-        boolean result;
         mAlgorithm.lock();
         try {
-            result = mAlgorithm.removeItems(items);
+            return mAlgorithm.removeItems(items);
         } finally {
             mAlgorithm.unlock();
         }
-        return result;
     }
 
     /**
@@ -218,14 +212,12 @@ public boolean removeItems(Collection<T> items) {
      * @return true if the item was removed from the cluster manager as a result of this call
      */
     public boolean removeItem(T item) {
-        boolean result;
         mAlgorithm.lock();
         try {
-            result = mAlgorithm.removeItem(item);
+            return mAlgorithm.removeItem(item);
         } finally {
             mAlgorithm.unlock();
         }
-        return result;
     }
 
     /**
@@ -236,14 +228,12 @@ public boolean removeItem(T item) {
      * contained within the cluster manager and the cluster manager contents are unchanged
      */
     public boolean updateItem(T item) {
-        boolean result;
         mAlgorithm.lock();
         try {
-            result = mAlgorithm.updateItem(item);
+            return mAlgorithm.updateItem(item);
         } finally {
             mAlgorithm.unlock();
         }
-        return result;
     }
 
     /**

From acfdec032aee78e76741aa4221bd8c386ee1dd32 Mon Sep 17 00:00:00 2001
From: Sean Barbeau <barbeau@cutr.usf.edu>
Date: Thu, 20 Feb 2020 16:07:40 -0500
Subject: [PATCH 5/5] Remove refactor TODO

See discussion at https://github.com/googlemaps/android-maps-utils/pull/627/files#r382258252. We'll open a new issue to discuss this specifically.
---
 .../maps/android/clustering/view/DefaultClusterRenderer.java     | 1 -
 1 file changed, 1 deletion(-)

diff --git a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
index 76fc8b396..7c0e01d52 100644
--- a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
+++ b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
@@ -952,7 +952,6 @@ private void perform(MarkerModifier markerModifier) {
                         } else {
                             markerOptions.position(item.getPosition());
                         }
-                        // TODO (?) - Refactor set marker text into onBeforeClusterItemRendered()
                         if (!(item.getTitle() == null) && !(item.getSnippet() == null)) {
                             markerOptions.title(item.getTitle());
                             markerOptions.snippet(item.getSnippet());