Issue 5735 - Use JS Map instead of Object for Filter.knownFilters

lib/filterClasses.js

Index: lib/filterClasses.js
diff --git a/lib/filterClasses.js b/lib/filterClasses.js
index 626eabad9538553550f08df724b14fbd077cd08b..95ac951962ebb81c8a534bd7f9c1d53264b0383e 100644
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -81,7 +81,7 @@ Filter.prototype =
  * Cache for known filters, maps string representation to filter objects.
  * @type {Object}

[Wladimir Palant, 2017/09/20 11:00:27]

Actually, this needs to be adjusted - type should be Map.<string,Filter>

[sergei, 2017/09/20 11:31:27]

Done.

  */
-Filter.knownFilters = Object.create(null);
+Filter.knownFilters = new Map();
 
 /**
  * Regular expression that element hiding filters should match
@@ -108,10 +108,10 @@ Filter.optionsRegExp = /\$(~?[\w-]+(?:=[^,\s]+)?(?:,~?[\w-]+(?:=[^,\s]+)?)*)$/;
  */
 Filter.fromText = function(text)
 {
-  if (text in Filter.knownFilters)
-    return Filter.knownFilters[text];
+  let ret = Filter.knownFilters.get(text);

[Wladimir Palant, 2017/09/20 10:04:13]

Nit: maybe a more meaningful name for this variable, such as existingFilter?

[sergei, 2017/09/20 10:27:33]

I think it's not for this change because `ret` is used here and in the method
`Filter.fromObject` below, looks already like a pattern and it would be better
to rename them at the same time, for example to `filter`. In addition later this
variable is re-used for a new filter and one variable is actually sufficient
here.

Is it worth creation of the Noissue review with the renaming to `filter`?

[Wladimir Palant, 2017/09/20 10:58:00]

I see.

[kzar, 2017/09/20 12:30:24]

Yea I think so, if you don't want to just give it a better name now.

[sergei, 2017/09/20 13:12:45]

Done in https://codereview.adblockplus.org/29550667/

+  if (ret)
+    return ret;
 
-  let ret;
   let match = (text.includes("#") ? Filter.elemhideRegExp.exec(text) : null);
   if (match)
   {
@@ -136,7 +136,7 @@ Filter.fromText = function(text)
   else
     ret = RegExpFilter.fromText(text);
 
-  Filter.knownFilters[ret.text] = ret;
+  Filter.knownFilters.set(ret.text, ret);
   return ret;
 };