From 29adbb7cd7c8080282bef4a1095effd1a4856424 Mon Sep 17 00:00:00 2001
From: Tim van der Meij <timvandermeij@gmail.com>
Date: Sun, 12 Jul 2020 11:51:27 +0200
Subject: [PATCH 1/2] Implement unit tests for the `RefSetCache` primitive

This primitive did not have unit test coverage yet, which is important
for upcoming refactoring of the primitive.
---
 test/unit/primitives_spec.js | 59 ++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/test/unit/primitives_spec.js b/test/unit/primitives_spec.js
index 5ed82e9a8..fa1c3ab9b 100644
--- a/test/unit/primitives_spec.js
+++ b/test/unit/primitives_spec.js
@@ -27,6 +27,7 @@ import {
   Name,
   Ref,
   RefSet,
+  RefSetCache,
 } from "../../src/core/primitives.js";
 import { StringStream } from "../../src/core/stream.js";
 import { XRefMock } from "./test_utils.js";
@@ -336,6 +337,64 @@ describe("primitives", function () {
     });
   });
 
+  describe("RefSetCache", function () {
+    const ref1 = Ref.get(4, 2);
+    const ref2 = Ref.get(5, 2);
+    const obj1 = Name.get("foo");
+    const obj2 = Name.get("bar");
+    let cache;
+
+    beforeEach(function (done) {
+      cache = new RefSetCache();
+      done();
+    });
+
+    afterEach(function () {
+      cache = null;
+    });
+
+    it("should put, have and get a value", function () {
+      cache.put(ref1, obj1);
+      expect(cache.has(ref1)).toBeTruthy();
+      expect(cache.has(ref2)).toBeFalsy();
+      expect(cache.get(ref1)).toBe(obj1);
+    });
+
+    it("should put, have and get a value by alias", function () {
+      cache.put(ref1, obj1);
+      cache.putAlias(ref2, ref1);
+      expect(cache.has(ref1)).toBeTruthy();
+      expect(cache.has(ref2)).toBeTruthy();
+      expect(cache.get(ref1)).toBe(obj1);
+      expect(cache.get(ref2)).toBe(obj1);
+    });
+
+    it("should report the size of the cache", function () {
+      cache.put(ref1, obj1);
+      expect(cache.size).toEqual(1);
+      cache.put(ref2, obj2);
+      expect(cache.size).toEqual(2);
+    });
+
+    it("should clear the cache", function () {
+      cache.put(ref1, obj1);
+      expect(cache.size).toEqual(1);
+      cache.clear();
+      expect(cache.size).toEqual(0);
+    });
+
+    it("should support iteration", function () {
+      cache.put(ref1, obj1);
+      cache.put(ref2, obj2);
+
+      const values = [];
+      cache.forEach(function (value) {
+        values.push(value);
+      });
+      expect(values).toEqual([obj1, obj2]);
+    });
+  });
+
   describe("isEOF", function () {
     it("handles non-EOF", function () {
       const nonEOF = "foo";

From b19a1796ac49b4357d0b2332c829ea837dbbbea5 Mon Sep 17 00:00:00 2001
From: Tim van der Meij <timvandermeij@gmail.com>
Date: Sun, 12 Jul 2020 12:00:42 +0200
Subject: [PATCH 2/2] Convert `RefSetCache` to a proper class and to use a
 `Map` internally

Using a `Map` instead of an `Object` provides some advantages such as
cheaper ways to get the size of the cache, to find out if an entry is
contained in the cache and to iterate over the cache. Moreover, we can
clear and re-use the same `Map` object now instead of creating a new
one.
---
 src/core/primitives.js | 59 +++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/src/core/primitives.js b/src/core/primitives.js
index 9613e789e..f9c7ecbdb 100644
--- a/src/core/primitives.js
+++ b/src/core/primitives.js
@@ -239,46 +239,41 @@ class RefSet {
   }
 }
 
-var RefSetCache = (function RefSetCacheClosure() {
-  // eslint-disable-next-line no-shadow
-  function RefSetCache() {
-    this.dict = Object.create(null);
+class RefSetCache {
+  constructor() {
+    this._map = new Map();
   }
 
-  RefSetCache.prototype = {
-    get size() {
-      return Object.keys(this.dict).length;
-    },
+  get size() {
+    return this._map.size;
+  }
 
-    get: function RefSetCache_get(ref) {
-      return this.dict[ref.toString()];
-    },
+  get(ref) {
+    return this._map.get(ref.toString());
+  }
 
-    has: function RefSetCache_has(ref) {
-      return ref.toString() in this.dict;
-    },
+  has(ref) {
+    return this._map.has(ref.toString());
+  }
 
-    put: function RefSetCache_put(ref, obj) {
-      this.dict[ref.toString()] = obj;
-    },
+  put(ref, obj) {
+    this._map.set(ref.toString(), obj);
+  }
 
-    putAlias: function RefSetCache_putAlias(ref, aliasRef) {
-      this.dict[ref.toString()] = this.get(aliasRef);
-    },
+  putAlias(ref, aliasRef) {
+    this._map.set(ref.toString(), this.get(aliasRef));
+  }
 
-    forEach: function RefSetCache_forEach(callback) {
-      for (const i in this.dict) {
-        callback(this.dict[i]);
-      }
-    },
+  forEach(callback) {
+    for (const value of this._map.values()) {
+      callback(value);
+    }
+  }
 
-    clear: function RefSetCache_clear() {
-      this.dict = Object.create(null);
-    },
-  };
-
-  return RefSetCache;
-})();
+  clear() {
+    this._map.clear();
+  }
+}
 
 function isEOF(v) {
   return v === EOF;