search

rmurphey / js-assessment-answers

125 stars 84 forks source link

Faster duplicate finder #20

Closed MattSurabian closed 9 years ago

MattSurabian commented 10 years ago

Instead of needing to check an object for a calculated occurrence count we can check if the first index matches the last index of a given value. Though I suppose there could be a cross browser issue here as lastIndexOf isn't supported by IE 8 and below. Maybe lastIndexOf could be another test function if that's a concern :-p

When I first saw the current answer I expected it to be faster given the number of times the proposed change set calls indexOf. Ran a JSPerf test on it and found the opposite is true, didn't check in IE, but all modern browsers show varying degrees of speed increase by using the indexOf checks, chrome being the most dramatic. Anyway, figured I'd submit a PR.

MattSurabian commented 10 years ago

A few more rounds of testing the JSperf in firefox indicate the speed difference between the approaches to be negligible. V8 optimization must really love indexOf and lastIndexOf, can't understand why it's so much faster in chrome...

ashleygwilliams commented 9 years ago

hey @MattSurabian not sure what you think of this PR anymore, but it's now quite out of sync with master. i'm gonna close- please open a new issue/PR if you have any further questions/suggestions!