rmurphey / js-assessment-answers

125 stars 84 forks source link

Answer for Array duplicates doesn't pass test #41

Open ryanhamley opened 9 years ago

ryanhamley commented 9 years ago

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.

ldskywalker commented 9 years ago

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

So my suggestion would be

dupes.push(Number(item));
technowar commented 9 years ago

I made a pull-req on #43.

geekinglcq commented 8 years ago

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

adamdonahue commented 8 years ago

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.

technowar commented 8 years ago

@adamdonahue

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

praffn commented 8 years ago

@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;
}
zeckdude commented 6 years ago

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