Skip to content

lastIndexOf' does something odd with indices #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
paf31 opened this issue Jan 9, 2016 · 4 comments
Closed

lastIndexOf' does something odd with indices #55

paf31 opened this issue Jan 9, 2016 · 4 comments

Comments

@paf31
Copy link
Contributor

paf31 commented Jan 9, 2016

Try it here http://sharkdp.github.io/purescript-strings/

@paf31 paf31 changed the title lastIndexOf does something odd with indices lastIndexOf' does something odd with indices Jan 9, 2016
@sharkdp
Copy link
Contributor

sharkdp commented Jan 16, 2016

Apparently that is the intended behavior. From MDN:

fromIndex (Optional)
The index at which to start searching backwards in the string. It can be any integer. The default value is str.length - 1, so the whole array is searched. If fromIndex >= str.length, the whole string is searched. If fromIndex < 0, the behavior will be the same as if it would be 0.

So we should probably just document it.

@sharkdp
Copy link
Contributor

sharkdp commented Jan 16, 2016

Or are you referring to the fact that it does not yield Nothing if the index is equal to the string length (i.e. out of bounds):

exports["_lastIndexOf'"] = function (just) {
  return function (nothing) {
    return function (x) {
      return function (startAt) {
        return function (s) {
          if (startAt < 0 || startAt > s.length) return nothing;
          var i = s.lastIndexOf(x, startAt);
          return i === -1 ? nothing : just(i);
        };
      };
    };
  };
};

I think this should probably be changed to startAt > s.length - 1.

@sharkdp
Copy link
Contributor

sharkdp commented Mar 24, 2016

I have looked at this again, and it actually seems fine to me like it is. The index thing I mentioned in the last post still seems weird to me, but it makes sense when looking at the test cases (i.e. if you search for an empty string).

In my opinion, this issue can be closed.

I have to say, though, that I would be completely fine with removing this function alltogether (same holds for indexOf').

@garyb
Copy link
Member

garyb commented Mar 24, 2016

The indexOf' varieties are much faster than dropping from a string and then searching again, but maybe you're right... I've never used them myself, although I think I did add them originally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants