Noissue - Avoid redundant lookup in keyword-by-filter map

lib/matcher.js

Index: lib/matcher.js
===================================================================
--- a/lib/matcher.js
+++ b/lib/matcher.js
@@ -341,18 +341,19 @@
    * @param {RegExpFilter} filter
    * @returns {boolean}
    */
   isSlowFilter(filter)
   {
     let matcher = (
       filter instanceof WhitelistFilter ? this.whitelist : this.blacklist
     );
-    if (matcher.hasFilter(filter))
-      return !matcher.getKeywordForFilter(filter);
+    let keyword = matcher.getKeywordForFilter(filter);
+    if (keyword != null)
+      return !keyword;

[hub, 2018/09/13 15:52:40]

Why the `!keyword`. I'd just return the proper boolean value here.

[Manish Jethani, 2018/09/14 05:28:32]

Do you mean `keyword != ""`?

[hub, 2018/09/14 12:20:49]

no. just `return true`

[Manish Jethani, 2018/09/14 13:36:25]

Well it should be false if the keyword is an empty string.

[Jon Sonesen, 2018/09/17 19:13:18]

So, returning !keyword here would return true in the case where keyword is an
empty string, and false if it is not empty. Is that correct? perhaps based
solely on the discussion here it makes sense to return a bool for readability? I
do see that that would add an additional step. Personally, I find that this
approach makes sense and fits the previous convention (although that is not an
argument one way or another just a fact)

[Manish Jethani, 2018/09/17 23:36:35]

Sorry, I got it backwards. It should return true if it's empty. So it should be
`return !keyword` or `return keyword == ""`.
We can't return either `true` or `false` ... unless you're suggesting `return
keyword == "" ? true : false` (which doesn't seem more readable to me, YMMV).

[hub, 2018/09/18 12:19:40]

I see. Works for me as it is.

     return !matcher.findKeyword(filter);
   }
 
   /**
    * Optimized filter matching testing both whitelist and blacklist matchers
    * simultaneously. For parameters see
      {@link Matcher#matchesAny Matcher.matchesAny()}.
    * @see Matcher#matchesAny