CODEWITHSUNDEEP
javascript
GTA.sprx
GTA.sprx

Reputation: 827

Return index of last occurrence in array

I have been working on this for a while now and I have no idea why this isn't working. I've read over it a few times and I can't seem to find the problem in my code.

I thought if(arr[i] === item) would check if the current element is equal to the item parameter and if true, return the index.

const lastIndexOf = (arr, item) => {
  if(arr.includes(item)){
    for(let i = arr.length - 1; i >= 0; i--){
      if(arr[i] === item){
        return i;
      } else if (arr.includes(i) === false){
        return Number('-1');
      }
    }
  } else if (!arr.length){
    return Number('-1');
  } else {
    return Number('-1');
  }
}

console.log(lastIndexOf([5,5,5,5,0],5));

Upvotes: 1

Views: 802

Answers (2)

user12407908
user12407908

Reputation:

The problem is here:

} else if (arr.includes(i) === false){

You're asking if any of the array members equals the current index. Because 4 (your first checked index) is not accounted for in that array, it returns.

Seems like you don't need that else if at all.

const lastIndexOf = (arr, item) => {
  if(arr.includes(item)){
    for(let i = arr.length - 1; i >= 0; i--){
      if(arr[i] === item){
        return i;
      }
    }
  } else {
    return -1;
  }
}

console.log(lastIndexOf([5,5,5,5,0],5));

Also, the last, outer else if and else branches yield the same result, so the else if is extraneous, so I removed it.

And Number('-1') is an unnecessarily roundabout way of returning -1.

And actually, your .includes() check seems redundant. It's going to have to iterate before you iterate. Might as well just do it once.

const lastIndexOf = (arr, item) => {
  for(let i = arr.length - 1; i >= 0; i--){
    if(arr[i] === item){
      return i;
    }
  }
  return -1;
}

console.log(lastIndexOf([5,5,5,5,0],5));

Upvotes: 0

Unmitigated
Unmitigated

Reputation: 89179

Remove the else if branch, as it is checking if the array contains the current index, which is nonsensical. Also, just use -1 instead of Number('-1').

const lastIndexOf = (arr, item) => {
  if(arr.includes(item)){
    for(let i = arr.length - 1; i >= 0; i--){
      if(arr[i] === item){
        return i;
      }
    }
    return -1;
  } else {
    return -1;
  }
}

console.log(lastIndexOf([5,5,5,5,0],5));

You are currently passing through the array twice, once using Array#includes and once with your for loop, which is suboptimal. You can simply loop over the array backwards once, like you are currently doing, and return -1 at the end of the function, since it will only reach there if the item has not been found.

const lastIndexOf = (arr, item) => {
  for(let i = arr.length - 1; i >= 0; i--){
    if(arr[i] === item){
      return i;
    }
  }
  return -1;
}
console.log(lastIndexOf([5,5,5,5,0],5));

The Array#lastIndexOf method already exists, so there is no need to reinvent the wheel, unless you are trying to make a polyfill.

console.log([5,5,5,5,0].lastIndexOf(5));

Upvotes: 3

Related Questions