Skip to content
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

Indexable#reverse_map #8998

Closed
caspiano opened this issue Apr 3, 2020 · 4 comments
Closed

Indexable#reverse_map #8998

caspiano opened this issue Apr 3, 2020 · 4 comments

Comments

@caspiano
Copy link
Contributor

caspiano commented Apr 3, 2020

We have compact_map, flat_map... why not reverse_map?

Here's a gist with Array benchmarks... the benchmarking seems favourable.
I've got a PR with tests ready #8997

@straight-shoota
Copy link
Member

I'm not sure about this. There are surely use cases for that.

We also have a lot more iteration methods than map. Why should we have reversed implementation for map only? There are a lot of methods in Indexable, Enumerable and Iterable which could have reversed implementeations.
We should discuss not only reverse_map but also reverse_reduce, reverse_index, reverse_each_*, reverse_join, reverse_cycle, reverse_group_by, reverse_flat_map and many more.
Maybe not all would be very useful, but it's hard to draw a line and certainly quite a few would seem at least as useful as reverse_map.

We already have efficient and composable iterators which can be used for that. A dedicated implementation is only significantly more performant for small sizes. The to_s specs reflect that, but I'm not sure why the i * 3 don't (supposedly a benchmarking error).

@caspiano
Copy link
Contributor Author

caspiano commented Apr 3, 2020

I agree, where does one draw the line with these specialised methods? I personally had a use for an efficient reverse_map so I put together a PR.

reverse_reduce would be a handy method. I wouldn’t mind putting together a PR for the other reversed methods.

@jhass
Copy link
Member

jhass commented Apr 6, 2020

I feel like these are getting too specialized. At the same time it seems to be why we support an open class model, so anyone can reopen any class. I think these should proliferate as shard, collection_helpers or so. If we see some of these get used a lot we can then always revisit upstreaming them.

@asterite
Copy link
Member

asterite commented Apr 6, 2020

Yes, I don't think these are common enough to belong in the standard library. And if you need such things it shouldn't be that frequent so adding a reverse in between is usually fine.

@straight-shoota straight-shoota linked a pull request Apr 8, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants