CODEWITHSUNDEEP
javascript
WinchenzoMagnifico
WinchenzoMagnifico

Reputation: 3425

What's wrong with this indexOf implementation?

I know I'm leaving out the -1 option when value can't be found, but why doesn't it work when the value is in the Array? It should be returning 1, but is returning undefined.

function each(collection, callback) {
  if (Array.isArray(collection)) {
    for (var i = 0; i < collection.length; i++) {
      callback(collection[i], i, collection);
    }
  }
  else {
    for (var prop in collection) {
      callback(collection[prop], prop, collection);
    }
  }
}

function indexOf(array, value) {
  each(array, function(e, index) {
    if (e === value) {
      return index; 
    }
  })
}


console.log(indexOf([1, 2, 3, 4, 5], 2)); ---->>> undefined;

Upvotes: 0

Views: 505

Answers (5)

AlliterativeAlice
AlliterativeAlice

Reputation: 12577

You're returning the value of index within the callback, not within the indexOf() function itself.

Try this implementation:

function indexOf(array, value) {
   var returnVal = -1;
   each(array, function(e, index) {
        if (e === value) {
            returnVal = index; 
            return false;
        }
    });
    return returnVal;
}

EDIT: As Barmar pointed out, this will return the index of the last occurrence of the element, to return the index of the first occurrence, you'll also have to update each() to be:

function each(collection, callback) {
    if (Array.isArray(collection)) {
        for (var i = 0; i < collection.length; i++) {
            if (callback(collection[i], i, collection) === false) break;
        }
    }
    else {
        for (var prop in collection) {
            if (callback(collection[prop], prop, collection) === false) break;
        }
    }
}

Upvotes: 1

fjohannesen
fjohannesen

Reputation: 11

The indexOf function is always returning undefined because the you are only returning a value from the callback function, not the indexOf function.

Try this:

function indexOf(array, value) {
    var i;

    each(array, function(e, index) {
        if (e === value) {
            i = index; 
        }
    })

   return i;
}

Upvotes: 1

David Sherret
David Sherret

Reputation: 106640

You'll want to return the index if found. Also, you need to exit the loop once you've found the value, otherwise you're doing a lastIndexOf instead of indexOf.

function each(collection, callback) {
    if (Array.isArray(collection)) {
        for (var i = 0; i < collection.length; i++) {
            if (callback(collection[i], i, collection) === true) {
                return;
            }
        }
    }
    else {
        for (var prop in collection) {
            if (callback(collection[prop], prop, collection) === true) {
                return;
            }
        }
    }
}

function indexOf(array, value) {
    var foundIndex = -1;

    each(array, function(e, index) {
        if (e === value) {
            foundIndex = index;
            return true;
        }
    });

    return foundIndex;
}

Upvotes: 1

CupawnTae
CupawnTae

Reputation: 14580

Your indexOf() function isn't returning anything. The callback returns the index if found, but you're not doing anything with that value. You could do this:

for (var i = 0; i < collection.length; i++) {
  var result= callback(collection[i], i, collection);
  if (result) return result;
}

Upvotes: 1

James Montagne
James Montagne

Reputation: 78650

function indexOf(array, value) {
  each(array, function(e, index) {
    if (e === value) {
      return index; 
    }
  })
}

The return here is returning from the function that is being passed to each and not from your indexOf function. You need to somehow get that value outside of that function scope to return. With your current implementation of each, you would not be able to break the loop, so you would have to do something like this:

function indexOf(array, value) {
  var result = -1;

  each(array, function(e, index) {
    if (e === value) {
      result = index; 
    }
  })

  return result;
}

http://jsfiddle.net/efzogzxq/

Upvotes: 1

Related Questions