Performant removeMarkers (removing multiple markers fast)

[GoogleCodeExporter]

Removing many markers individually is really slow, presumably due to the
repeated resetViewport()/redraw() calls. Below is removeMarkers() that
removes an array of markers and then does the reset/redraw. You have my
permission to include it in the lib.

PS. The code in the inner loop should be factored out to avoid duplication
with the removeMarker function.

MarkerClusterer.prototype.removeMarkers = function (toRemove) {
    for(var j = 0, marker; marker = toRemove[j]; j++)
    {
        var index = -1;
        if (this.markers_.indexOf) {
            index = this.markers_.indexOf(marker);
        } else {
            for (var i = 0, m; m = this.markers_[i]; i++) {
                if (m == marker) {
                    index = i;
                    continue;
                }
            }
        }

        this.markers_.splice(index, 1);
        marker.setVisible(false);
        marker.setMap(null);
    }
    this.resetViewport();
    this.redraw();
};

Original issue reported on code.google.com by soren.sv...@eb.dk on 1 May 2010 at 7:44

[GoogleCodeExporter]

Here's a refactored version: 

MarkerClusterer.prototype.removeMarkerBase_ = function (marker) {
    var index = -1;
    if (this.markers_.indexOf) {
        index = this.markers_.indexOf(marker);
    } else {
        for (var i = 0, m; m = this.markers_[i]; i++) {
            if (m == marker) {
                index = i;
                continue;
            }
        }
    }

    if (index == -1) {
        // Marker is not in our list of markers.
        return false;
    }

    this.markers_.splice(index, 1);
    marker.setVisible(false);
    marker.setMap(null);
    return true;
};

/**
* Remove a marker from the cluster.
*
* @param {google.maps.Marker} marker The marker to remove.
* @return {boolean} True if the marker was removed.
*/
MarkerClusterer.prototype.removeMarker = function (marker) {
    var removed = removed || this.removeMarkerBase_(marker);
    if (removed) {
        this.resetViewport();
        this.redraw();
        return true;
    } else {
        return false;
    }
};

MarkerClusterer.prototype.removeMarkers = function (toRemove) {
    var removed = false;
    for (var j = 0, marker; marker = toRemove[j]; j++) {
        removed = this.removeMarkerBase_(marker);
    }
    if (removed) {
        this.resetViewport();
        this.redraw();
    }
};

Original comment by soren.sv...@eb.dk on 3 May 2010 at 6:14

[GoogleCodeExporter]

The code above has very nasty bugs, this is better:
MarkerClusterer.prototype.removeMarkerBase_ = function (marker) {
    var index = -1;
    if (this.markers_.indexOf) {
        index = this.markers_.indexOf(marker);
    } else {
        for (var i = 0, m; m = this.markers_[i]; i++) {
            if (m == marker) {
                index = i;
                continue;
            }
        }
    }

    if (index == -1) {
        // Marker is not in our list of markers.
        return false;
    }

    this.markers_.splice(index, 1);
    marker.setVisible(false);
    marker.setMap(null);
    return true;
};

/**
* Remove a marker from the cluster.
*
* @param {google.maps.Marker} marker The marker to remove.
* @return {boolean} True if the marker was removed.
*/
MarkerClusterer.prototype.removeMarker = function (marker) {
    var removed = this.removeMarkerBase_(marker);
    if (removed) {
        this.resetViewport();
        this.redraw();
        return true;
    } else {
        return false;
    }
};

MarkerClusterer.prototype.removeMarkers = function (toRemove) {
    var removed = false;
    for (var j = 0, marker; marker = toRemove[j]; j++) {
        var thisremoved = this.removeMarkerBase_(marker);
        removed = removed || thisremoved;
    }
    if (removed) {
        this.resetViewport();
        this.redraw();
    }
};

Original comment by soren.sv...@eb.dk on 9 May 2010 at 12:18

[GoogleCodeExporter]

        for (var i = 0, m; m = this.markers_[i]; i++) {
            if (m == marker) {
                index = i;
                break;
            }
        }

using break breaks the loop, continue does NOT... which makes it even faster, 
yey!

Original comment by elak....@gmail.com on 2 Sep 2010 at 11:48

[GoogleCodeExporter]

Thanks for that guys, I've added the code.

Original comment by lu...@google.com on 6 Oct 2010 at 12:11

• Changed state: Fixed