I need to speed this up to pass a code fights test (javascript)

Question

The task was to take an array and return the earliest duplicate, and if there are none return -1. I wrote it like this:

function firstDuplicate(a) {
    let singles = [];

    for (let i = 0; i < a.length; i++) {

        if (singles.indexOf(a[i]) == -1) {
            singles.push(a[i]);
        }
        else {
            return a[i];
        }

    }

    return -1;
}

It passed all tests except the hidden speed test. Is there another way to write this faster in JS? I saw a Java solution that used sets instead of arrays but I wanted to stick with JS.

Use a hash (an object) for keeping track of duplicates instead of another array. — Mark
– Mark, Commented Jul 14, 2017 at 15:49
Use a js object, not an array. Not giving you the answer since it's a quiz :) — Doug
– Doug, Commented Jul 14, 2017 at 15:49
First you should learn how array#indexOf works. It will search from the starting position to the ending position, each time. The search is in O(n) which is pretty slow. There are multiple ways of speeding this up, they all include using a different strategy / data-structure. Better data-structures are HashSet (contains in O(1)), TreeSet (contains in O(log n)). Better strategies when sticking to arrays are search algorithms like BinarySearch or sorting techniques like QuickSort. I am sure there are some implementations available in JS already. — Zabuzard
– Zabuzard, Commented Jul 14, 2017 at 15:56

Patrick Roberts · Accepted Answer · 2017-07-14 16:01:29Z

4

You can use Set in JavaScript to achieve this as well:

function firstDuplicate(array) {
  const set = new Set()

  for (const value of array) {
    if (set.has(value)) {
      return value
    }

    set.add(value)
  }

  return -1
}

console.log(firstDuplicate(['a', 1, 'b', 2, 'c', 1, 2, 3]))

The problem with your solution, as explained by @Zabuza, is your use of indexOf() which changes the time complexity of your algorithm from O(n) to O(n²), because each indexOf() is O(n).

Using Set instead Array takes advantage of hashing, which changes your lookup time by using has() for duplicates from O(n) to O(1), thus bringing the algorithm back to O(n) complexity overall.

edited Jul 14, 2017 at 16:01

answered Jul 14, 2017 at 15:51

Patrick Roberts

52.4k10 gold badges120 silver badges166 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Zabuzard Over a year ago

You could explain the reason why this is faster. For a beginner this is not clear. What is the technique used by Array#indexOf and where is the difference to Set#has. I think that would also help OP.

James Hamann Over a year ago

Thank you so much for explaining the answer. I admit it is a little over my head at the moment, but I get the gist of what you are saying. I knew it had something to do with indexOf being slow, but I had no idea you could even use a Set() in JS. Until now I have never heard of that. Thanks again, I love starting the day learning new things!

pinturic · Accepted Answer · 2017-07-14 15:49:58Z

0

Instead of pushing on singles just allocate a frequency array and increase the frequency when looping through the original array. The first time the frequency is 2 you have the earliest duplicate.

Increasing by '1' the frequency at the ith element of the array is for sure faster then push on an array and then any time searching on it.

answered Jul 14, 2017 at 15:49

pinturic

2,2631 gold badge17 silver badges31 bronze badges

2 Comments

James Hamann Over a year ago

I am not sure how to do that without using sort() and sort won't work in this scenario. How would I update frequency without knowing all the numbers that came before to test against? Wouldn't I always need to be adding to an array for that?

pinturic Over a year ago

You can preinitialize de frequency array with zero values and adding one anytime you walk the array.

yBrodsky · Accepted Answer · 2017-07-14 15:51:16Z

0

Try this one. It should give you some more speed in the cases where there are no duplicates in a large array.

function firstDuplicate(a) {
    let singles = new Set(a);

    if(singles.size == a.length)
      return -1;

    let temp = [];


    for (let i = 0; i < a.length; i++) {

        if (temp.indexOf(a[i]) == -1) {
            temp.push(a[i]);
        }
        else {
            return a[i];
        }

    }
}

answered Jul 14, 2017 at 15:51

yBrodsky

5,0614 gold badges24 silver badges35 bronze badges

Collectives™ on Stack Overflow

I need to speed this up to pass a code fights test (javascript)

3 Answers 3

2 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related