Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve .unique to make use of es6 Set performance #185

Merged
merged 2 commits into from Apr 25, 2016
Merged

improve .unique to make use of es6 Set performance #185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d01f254
improve .unique to make use of es6 `Set` performance
catalint Apr 5, 2016
ae706c7
added more tests for unique, check for same result with deprecated ve…
catalint Apr 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 17 additions & 12 deletions lib/index.js
View file
Open in desktop
Expand Up @@ -369,18 +369,23 @@ exports.deepEqual = function (obj, ref, options, seen) {

// Remove duplicate items from array

exports.unique = function (array, key) {

const index = {};
const result = [];

for (let i = 0; i < array.length; ++i) {
const id = (key ? array[i][key] : array[i]);
if (index[id] !== true) {

result.push(array[i]);
index[id] = true;
}
exports.unique = (array, key) => {

let result;
if (key) {
result = [];
const index = new Set();
array.forEach((item) => {

const identifier = item[key];
if (!index.has(identifier)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if it would be more efficient to build the set without checking .has (since sets don't allow duplicate members anyway), and then returning Array.from(theSet) like we do below. building both a set and an array at the same time seems wasteful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, scratch that, i just realized what this is actually doing

index.add(identifier);
result.push(item);
}
});
}
else {
result = Array.from(new Set(array));
}

return result;
Expand Down
69 changes: 63 additions & 6 deletions test/index.js
View file
Open in desktop
Expand Up @@ -36,8 +36,32 @@ const nestedObj = {
z: new Date(1378775452757)
};

const dupsArray = [nestedObj, { z: 'z' }, nestedObj];
const reducedDupsArray = [nestedObj, { z: 'z' }];
internals.unique = {
item: {
objects: [nestedObj, { z: 'z' }]
}
};

internals.unique.objectsByKey = {
dups: [internals.unique.item.objects[0], internals.unique.item.objects[1], internals.unique.item.objects[0]],
result: [internals.unique.item.objects[0], internals.unique.item.objects[1]]
};
internals.unique.objects = {
dups: [internals.unique.item.objects[1], internals.unique.item.objects[0], internals.unique.item.objects[0]],
result: [internals.unique.item.objects[1], internals.unique.item.objects[0]]
};
internals.unique.integers = {
dups: [1, 2, 3, 2, 2, 1, 3, 4, 5],
result: [1, 2, 3, 4, 5]
};
internals.unique.strings = {
dups: ['a', 'b', 'c', 'd', 'a', 'c', 'e'],
result: ['a', 'b', 'c', 'd', 'e']
};
internals.unique.mixed = {
dups: [1, 2, 'a', 'b', internals.unique.item.objects[0], 'a', '2', 3, internals.unique.item.objects[0]],
result: [1, 2, 'a', ',b', internals.unique.item.objects[0], 3]
};

describe('clone()', () => {

Expand Down Expand Up @@ -1249,16 +1273,49 @@ describe('deepEqual()', () => {

describe('unique()', () => {

const deprecatedUnique = function (array, key) { // previous method of unique from hapi 3.0.4

const index = {};
const result = [];

for (let i = 0; i < array.length; ++i) {
const id = (key ? array[i][key] : array[i]);
if (index[id] !== true) {

result.push(array[i]);
index[id] = true;
}
}

return result;
};

it('ensures uniqueness within array of objects based on subkey', (done) => {

const a = Hoek.unique(dupsArray, 'x');
expect(a).to.deep.equal(reducedDupsArray);
expect(Hoek.unique(internals.unique.objectsByKey.dups, 'x')).to.deep.equal(internals.unique.objectsByKey.result);
expect(deprecatedUnique(internals.unique.objectsByKey.dups, 'x')).to.deep.equal(internals.unique.objectsByKey.result);

done();
});

it('removes duplicated integers without key', (done) => {

expect(Hoek.unique(internals.unique.integers.dups)).to.deep.equal(internals.unique.integers.result);
expect(deprecatedUnique(internals.unique.integers.dups)).to.deep.equal(internals.unique.integers.result);
done();
});

it('removes duplicated strings without key', (done) => {

expect(Hoek.unique(internals.unique.strings.dups)).to.deep.equal(internals.unique.strings.result);
expect(deprecatedUnique(internals.unique.strings.dups)).to.deep.equal(internals.unique.strings.result);
done();
});

it('removes duplicated without key', (done) => {
it('removes duplicated objects without key', (done) => { // this was not supported in earlier versions

expect(Hoek.unique([1, 2, 3, 4, 2, 1, 5])).to.deep.equal([1, 2, 3, 4, 5]);
expect(Hoek.unique(internals.unique.objects.dups)).to.deep.equal(internals.unique.objects.result);
expect(deprecatedUnique(internals.unique.objects.dups)).to.not.deep.equal(internals.unique.objects.result);
done();
});
});
Expand Down