Skip to content

Add ImmutableEqVector::find_slice #15948

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
wants to merge 2 commits into from
Closed

Add ImmutableEqVector::find_slice #15948

wants to merge 2 commits into from

Conversation

nwin
Copy link
Contributor

@nwin nwin commented Jul 24, 2014

Provide an equivalent of &str::find_str().
Closes #15853.

Provide an equivalent of &str::find_str().
Closes #15853.
@lilyball
Copy link
Contributor

I am inclined to say find_slice() is a sensible addition, but this is a very naive implementation. Unfortunately I'm not sure if there's a better way to search for arbitrary equatable elements. find_str() has a much better implementation but it relies on being able to search bytes. I'm not sure if the dependence on a u8 value is actually critical to the implementation, but it's possible.

FWIW find_slice() can be done in one line as

let i = haystack.windows(needle.len()).position(|v| v == needle);

Not that I'm saying that this is how it should be implemented, but I am saying that, as long as it's implemented with a naive solution, I'm not sure if there's actually a particularly compelling reason to have it. I'd much rather provide a solution that's more efficient than what people can trivially do on their own.

@nwin
Copy link
Contributor Author

nwin commented Jul 24, 2014

Well, looking at the implementations of the other functions this one-liner is exactly how it should be implemented. Makes me cry.

@bluss
Copy link
Member

bluss commented Jul 24, 2014

@nwin, the simple implementation has a worst case behaviour with complexity O(n²); there exists better algorithms, and maybe the two-way algorithm used for strings can be adapted for this. It doesn't look impossible.

@nwin
Copy link
Contributor Author

nwin commented Jul 25, 2014

I see the problem. The two-way algorithm needs an ordered alphabet. KMP would be fine for the general case but the implementation is a bit more complex and would profit from unsafe blocks. Dunno if that is worth to push. But as far as I see we require at least Eq for more intelligent algorithms.

@alexcrichton
Copy link
Member

Closing due to inactivity. For now we probably shouldn't be including quadratic complexity algorithms where there exist much better solutions, but this seems like a nice function to have!

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

Successfully merging this pull request may close these issues.

Provide method find_slice() for &[T]
4 participants