0

A file I'm working with that was made long before I started working here has a list of 53 different numeric values. But it has them all listed as a string separated by commas as such for example:

var numbers = "1,2,3,4,5,...,48,49,50";

Later in the script, when there is a check to see if a variable that's passed in exists, it uses: 

if(numbers.indexOf(String(x),0) < 0) {/*do stuff*/} else {/*do other stuff*/}

While this works, I can't help but wonder why this way of doing it was chosen as opposed to making it an array of numbers. Would that be more optimal in terms of performance then doing it the way it is currently set up?

JoeL
  • 710
  • 4
  • 18
  • 2
    Why? Because the guy before you thought it was a good idea. If you're doing frequent lookups then you should convert it to an array [since `indexOf` is pretty slow](http://stackoverflow.com/questions/6682951/why-is-looping-through-an-array-so-much-faster-than-javascripts-native-indexof) and the data seems to imply an array. – Mike Cluck May 24 '16 at 16:55
  • 1
    probably it was written before array indexOf was supported. But the string indexof is flawed since it does partial matches. – epascarello May 24 '16 at 16:56
  • 3
    if string not contains `4` and contains `14`, if you checked for index of `4` it will return index of `4` in `14`. but in case of array it will return `-1`. – Pranav C Balan May 24 '16 at 16:58
  • 2
    i suggest to use an object with the numbers as key. the access ist much more faster than indexOf. – Nina Scholz May 24 '16 at 16:58
  • @PranavCBalan I get what you're saying, but I used 1-50 as an example. The actual values are all 9 digit id numbers that are all different so that problem wouldn't arise. – JoeL May 24 '16 at 17:01
  • @MikeC Thank you for the link reference. The performance difference is really what I was looking for. – JoeL May 24 '16 at 17:02
  • Thank you @NinaScholz I'll look into doing that too. – JoeL May 24 '16 at 17:03
  • 2
    or event better use an ES6 [set](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Set) as this is exactly what it was designed for. – Euan Smith May 24 '16 at 17:05
  • @MikeC Although apparently the newest answer at the link you provided seems to point out that the original response is quite old and now indexOf is the fastest method. I'll do more searching on this as I'm now quite curious. – JoeL May 24 '16 at 17:13
  • 1
    @NinaScholz : always object is better than array.... indexOf method will be much slower.... – Pranav C Balan May 24 '16 at 17:16
  • @JoeL : I just specified one possible difference – Pranav C Balan May 24 '16 at 17:17
  • The above given code is horrible for what it tries to accomplish... – Redu May 24 '16 at 20:12

1 Answers1

0

A proposal for faster access over the content of a string with values, separated by comma with an object.

var numbers = "1,2,3,4,5,6,48,49,50",
    object = Object.create(null), // create empty object without any properties/prototypes
    x = 48;

numbers.split(',').forEach(function (a, i) {
    object[a] = i; // position of item
});

// usage
if (x in object) {
    /*do stuff*/
    alert('pos: ' + object[x]);
} else {
    /*do other stuff*/
}
Nina Scholz
  • 376,160
  • 25
  • 347
  • 392