js-assessment-answers icon indicating copy to clipboard operation
js-assessment-answers copied to clipboard

Answer for Array duplicates doesn't pass test

Open ryanhamley opened this issue 9 years ago • 7 comments

The current answer for the duplicates question is this:

var seen = {};
var dupes = [];

for (var i = 0, len = arr.length; i < len; i++) {
  seen[arr[i]] = seen[arr[i]] ? seen[arr[i]] + 1 : 1;
}

for (var item in seen) {
  if (seen.hasOwnProperty(item) && seen[item] > 1) {
    dupes.push(item);
  }
}

return dupes;

The test rejects it saying:

AssertionError: expected [ '1', '3', '4' ] to deeply equal [ 1, 3, 4 ]
    at Context.<anonymous> (tests/app/arrays.js:96:30)

I changed the following line in my code to make it pass:

dupes.push(item)

//changes to

dupes.push(parseInt(item, 10));

It seems like either the test should expect strings or the answer should be updated.

ryanhamley avatar Jul 09 '15 23:07 ryanhamley

+1, except I would assume them to be float number as well.

So my suggestion would be

dupes.push(Number(item));

ldskywalker avatar Oct 08 '15 01:10 ldskywalker

I made a pull-req on #43.

technowar avatar Oct 23 '15 06:10 technowar

The same problem I meet and .push(parseInt(item)); also works.

geekinglcq avatar Feb 09 '16 14:02 geekinglcq

None of these solutions is optimal as it assumes the array entries are numbers. What is needed is a data structure that preserves the type of the key when tracking for duplicates. An object won't do this as the keys will be converted to strings.

adamdonahue avatar Apr 08 '16 12:04 adamdonahue

@adamdonahue

I'm curious. Can I look at your solution sir? I'd like to learn more of this.

technowar avatar Apr 09 '16 06:04 technowar

@technowar A solution could be to use a map. Maps can have any value – objects and primitives – as its keys.

I just tried it out, and it works flawlessly

duplicates : function(arr) {
    var observations = new Map();
    var duplicates = [];

    for (var i = 0, len = arr.length; i < len; i++) {
      let seen = observations.get(arr[i]) || 0;
      observations.set(arr[i], seen + 1);
    }

    observations.forEach(function (val, key) {
      if (val > 1) duplicates.push(key);
    });

    return duplicates;
}

praffn avatar Apr 11 '16 08:04 praffn

Are we supposed to purposely avoid ES6 methods? Cause this works much better than the proposed solution:

    var storage = [];
    var solution = [];
    
    arr.forEach(function(value) {
      if (storage.includes(value) && !solution.includes(value)){
        solution.push(value);
      }
      storage.push(value);
    });
    
    return solution;

And the performance is much better too: https://jsperf.com/check-for-duplicates-in-array

screen shot 2018-02-08 at 11 03 32 am

zeckdude avatar Feb 08 '18 19:02 zeckdude