Use more consistent variable names and reduce findListener_ calls

src/ol/events.js

@@ -93,6 +93,7 @@ ol.events.ListenerFunctionType;
  * @typedef {{bindTo: (Object|undefined),
  *     boundListener: (ol.events.ListenerFunctionType|undefined),
  *     callOnce: boolean,
+ *     deleteIndex: (number|undefined),
  *     listener: ol.events.ListenerFunctionType,
  *     target: (EventTarget|ol.events.EventTarget),
  *     type: string}}
@@ -121,22 +122,23 @@ ol.events.bindListener_ = function(listenerObj) {
  * Finds the matching {@link ol.events.ListenerObjType} in the given listener
  * array.
  *
- * @param {!Array<!ol.events.ListenerObjType>} listenerArray Array of listeners.
+ * @param {!Array<!ol.events.ListenerObjType>} listeners Array of listeners.
  * @param {!Function} listener The listener function.
  * @param {Object=} opt_this The `this` value inside the listener.
- * @param {boolean=} opt_remove Remove the found listener from the array.
+ * @param {boolean=} opt_setDeleteIndex Set the deleteIndex on the matching
- * @return {ol.events.ListenerObjType|undefined} The matching listener.
+ *     listener, for {@link ol.events.unlistenByKey}.
+ * @return {ol.events.ListenerObjType|undefined} The matching listener object.
  * @private
  */
-ol.events.findListener_ = function(
+ol.events.findListener_ = function(listeners, listener, opt_this,
-    listenerArray, listener, opt_this, opt_remove) {
+    opt_setDeleteIndex) {
   var listenerObj;
-  for (var i = 0, ii = listenerArray.length; i < ii; ++i) {
+  for (var i = 0, ii = listeners.length; i < ii; ++i) {
-    listenerObj = listenerArray[i];
+    listenerObj = listeners[i];
     if (listenerObj.listener === listener &&
         listenerObj.bindTo === opt_this) {
-      if (opt_remove) {
+      if (opt_setDeleteIndex) {
-        listenerArray.splice(i, 1);
+        listenerObj.deleteIndex = i;
       }
       return listenerObj;
     }
@@ -217,11 +219,11 @@ ol.events.removeListeners_ = function(target, type) {
  */
 ol.events.listen = function(target, type, listener, opt_this, opt_once) {
   var listenerMap = ol.events.getListenerMap_(target);
-  var listenerArray = listenerMap[type];
+  var listeners = listenerMap[type];
-  if (!listenerArray) {
+  if (!listeners) {
-    listenerArray = listenerMap[type] = [];
+    listeners = listenerMap[type] = [];
   }
-  var listenerObj = ol.events.findListener_(listenerArray, listener, opt_this,
+  var listenerObj = ol.events.findListener_(listeners, listener, opt_this,
       false);
   if (listenerObj) {
     if (!opt_once) {
@@ -237,7 +239,7 @@ ol.events.listen = function(target, type, listener, opt_this, opt_once) {
       type: type
     });
     target.addEventListener(type, ol.events.bindListener_(listenerObj));
-    listenerArray.push(listenerObj);
+    listeners.push(listenerObj);
   }
 
   return listenerObj;
@@ -284,10 +286,10 @@ ol.events.listenOnce = function(
  *     listener. Default is the `target`.
  */
 ol.events.unlisten = function(target, type, listener, opt_this) {
-  var listenerArray = ol.events.getListeners(target, type);
+  var listeners = ol.events.getListeners(target, type);
-  if (listenerArray) {
+  if (listeners) {
-    var listenerObj = ol.events.findListener_(listenerArray, listener,
+    var listenerObj = ol.events.findListener_(listeners, listener, opt_this,
-        opt_this, false);
+        true);
     if (listenerObj) {
       ol.events.unlistenByKey(listenerObj);
     }
@@ -307,10 +309,13 @@ ol.events.unlisten = function(target, type, listener, opt_this) {
 ol.events.unlistenByKey = function(key) {
   if (key && key.target) {
     key.target.removeEventListener(key.type, key.boundListener);
-    var listenerArray = ol.events.getListeners(key.target, key.type);
+    var listeners = ol.events.getListeners(key.target, key.type);
-    if (listenerArray) {
+    if (listeners) {
-      ol.events.findListener_(listenerArray, key.listener, key.bindTo, true);
+      var i = 'deleteIndex' in key ? key.deleteIndex : listeners.indexOf(key);
-      if (listenerArray.length === 0) {
+      if (i !== -1) {
+        listeners.splice(i, 1);
+      }
+      if (listeners.length === 0) {
         ol.events.removeListeners_(key.target, key.type);
       }
     }

test/spec/ol/events.test.js

@@ -46,25 +46,35 @@ describe('ol.events', function() {
   });
 
   describe('findListener_()', function() {
-    it('searches a listener array for a specific listener', function() {
+    var listener, listenerObj, listeners;
-      var bindTo = {id: 1};
+
-      var listener = function() {};
+    beforeEach(function() {
-      var listenerObj = {
+      listener = function() {};
+      listenerObj = {
         type: 'foo',
         target: target,
         listener: listener
       };
-      var listenerArray = [listenerObj];
+      listeners = [listenerObj];
-      var result = ol.events.findListener_(listenerArray, listener);
+    });
+
+    it('searches a listener array for a specific listener', function() {
+      var bindTo = {id: 1};
+      var result = ol.events.findListener_(listeners, listener);
       expect(result).to.be(listenerObj);
-      result = ol.events.findListener_(listenerArray, listener, bindTo);
+      result = ol.events.findListener_(listeners, listener, bindTo);
       expect(result).to.be(undefined);
       listenerObj.bindTo = bindTo;
-      result = ol.events.findListener_(listenerArray, listener);
+      result = ol.events.findListener_(listeners, listener);
       expect(result).to.be(undefined);
-      result = ol.events.findListener_(listenerArray, listener, bindTo);
+      result = ol.events.findListener_(listeners, listener, bindTo);
       expect(result).to.be(listenerObj);
     });
+    it('marks the delete index on a listener object', function() {
+      var result = ol.events.findListener_(listeners, listener, undefined, true);
+      expect(result).to.be(listenerObj);
+      expect(listenerObj.deleteIndex).to.be(0);
+    });
   });
 
   describe('getListeners()', function() {