Add Buffer.prototype.lastIndexOf()

[felixfbecker]

The docs only mention indexOf, but it's there, I would like to use it and don't feel good using undocumented API

$ node
> Buffer.prototype.lastIndexOf
[Function: lastIndexOf]

[cjihrig]

I believe that comes from the Uint8Array prototype, not Buffer itself like indexOf() does.

EDIT:

> Buffer.prototype.lastIndexOf === Uint8Array.prototype.lastIndexOf;
true
> Buffer.prototype.indexOf === Uint8Array.prototype.indexOf;
false

[felixfbecker]

> Uint8Array.prototype.indexOf
[Function: indexOf]
> Uint8Array.prototype.lastIndexOf
[Function: lastIndexOf]

[felixfbecker]

It also has the same method signature: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/indexOf

[mscdex]

@felixfbecker I think Buffer's indexOf() is a custom implementation though, it doesn't use Uint8Array?

[felixfbecker]

[mscdex]

@felixfbecker Presumably because implementations for uint8Arrray.indexOf() aren't necessarily fast? buffer.indexOf() uses Boyer Moore, I'm not sure what v8 uses for their uint8Array.indexOf() implementation.

[felixfbecker]

I'm not familiar with the different algorithms but why has indexOf then a special implementation and lastIndexOf doesn't?

[mscdex]

@felixfbecker Probably because nobody has created a PR to add lastIndexOf() yet.

[felixfbecker]

@mscdex thanks for the explanation. I would say this isnt a high priority, but it's confusing when browsing the API docs to see the indexOf method documented like it was special to Buffer even though it has the exact same API as the Uint8Array, just different implementation.

[feross]

The implementation of buffer.indexOf is different from unit8array.indexOf.

Uint8Array.prototype.indexOf can only search for individual elements (i.e. numbers).

Buffer.prototype.indexOf can search for string, Buffer or numbers, so it's more similar to String.prototype.indexOf than Uint8Array.prototype.indexOf.

[emars]

Does documentation for this belong in the buffer API file? (https://github.com/emars/node/blob/master/doc/api/buffer.markdown). I found that the method does not work to search for Sub-Buffers or Strings(returns -1 in both cases), only numbers. Seems kinda buggy to put in the core docs. Any thoughts?

[mscdex]

@emars That's probably worth creating a new issue for.

[zeusdeux]

I would like to take this up if no one else is already on it.

Edit: What do these byteOffset checks signify?

[zeusdeux]

So I have two ways of looking at the implementation for this:

1. Call indexOfString, etc with a reversed copy of array pointed to by this and include adjusted offset (dunno what the adjustment is as yet)
2. Implement indexOfNumberFromEnd, etc which use something like memrchr and then build lastIndexOf in terms of 'em

Option one obviously has a big perf hit and two involves either implementing new functions or passing flags to existing ones to tell which end to search from.

Which sounds better? Is there another approach that anyone has in mind?

/cc @mscdex @cjihrig

[dcposch]

@zeusdeux sorry, just saw your posts. just posted a PR with a patch. i started working on it on a few days ago on an island in the middle of nowhere :)

[dcposch]

the PR is a work in progress: interface and tests, boyer moore not done. in short, it works but it's slow

[dcposch]

PR now done, I think. Reviewers appreciated!

#4846

@felixfbecker @trevnorris @mikeal

[silverwind]

Oh hey, this was implemented in 6c1e5ad!