30

this seems to be a common javascript idiom:

function foo (array, index) {
    if (typeof array[index] == 'undefined')
        alert ('out of bounds baby');
}

as opposed to the more prevalent (in other languages) and conceptually simpler:

function foo (array, index) {
    if (index >= array.length)
        alert ('boo');
}

I understand that the first case will also work for arrays which have 'gaps' in them, but is that a common enough case to warrant the idiom?

The code sample that prompted this question can be seen here. In this case, when using the 'argument' variable inside a function, isn't it sane to assume that it will be a contiguous array?

Community
  • 1
  • 1
Manav
  • 10,094
  • 6
  • 44
  • 51
  • I think you're misunderstanding the linked code. The replace function could access the arguments array randomly (a string looking like `"{1} {3} {5} {2}"` is one example), which might have interesting effects on the arguments you'd pass it. My point is that the check there isn't to check the bounds of the array, it's to check existence and provide fallback if the named property is undefined. 99% of the time you want to check against the `length` property, the linked code is one of the few times you don't. – Nick Husher Jul 18 '11 at 03:59
  • Also, the test should be `if (index < array.length)` since the length is **always** one more than the last index and `array[array.length]` will always be undefined. – RobG Jul 18 '11 at 04:05
  • 1
    As an exercise, try to execute the following code: `var arr = []; arr[5] = 2;`. Executing the first first loop, you will get an out of bounds message immediately at index 0, but arr.length will correctly tell you that `arr` has 6 elements. – c1moore Sep 05 '15 at 13:12

6 Answers6

37

The only correct way is to check the index vs. the length.

An element may be assigned the value undefined. It is just silly to use it for a sentinel here. (There may be other, valid and possibly overlapping, reasons for checking for undefined, but not "for an out of bound check" -- the code in the other question will present arguably wrong results when the value of the given arg is really undefined.)

Happy coding.

22

You can also write:

if (index in array) {

which will return true even if array[index] is set to undefined.

Vlad
  • 7,997
  • 3
  • 56
  • 43
Sophie Alpert
  • 139,698
  • 36
  • 220
  • 238
  • 12
    This won't always work -- although I believe it will always work in the case of `arguments` which is merely "array-like" -- because arrays in JavaScript are "sparse". This is why `for (v in array) { ... }` is sometimes problematic. (There is a difference between having an `undefined` value, which this works for, and a *not set* index, which this misses). For instance, consider that `var a = []; a[1] = "foo"; 0 in a` results in false. The array in `a` has the own-properties "1" and "length" but it does not have the property named "0". –  Jul 18 '11 at 16:35
1

Do not test for undefined. You should use the length of the array. There are cases where it simply does not work to test for undefined because undefined is a legal value for legitimate array entry. Here's a legal JS array:

var legalArray = [4, undefined, "foo"];

And you can access it like this:

var legalArray = [4, undefined, "foo"];

var result = "";
for (var i = 0; i < legalArray.length; i++) {
    result += legalArray[i] + "<br>";
}

$("#result").html(result);

Generates this output:

4
undefined
foo

As seen in this jsFiddle: http://jsfiddle.net/jfriend00/J5PPe/

jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

It's not common as far as I know, much more common is:

for (var i=0, iLen=array.length; i<iLen; i++) {
  // do stuff
}

You should not compare to undefined, since a member of the array may have been assigned a value of undefined, or may not have been assigned any value.

e.g.

var a = [0,,,,];

alert(a.length); // 4 (or 5 in buggy IE);

a[1] === undefined; // true but not out of bounds

The main reason for using a for loop is that array properties may not be returned in numeric order if a for..in loop is used.

A for..in loop is much more efficient in sparse arrays but the possible out-of-order access must be dealt with if it matters (as must inherited and non-numeric enumerable properties if they should be avoided).

RobG
  • 142,382
  • 31
  • 172
  • 209
0

In JavaScript arrays can be sparse - they can contain "holes". For example

const array = new Array(3);

results in an array of three "holes" - not values. So while

const isInBounds = 0 <= index && index < array.length;

correctly identifies whether index is within bounds on array it does not indicate whether there is a value at array[index].

Object.prototype.hasOwnProperty() can be used to determine whether a value exists at an index. It also needs to be noted that different parts of the language can behave quite differently in the presence of "holes".

// ESLint: no-prototype-builtins)
const hasOwnProperty = Object.prototype.hasOwnProperty;

function show(array, index) {
  const msg =
    0 > index || index >= array.length
      ? `index ${index} is out of bounds`
      : !hasOwnProperty.call(array, index)
      ? `index ${index} is a hole`
      : `index ${index} holds ${array[index]}`;

  console.log(msg);
}

const subject = [undefined, , 1];

show(subject, -1);
// "index -1 is out of bounds"

for (let i = 0; i < subject.length; i += 1) show(subject, i);
// "index 0 holds undefined"
// "index 1 is a hole"
// "index 2 holds 1"

show(subject, 3);
// "index 3 is out of bounds"

const toString = (value) =>
  value !== undefined ? value.toString() : 'undefined';

// for..of doesn't skip holes
const byForOf = [];
for (const value of subject) byForOf.push(toString(value));
console.log(`Values found by for..of: ${byForOf.join(', ')}`);
// "Values found by for..of: undefined, undefined, 1"

// .forEach skips holes
const byForEach = [];
subject.forEach((value) => byForEach.push(toString(value)));
console.log(`Values found by .forEach: ${byForEach.join(', ')}`);
// "Values found by .forEach: undefined, 1"

// .reduce skips holes
const reducer = (acc, value) => {
  acc.push(toString(value));
  return acc;
};
const byReduce = subject.reduce(reducer, []);
console.log(`Values found by .reduce: ${byReduce.join(', ')}`);
// "Values found by .reduce: undefined, 1"

// .map preserves holes
const byMap = subject.map(toString);
console.log(`Values found by .map: ${byMap.join(', ')}`);
// "Values found by .map: undefined, , 1"
Peer Reynders
  • 546
  • 3
  • 6
-2

In that case, the test it to make sure that it does not accidentally add the String "undefined" to the calling String. In the case below, it will actually do just that:

var undefd;
"{0} is dead, but {1} is alive! {0} {2}".format("ASP", undefd, "ASP.NET")
// output as "ASP is dead, but {1} is alive! ASP ASP.NET"

Personally, though, I would probably simply cache the length and then do a numeric comparison.

EDIT

Side note: his method also avoids a NaN check but forces a strict parallel:

// this will fail unless 0001 is cast to a number, which means the method
// provided will fail. 
"{0} is dead, but {1} is alive! {0001} {2}".format("ASP", "ASP.NET")
cwallenpoole
  • 79,954
  • 26
  • 128
  • 166