Skip to content

Commit f891ddf

Browse files
authored
refactor: simplify switch and improve locking in ClusterRendererMultipleItems (#1557)
* refactor: simplify switch and improve locking in ClusterRendererMultipleItems - Replaced a traditional switch statement with an enhanced switch expression in `setAnimationType`. - Added a `setAnimationInterpolator` method to allow custom interpolators. - Ensured `lock.unlock()` is always called in `finally` blocks within `MarkerModifier` and `AnimationTask` for better resource management. - Added `@Override` annotation to `ViewModifier.run()`. * refactor: Refactor MarkerModifier to use a withLock method This commit refactors the `MarkerModifier` class to use a `withLock` method. This reduces code repetition and improves readability.
1 parent 12c5a3b commit f891ddf

File tree

1 file changed

+75
-65
lines changed

1 file changed

+75
-65
lines changed

library/src/main/java/com/google/maps/android/clustering/view/ClusterRendererMultipleItems.java

Lines changed: 75 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -107,31 +107,24 @@ public enum AnimationType {
107107
}
108108

109109
public void setAnimationType(AnimationType type) {
110-
switch (type) {
111-
case EASE_IN, ACCELERATE:
112-
animationInterp = new AccelerateInterpolator();
113-
break;
114-
case EASE_OUT:
115-
animationInterp = new DecelerateInterpolator();
116-
break;
117-
case EASE_IN_OUT:
118-
animationInterp = new AccelerateDecelerateInterpolator();
119-
break;
120-
case FAST_OUT_SLOW_IN:
121-
animationInterp = new FastOutSlowInInterpolator();
122-
break;
123-
case BOUNCE:
124-
animationInterp = new BounceInterpolator();
125-
break;
126-
case DECELERATE:
127-
animationInterp = new DecelerateInterpolator();
128-
break;
129-
default:
130-
animationInterp = new LinearInterpolator();
131-
break;
132-
}
110+
animationInterp = switch (type) {
111+
case LINEAR -> new LinearInterpolator();
112+
case EASE_IN, ACCELERATE -> new AccelerateInterpolator();
113+
case EASE_OUT, DECELERATE -> new DecelerateInterpolator();
114+
case EASE_IN_OUT -> new AccelerateDecelerateInterpolator();
115+
case FAST_OUT_SLOW_IN -> new FastOutSlowInInterpolator();
116+
case BOUNCE -> new BounceInterpolator();
117+
};
133118
}
134119

120+
/**
121+
* Sets the interpolator for the animation.
122+
*
123+
* @param interpolator the interpolator to use for the animation.
124+
*/
125+
public void setAnimationInterpolator(TimeInterpolator interpolator) {
126+
animationInterp = interpolator;
127+
}
135128

136129
/**
137130
* Markers that are currently on the map.
@@ -459,6 +452,7 @@ public void setMapZoom(float zoom) {
459452
}
460453

461454
@SuppressLint("NewApi")
455+
@Override
462456
public void run() {
463457
final MarkerModifier markerModifier = new MarkerModifier();
464458
final float zoom = mMapZoom;
@@ -676,6 +670,15 @@ private class MarkerModifier extends Handler implements MessageQueue.IdleHandler
676670
*/
677671
private boolean mListenerAdded;
678672

673+
private void withLock(Runnable runnable) {
674+
lock.lock();
675+
try {
676+
runnable.run();
677+
} finally {
678+
lock.unlock();
679+
}
680+
}
681+
679682
private MarkerModifier() {
680683
super(Looper.getMainLooper());
681684
}
@@ -686,14 +689,14 @@ private MarkerModifier() {
686689
* @param priority whether this operation should have priority.
687690
*/
688691
public void add(boolean priority, CreateMarkerTask c) {
689-
lock.lock();
690-
sendEmptyMessage(BLANK);
691-
if (priority) {
692-
mOnScreenCreateMarkerTasks.add(c);
693-
} else {
694-
mCreateMarkerTasks.add(c);
695-
}
696-
lock.unlock();
692+
withLock(() -> {
693+
sendEmptyMessage(BLANK);
694+
if (priority) {
695+
mOnScreenCreateMarkerTasks.add(c);
696+
} else {
697+
mCreateMarkerTasks.add(c);
698+
}
699+
});
697700
}
698701

699702
/**
@@ -703,14 +706,14 @@ public void add(boolean priority, CreateMarkerTask c) {
703706
* @param m the markerWithPosition to remove.
704707
*/
705708
public void remove(boolean priority, Marker m) {
706-
lock.lock();
707-
sendEmptyMessage(BLANK);
708-
if (priority) {
709-
mOnScreenRemoveMarkerTasks.add(m);
710-
} else {
711-
mRemoveMarkerTasks.add(m);
712-
}
713-
lock.unlock();
709+
withLock(() -> {
710+
sendEmptyMessage(BLANK);
711+
if (priority) {
712+
mOnScreenRemoveMarkerTasks.add(m);
713+
} else {
714+
mRemoveMarkerTasks.add(m);
715+
}
716+
});
714717
}
715718

716719
/**
@@ -721,19 +724,19 @@ public void remove(boolean priority, Marker m) {
721724
* @param to the position to animate to.
722725
*/
723726
public void animate(MarkerWithPosition marker, LatLng from, LatLng to) {
724-
lock.lock();
725-
AnimationTask task = new AnimationTask(marker, from, to, lock);
727+
withLock(() -> {
728+
AnimationTask task = new AnimationTask(marker, from, to, lock);
726729

727-
for (AnimationTask existingTask : ongoingAnimations) {
728-
if (existingTask.marker.getId().equals(task.marker.getId())) {
729-
existingTask.cancel();
730-
break;
730+
for (AnimationTask existingTask : ongoingAnimations) {
731+
if (existingTask.marker.getId().equals(task.marker.getId())) {
732+
existingTask.cancel();
733+
break;
734+
}
731735
}
732-
}
733736

734-
mAnimationTasks.add(task);
735-
ongoingAnimations.add(task);
736-
lock.unlock();
737+
mAnimationTasks.add(task);
738+
ongoingAnimations.add(task);
739+
});
737740
}
738741

739742
/**
@@ -745,19 +748,19 @@ public void animate(MarkerWithPosition marker, LatLng from, LatLng to) {
745748
* @param to the position to animate to.
746749
*/
747750
public void animateThenRemove(MarkerWithPosition marker, LatLng from, LatLng to) {
748-
lock.lock();
749-
AnimationTask animationTask = new AnimationTask(marker, from, to, lock);
750-
for (AnimationTask existingTask : ongoingAnimations) {
751-
if (existingTask.marker.getId().equals(animationTask.marker.getId())) {
752-
existingTask.cancel();
753-
break;
751+
withLock(() -> {
752+
AnimationTask animationTask = new AnimationTask(marker, from, to, lock);
753+
for (AnimationTask existingTask : ongoingAnimations) {
754+
if (existingTask.marker.getId().equals(animationTask.marker.getId())) {
755+
existingTask.cancel();
756+
break;
757+
}
754758
}
755-
}
756759

757-
ongoingAnimations.add(animationTask);
758-
animationTask.removeOnAnimationComplete(mClusterManager.getMarkerManager());
759-
mAnimationTasks.add(animationTask);
760-
lock.unlock();
760+
ongoingAnimations.add(animationTask);
761+
animationTask.removeOnAnimationComplete(mClusterManager.getMarkerManager());
762+
mAnimationTasks.add(animationTask);
763+
});
761764
}
762765

763766
@Override
@@ -821,9 +824,13 @@ private void removeMarker(Marker m) {
821824
* @return true if there is still work to be processed.
822825
*/
823826
public boolean isBusy() {
827+
lock.lock();
824828
try {
825-
lock.lock();
826-
return !(mCreateMarkerTasks.isEmpty() && mOnScreenCreateMarkerTasks.isEmpty() && mOnScreenRemoveMarkerTasks.isEmpty() && mRemoveMarkerTasks.isEmpty() && mAnimationTasks.isEmpty());
829+
return !(mCreateMarkerTasks.isEmpty()
830+
&& mOnScreenCreateMarkerTasks.isEmpty()
831+
&& mOnScreenRemoveMarkerTasks.isEmpty()
832+
&& mRemoveMarkerTasks.isEmpty()
833+
&& mAnimationTasks.isEmpty());
827834
} finally {
828835
lock.unlock();
829836
}
@@ -1257,11 +1264,11 @@ public void cancel() {
12571264
new Handler(Looper.getMainLooper()).post(this::cancel);
12581265
return;
12591266
}
1267+
lock.lock();
12601268
try {
12611269
markerWithPosition.position = to;
12621270
mRemoveOnComplete = false;
12631271
valueAnimator.cancel();
1264-
lock.lock();
12651272
ongoingAnimations.remove(this);
12661273
} finally {
12671274
lock.unlock();
@@ -1279,8 +1286,11 @@ public void onAnimationEnd(Animator animation) {
12791286

12801287
// Remove the task from the queue
12811288
lock.lock();
1282-
ongoingAnimations.remove(this);
1283-
lock.unlock();
1289+
try {
1290+
ongoingAnimations.remove(this);
1291+
} finally {
1292+
lock.unlock();
1293+
}
12841294
}
12851295

12861296
public void removeOnAnimationComplete(MarkerManager markerManager) {

0 commit comments

Comments
 (0)